Bitte um Prüfung meines C-Codes

Dieser "atomare" Zugriff ist unnötig, weil enc_delta eh' nur 8 Bit hat 
und sowieso unterbrechungsfrei ausgelesen wird:

1

  cli();

2

  val = enc_delta;

3

  enc_delta = 0;

4

  sei();

feg_zeven schrieb:
> Leider tut sich beim drehen am Encoder nichts.
Wie sieht die Hardware aus? Funktioniert die? Hast du Pulse am uC-Pin?
Im Code werden keine Pullups am PortC eingeschlatet. Hast du externe 
Pullups?

feg_zeven schrieb:

> des Timer E geht, in dem die Abfrage des Encoders stattfindet.  Es
> scheint als stimme die Abfrage der Pins nicht. Bin mir aber nicht
> sicher.

Na, ja
Wenn du das denkst, was hindert dich daran, das mal in einem eigenen 
Testprogramm zu verifzieren?

1

...

2

3

int main(void)

4

{

5

  PORTD.DIR = 0x00;  

6

7

  while( 1 ) {

8

9

    lass dir PORTD_IN wo ausgeben

10

  }

11

}

du musst ja nicht gleich in die vollen gehen. Genau dieses (Port 
Initialisieren und PORTD_IN) abfragen ist ja auch der Kern der Encoder 
Auswertung. Wenn du da ein Problem vermutest, dann nimm dir diese 
Kernfunktionalität getrennt vor.

Zb kommt mir komisch vor, dass in dem Code die Wörter Pullup bzw. 
Pulldown nirgends vorkommen.

(Das Testprogramm wirst du wahrscheinlich noch um eine UART ergänzen, 
damit du auch irgendwo Output generieren kannst)

feg_zeven schrieb:
> ISR(TCE0_OVF_vect)
> {
>     int8_t new, diff;
>         new = 0;
>     if( PHASE_A )
>     new = 3;
>     if( PHASE_B )
>     new ^= 1;                   // convert gray to binary
>     diff = last - new;                // difference last - new
>     if( diff & 1 ){               // bit 0 = value (1)
>       last = new;                 // store new as next last
>       enc_delta += (diff & 2) - 1;        // bit 1 = direction (+/-)
>     }
>
>    TCE0.PER=TETW;
>
> }

Ich verstehe gerade nicht warum du den Drehencoder in einer Timer_ISR 
bearbeitest, wäre es nicht besser das per PinChange-Interrupt zu machen?

Du fragst Regelmäßig die Pins ab, du must diese aber auch mit dem 
letzten Wert der Pins vergleichen.

Und zwar so:

00 => 01 => +1
00 => 10 => -1

01 => 11 => +1
01 => 00 => -1

10 => 00 => +1
10 => 11 => -1

11 => 10 => +1
11 => 01 => -1

Alle andern Übergänge sind invalid, da wurde dann ein Schritt nicht 
erkannt

Entweder du pollst das weiterhin in einer TimerISR und besser über einen 
pinInterrupt.

@ Lothar Miller (lkmiller) (Moderator) Benutzerseite

>Dieser "atomare" Zugriff ist unnötig,

Nein!

>weil enc_delta eh' nur 8 Bit hat
>und sowieso unterbrechungsfrei ausgelesen wird:

Das reicht nicht. Es muss auch unterbrechungsfrei gelöscht werden, damit 
nicht zufällig ein Interrupt GENAU zwischen Auslesen und Löschen 
reinspuckt und dadurch 1 Zählerstand verloren geht! Das ist ein SEHR 
schwer lokalisierbarer Fehler! Siehe

https://www.mikrocontroller.net/articles/Interrupt#Atomarer_Datenzugriff

@ Christian K. (the_kirsch)

>Ich verstehe gerade nicht warum du den Drehencoder in einer Timer_ISR
>bearbeitest,

Weil das der normale, bewährte Weg ist.

> wäre es nicht besser das per PinChange-Interrupt zu machen?

Nein. Und nein, das will ich nicht wieder endlos diskutieren.

>Du fragst Regelmäßig die Pins ab, du must diese aber auch mit dem
>letzten Wert der Pins vergleichen.

Das macht die Routine schon, steht alles im Artikel Drehgeber, von 
dort stammt auch der Code!

Falk Brunner schrieb:
> Das reicht nicht. Es muss auch unterbrechungsfrei gelöscht werden
Ach schlags kaputt. Das ist ja ein Read-Modify-Write...
Gut. Das Zeug muss weiterhin drin bleiben.

Christian K. schrieb:
> wäre es nicht besser das per PinChange-Interrupt zu machen?
Ich stimme Falk zu: wäre es nicht.
Wozu unnötig viele Interrupts? Wenn man aus jedem "Furz" einen Interrupt 
macht, dann kanns durchaus mal hecktisch werden.
Das siehst du sicher an dir selber auch: wenn jede Minute einer kommt 
und dich unterbricht, ist das lästig. Wenn dann mal gleichzeitig sogar 2 
oder 3 kommen, dann kann es schon mal passieren, dass du was 
durcheinander bringst.
Ich spreche jetzt dem uC keine Nachlässigkeit oder Vergesslichkeit zu, 
aber das Hauptproblem ist meist, dass dein Programm (das tadellos läuft, 
solange alle Interrupts hübsch nacheinander kommen) auf einmal 
"abstürzt", wenn 3 Interrupts auf einmal anstehen. Und das ganz einfach, 
weil du solche Unterbrechungen nicht einkalkuliert hast...

@ feg_zeven (Gast)

>komisch, aber ist ja erst 9 Tage her :D) mal mit einem Oszi geprüft und
>der macht was er soll.

Gut.

>braune ist +, weis ist GND. Muss ich denn dann trotzdem noch für die
>Pins PullUps/-downs konfigurieren?

Nein.

Zu deinem Programm.


>#define PHASE_A     (PORTD_IN & 0x01)
>#define PHASE_B     (PORTD_IN & 0x02)

Ist dein Encoder WIRKLICH dort angeschlossen?

>  encval=encode_read1();

Das ist eher nicht das, was du willst, eher so.

encval += encode_read1();

Kleiner, aber feiner Unterschied ;-)


>  CCP = CCP_IOREG_gc;
>  CLK.CTRL = >CLK_SCLKSEL_RC32M_gc;                                          // 
Clock >auf 32MHz stellen

Das geht bisweilen schief, wenn die Optimierung nicht eingeschaltet ist. 
Besser mit einem ASM-Makro arbeiten, das ist sicher. Kann man als 
einfache Funktion nutzen.

Beitrag "Re: PLL Startprobleme beim ATXmega128A1U"

>void impulsgeber_init (void)

Wir bei dir beim Start nicht aufgerufen, ist aber unkritisch.

>ISR(TCC0_OVF_vect)
>{
>  Send_UART(encval);
>  TCC0.PER = TCTW;

Das braucht man nicht, der Timer wird per Hardware automatich 
nachgeladen.
Im Gegenteil, durch dein Nachladen stimmt das Timing nicht mehr exakt.


>   TCE0.PER=TETW;

Hier ebenso.

Funktioniert denn der Rest? Kannst du per UART was senden bzw. auf dem 
PC empfangen?

ein

volatile int8_t encval = 0;

wäre auch nicht schlecht.