Endlosschleife bei Division - Interrupt Trap!

[Florian B. (florian_b709)]

Hi Leute,

ich bin dabei einige Projekte für meinen Haus-BUS endlich 
fertigzustellen. Leider habe ich ein Problem mit einem BUS-Teilnehmer. 
Verwendet werden in allen Teilnehmern entweder PIC16F1827 oder 
PIC16F1936. Der Empfang von Daten wird über einen Interrupt gelöst. 
Außerdem gibt es noch weitere Timer-Interrupts.

Das Problem besteht mit einem PIC16F1936 bei 10MHz. Nach einer 
unterschiedlichen Anzahl von Sende- & Empfangsoperationen kommt das 
Programm zum stoppen. Der uC hängt in einer Endlosschleife fest in 
folgendem Programmabschnitt:

// long unsigned unsigned division

unsigned long int
#ifdef __PICC__
#warning TODO: update cgpic and this file to use the other prototype
__lldiv(unsigned long int divisor, unsigned long int dividend)
#else
__lldiv(unsigned long int dividend, unsigned long int divisor)
#endif
{
  unsigned long int  quotient;
  unsigned char  counter;

  quotient = 0;
  if(divisor != 0) {
    counter = 1;
    while((divisor & 0x80000000UL) == 0) {
      divisor <<= 1;
      counter++;
    }
    do {
      quotient <<= 1;
      if(divisor <= dividend) {
        dividend -= divisor;
        quotient |= 1;
      }
      divisor >>= 1;
    } while(--counter != 0);
  }
  return quotient;
}

Mit Hilfe des Debuggers (Pickit3) konnte ich feststellen, dass der uC 
bei einer Division hängen bleibt. Diese Division wird aber vor dem 
Auftreten der Endlosschleife mehrmals erfolgreich durchgeführt.

Mit dem Debugger konnte ich während der Endlosschleife erkennen, dass 
der "divisor" 0 war. Das kann aber zu Beginn der Division nicht der Fall 
gewesen sein, da dies ja vorher geprüft wird.

Beim letzten Mal wurde die Division mit "2500 / 1000" durchgeführt. Ich 
vermute, dann kam ein Interrupt und anschließend war der alte Wert vom 
"divisor" = 0.

Für Hilfe wäre ich wirklich dankbar.

[Markus F. (mfro)]

Florian B. schrieb:
> Beim letzten Mal wurde die Division mit "2500 / 1000" durchgeführt. Ich
> vermute, dann kam ein Interrupt und anschließend war der alte Wert vom
> "divisor" = 0.
>

Wenn das so ist, würde ich annehmen, der Fehler liegt in einem deiner 
Interrupt-Handler und nicht im gezeigten Code.

[Dumdi D. (dumdidum)]

Wann war divisor=0? Divisor kann am Ende der Funktion Null sein (weil Du 
nach Rechts schiebst)

[dummschwaetzer (Gast)]

kann wärend der abarbeitung dieser funktion irgend eint interrupt 
zuschlagen der diese funktion noch einmal aufruft?

[Florian B. (florian_b709)]

Dumdi D. schrieb:
> Wann war divisor=0? Divisor kann am Ende der Funktion Null sein (weil Du
> nach Rechts schiebst)

Ich hab gesehen, dass "devisor" während der while-Schleife 0 war. 
Vielleicht ist ja auch der Rücksprung vom Interrut das problem. Ich 
werde dem mal nachgehen.

dummschwaetzer schrieb:
> kann wärend der abarbeitung dieser funktion irgend eint interrupt
> zuschlagen der diese funktion noch einmal aufruft?

Auch das werde ich gleich mal prüfen.

Ich danke euch schonmal für eure Tips. Manchmal braucht man eben eine 
andere Sichtweise. Ich berichte wieder!

[Florian B. (florian_b709)]

Hey Leute,

ich konnte nun ausschließen, dass die Funktion während des Interrupts 
nochmals aufgerufen wird.

Meiner Meinung nach gibt es ein Problem beim Rücksprung zur 
Compiler-Funktion (aus meinem ersten Post), die diese "unsigned long 
Division" durchführt.

Da ich mir nicht anders zu helfen wusste, habe ich nun die Beechnung 
anders gerlöst. Ich habe nun eine float-Operation aus meiner Berechnung 
gemacht. Vorher war IOut_Faktor 1000x größer. Nun taucht der Fehler 
nichtmehr auf. Vielleicht nimmt sich ja trotzdem jemand der Sache an 
oder hat noch DIE Lösung für das Problem. Wenn nötig stelle ich gerne 
auch noch meine Interrupt-Routine zur Verfügung.

Cache[x] ist ein Messwert vom ADC mit 10Bit und ist proportional zu 
einem Strom.

Vorher:

unsigned int    IOut_Faktor[2];          // Array für Umrechnungsfaktoren der Strommessungen
char            x;                       // Variablen für Schleifenzähler
unsigned long   y;                       // 32Bit Variable für Berechnung    
unsigned int    Cache[6];                // Variable für 10Bit ADC-Wert


for (x=0; x <= 1; x++) {                 // Die Ströme nacheinander errechnen (Kanal 0 + Kanal 1)
   if (Cache[x] <= IOut_Offset[x]) {     // Wenn Messwert unter dem Offset liegt,
      IOut[x] =   0;                     // den Messwert auf Null setzen
   }
   
   else{                                 // Wenn der Messwert über dem Offset liegt,
      y = (Cache[x] - IOut_Offset[x]);   // den Offset vom Messwert abziehen,
      y = (y*IOut_Faktor[x]);            // das Ergebnis Faktorisieren,
      y = (y/1000);                      // durch 1000 teilen
      IOut[x] =   y;                     // Speichern vom Ergenbnis (Strom in mA)
      }
   }

Nachher:

float           IOut_Faktor[2];          // Array für Umrechnungsfaktoren der Strommessungen
char            x;                       // Variablen für Schleifenzähler
unsigned long   y;                       // 32Bit Variable für Berechnung    
unsigned int    Cache[6];                // Variable für 10Bit ADC-Wert


for (x=0; x <= 1; x++) {                 // Die Ströme nacheinander errechnen (Kanal 0 + Kanal 1)
   if (Cache[x] <= IOut_Offset[x]) {     // Wenn Messwert unter dem Offset liegt,
      IOut[x] =   0;                     // den Messwert auf Null setzen
   }
   
   else{                                 // Wenn der Messwert über dem Offset liegt,
      y = (Cache[x] - IOut_Offset[x]);   // den Offset vom Messwert abziehen,
      y = (y*IOut_Faktor[x]);            // das Ergebnis Faktorisieren,
      IOut[x] =   y;                     // Speichern vom Ergenbnis (Strom in mA)
      }
   }

[mh (Gast)]

Florian B. schrieb:
> Meiner Meinung nach gibt es ein Problem beim Rücksprung zur
> Compiler-Funktion (aus meinem ersten Post), die diese "unsigned long
> Division" durchführt.

Wenn das tatsächlich der Fall ist, handelt es sich vermutlich um ein 
Symptom eines Bugs der sich in deinem Teil des Codes befindet. Damit 
hast du jetzt das Symptom bekämft, aber der Bug ist vermutlich immer 
noch vorhanden.

In einer solchen Situation hilft es nur die Fehlerursache zu finden und 
vollständig zu erklären.

Wir können dir aktuell nicht wirklich Helfen, da wir kein vollständiges 
(und minimales) compilierbares Beispiel haben in dem der Fehler 
auftritt.

[asdfasd (Gast)]

Meine erste Vermutung: der Interrupt zermantscht ein Register oder den 
Stack.

[Markus F. (mfro)]

asdfasd schrieb:
> Meine erste Vermutung: der Interrupt zermantscht ein Register oder den
> Stack.

Sehr gut möglich.

Die 16x PICs haben ja nur einen 8 (?) Elemente tiefen HW-Stack. Wenn man 
da zwei unsigned longs und eine Rücksprungadresse (+ lokale Variablen) 
draufpackt, ist nicht mehr viel übrig. Wenn dann noch dummerweise ein 
Interrupt kommt, ist Ende Gelände.

[Florian B. (florian_b709)]

asdfasd schrieb:
> Meine erste Vermutung: der Interrupt zermantscht ein Register oder den
> Stack.

Das ist ja auch meine Vermutung, aber darauf habe ich keinen Einfluss. 
Ich habe ja bereits ausgeschlossen, dass diese Funktion nochmals 
aufgerufen wird. Auch ohne Code-Optimierungen ist der Fehler 
aufgetreten.

Aktuell verwende ich "MPLAB X IDE mit dem xc8 v1.44".

Markus F. schrieb:
> Die 16x PICs haben ja nur einen 8 (?) Elemente tiefen HW-Stack.

Der PIC16F1936 besitzt sogar 16 Stack Level. Außerdem habe ich den 
Stackoverflow-Reset eingeschaltet um diese Variante zu erkennen. Leider 
ohne Erfolg.

Anbei mal das komplette Programm.

[asdfasd (Gast)]

> Anbei mal das komplette Programm.

[Nur ein kurzer Blick - gut möglich, dass ich was falsch verstanden hab]

Das Handling der BUF-Länge schaut mir dubios aus. Insbesondere in der 
Interrupt-Routine wird BUF[6] gelesen, bevor es überhaupt empfangen 
wurde. Außerdem glaubt die Routine BUF[6] bedingungslos - wenn da Mist 
gesendet wurde, schreibt sie quer über den Speicher.

Btw, die ganzen "<="-Vergleiche sind oft ne Quelle von off-by-one-Bugs.

[Florian B. (florian_b709)]

Hi asdfasd,

danke schonmal, dass du dir so eine mühe gegeben hast. So schnell hatte 
ich mit keiner Antwort gerechnet.

asdfasd schrieb:
> [Nur ein kurzer Blick - gut möglich, dass ich was falsch verstanden hab]
>
> Das Handling der BUF-Länge schaut mir dubios aus. Insbesondere in der
> Interrupt-Routine wird BUF[6] gelesen, bevor es überhaupt empfangen
> wurde.

Ich hoffe, ich hab die richtige Passage gefunden, die du meinst:
Der Vergleich

else if (BUS_Pointer == (BUS[6] + 8))

 ist erst erfüllt, wenn mindestens 8Bytes fehlerfrei empfangen wurden. 
Sollte ein Byte zu spät kommen, wird BUS_Pointer resettet. Daher sehe 
ich dort kein Problem. Natürlich mache ich auch (offenkundig) Fehler.


asdfasd schrieb:
> Außerdem glaubt die Routine BUF[6] bedingungslos - wenn da Mist
> gesendet wurde, schreibt sie quer über den Speicher.

Das ist soweit korrekt, ich hoffe dem zu begegnen mit meiner "Lo-CRC" 
aus einem einfachen Zähler. Kommt allerdings ein Byte zu wenig an, wird 
BUS_Pointer resettet.


asdfasd schrieb:
> Btw, die ganzen "<="-Vergleiche sind oft ne Quelle von off-by-one-Bugs.

Dem werde ich gleich nochmal nachgehen. Ich habe diese Routine aber 
bereits in diversen Controllern problemfrei im Einsatz.

[asdfasd (Gast)]

> Ich hoffe, ich hab die richtige Passage gefunden, die du meinst:

Ja, BUS, nicht BUF.

> Der Vergleich
>

else if (BUS_Pointer == (BUS[6] + 8))

> ist erst erfüllt, wenn mindestens 8 Bytes fehlerfrei empfangen wurden.

Ein "BUS_Pointer >= 7" seh ich aber nirgendwo.
[Btw, wenn BUS[6] größer als 0xf7 ist, ist die Bedingung immer false.]

>> Außerdem glaubt die Routine BUF[6] bedingungslos - wenn da Mist
>> gesendet wurde, schreibt sie quer über den Speicher.
>
> Das ist soweit korrekt, ich hoffe dem zu begegnen mit meiner "Lo-CRC"

Zu spät, dann ist der Speicher schon überschrieben.

[Florian B. (florian_b709)]

asdfasd schrieb:
> Ein "BUS_Pointer >= 7" seh ich aber nirgendwo.
> [Btw, wenn BUS[6] größer als 0xf7 ist, ist die Bedingung immer false.]

Da bin ich dann auch drauf gekommen, nachdem du drauf hingewiesen 
hattest. Ich habe nur etwas länger gebraucht. Mir fehlt der Abstand zu 
dem Programm (Wald vor lauter Bäumen nicht und so...)

Ich habe nun folgende Zeilen eingefügt:

else if (BUS_Pointer == 6 && (BUS[6] > 50)) {    // -> Nachrichtenlänge nicht okay <-
   TMR0CS = 1;                                   // Timer0 zur Zeitüberlaufüberwachung stoppen
   BUS_Init();                                   // UART neustarten
}
else if (BUS_Pointer == (BUS[6] + 8)){          // Wurden alle Bytes empfangen?
...
etc.
...

...und in der BUS_Init(); werden anschließend alle "falsch" gefüllten 
Zellen geleert.

unsigned char   Pointer;
for (Pointer = 0; Pointer < 58; Pointer++){      // Wenn noch nicht alle Zellen gelöscht wurden,
   BUS[Pointer] = 0;                             // nächstes Byte löschen.
}

Danke schonmal bis hierher. Ich werde das nachher mal testen.

[Florian B. (florian_b709)]

Hi,

ich danke dir asdfasd. Nun scheint alles zu funktionieren. Da hatte ich 
mir aber einen schönen Fehler eingebaut ;-)

Grüße und vielen Dank,
Flo