C Code ich verstehe den Fehler nicht

OP #3253352
Lesenswert?

Hallo,
Ich versuche einen BCD-Schalter einzulesen und ihn vorher zu entprellen. 
Das soll alles mit einem Timerinterrupt geschehen.
1
#include <hidef.h> /* for EnableInterrupts macro */
2
#include "derivative.h" /* include peripheral declarations */
3
#include "MCUInit.h"     //Interrupts, IO-Ausgänge, SPI
4
#include "IOVerarbeitung.h"   //BCD-Schalter, DIL-Schalter, LEDS, SPI
5

6
#define uint8 unsigned char
7

8
uint8 key_state;  // debounced and inverted key state:
9
          
10
uint8 key_press;  // bit = 1: key pressed
11

12
volatile bool status_bcd;
13
volatile uint8 ISR_counter;
14

15
void main(void)
16
{
17
  PeriphInit();
18
  for(;;) {
19
  __RESET_WATCHDOG();
20
    
21
    if(status_bcd == true) {
22
     status_bcd = 0;
23
     //Sende_SPI(framerate);
24
    } else{
25
     //Der BCD-Schalter wurde nicht verändert 
26
    }
27
  }
28
 }
29
 
30
 interrupt VectorNumber_Vtpm2ovf void Timer_ISR() {
31

32
   static uint8 ct0, ct1;
33
   uint8 i;
34
 
35
   SRTISC_RTIACK= 1;
36
   
37
   ISR_counter++;
38
    if(ISR_counter == 4) {
39
       ISR_counter = 0;
40

41
      i = key_state ^ ~KEY_PIN;                       // key changed ?
42
      ct0 = ~( ct0 & i );                             // reset or count ct0
43
      ct1 = ct0 ^ (ct1 & i);                          // reset or count ct1
44
      i &= ct0 & ct1;                                 // count until roll over ?
45
      key_state ^= i;                                 // then toggle debounced state
46
      key_press |= key_state & i; 
47

48
      status_bcd = BCD_Verarbeiten(BCD_Auslesen());
49
      //Jetzt kann ich den Status des Schalters abfragen
50
   }  
51
 }
1
//IOVerarbeitung.h
2

3
//Wert von BCD-Schalter auslesen und in eine Dezimalzahl konvertieren 
4
uint8 BCD_Auslesen(void) {
5
  buffer = PTGD;
6
  buffer = (buffer & 0xf); 
7
  return ((((buffer & 0xF0) >> 4) * 10) + (buffer & 0x0F));   //Rückgabewert: BCD-Schalterzahl in dezimaler Form
8
}
9

10

11
//Wert von BCD-Schalter weiterverarbeiten => entsprechende Framerate setzen
12
bool BCD_Verarbeiten(uint8 i) {
13

14
  if(lastresult_bcd == i) {
15
   return false;
16
  } 
17
  else{
18
        if(i == 0)
19
          framerate = 23.97;
20
   }
21
//Hier sthet normal noch mehr
22
      return true;
23
}

Fehler kommt im True Time Simulator immer der hier:

Error: At location FFCC -
Error: Attempt to use invalid or uninitialized memory

Error: Interrupt processing failed.

Ich könnte mir vorstellen, dass etwas mit der Speicherbehandlung nicht 
stimmt. Es könnte auch damit zusammenhängen, dass ich eventuell die 
"PeDa"-Lösung falsch eingebunden habe.

Prozessor :MC9S08AW60 von freescale
http://www.freescale.com/files/microcontrollers/doc/data_sheet/MC9S08AW60.pdf
#3253382
Lesenswert?

Jan R. schrieb:

> Fehler kommt im True Time Simulator immer der hier:
>
> Error: At location FFCC -
> Error: Attempt to use invalid or uninitialized memory
>
> Error: Interrupt processing failed.


Hört sich erst mal nach einem Problem in der ISR an.

Geh halt mal so vor:
kommentiere alles aus der ISR aus, so dass nichts drinnen ist.
Fehler noch da?

Wenn nicht dann fang einzelne Teile wieder einzubinden. Als erstes
1
interrupt VectorNumber_Vtpm2ovf void Timer_ISR() {
2

3
   static uint8 ct0, ct1;
4
   uint8 i;
5
 
6
   SRTISC_RTIACK= 1;
7

8
/*   
9
   ISR_counter++;
10
    if(ISR_counter == 4) {
11
       ISR_counter = 0;
12

13
      i = key_state ^ ~KEY_PIN;                       // key changed ?
14
      ct0 = ~( ct0 & i );                             // reset or count ct0
15
      ct1 = ct0 ^ (ct1 & i);                          // reset or count ct1
16
      i &= ct0 & ct1;                                 // count until roll over ?
17
      key_state ^= i;                                 // then toggle debounced state
18
      key_press |= key_state & i; 
19

20
      status_bcd = BCD_Verarbeiten(BCD_Auslesen());
21
      //Jetzt kann ich den Status des Schalters abfragen
22
   }
23
*/  
24
 }

Fehler da?

Wenn nicht, dann wieder einen Teil mit dazu.
1
interrupt VectorNumber_Vtpm2ovf void Timer_ISR() {
2

3
   static uint8 ct0, ct1;
4
   uint8 i;
5
 
6
   SRTISC_RTIACK= 1;
7
   
8
   ISR_counter++;
9
    if(ISR_counter == 4) {
10
       ISR_counter = 0;
11

12
/*
13
      i = key_state ^ ~KEY_PIN;                       // key changed ?
14
      ct0 = ~( ct0 & i );                             // reset or count ct0
15
      ct1 = ct0 ^ (ct1 & i);                          // reset or count ct1
16
      i &= ct0 & ct1;                                 // count until roll over ?
17
      key_state ^= i;                                 // then toggle debounced state
18
      key_press |= key_state & i; 
19

20
      status_bcd = BCD_Verarbeiten(BCD_Auslesen());
21
      //Jetzt kann ich den Status des Schalters abfragen
22
*/
23
   }  
24
 }

Wenn immer noch kein Fehler, dann nimmst du den Teil vor der BCD 
Auswertung mit dazu.

Solange bis dass du einen Anhaltspunkt hast, in welchem Bereich sich das 
Problem abspielt.
#3253391
Lesenswert?

Jan R. schrieb:
> Ja das mag schlecht oder unsystematisch programmiert sein, aber der
> Compiler meckert nicht.

Schlechtes Argument.
Der Duden hat gegen
 "Das Flugzeug taucht die Gabel mit der Fahne."
auch nichts einzuwenden. Alle Wörter, alle Wortstellungen sind korrekt, 
alle Grammatikregeln wurden eingehalten.
Und trotzdem ist der Satz Unsinn.


Wenn der Compiler nichts beanstandet, dann heißt das erst mal nur, dass 
die Grammatik stimmt. Mehr nicht.
OP #3253393
Lesenswert?

So habe alles auskommentiert in der ISR und der Fehler kam immer noch.
1
void PeriphInit(void) {
2

3
//Global Interrupts aktivieren
4
EnableInterrupts;
5

6
//PORT G für BCP und DIL Schalter auf Eingang stellen
7
PTGDD = 0x00;
8
//PORT D für LEDS auf Ausgang stellen
9
PTADD = 0xff;
10

11
//Timer initialisieren
12
SRTISC_RTIACK= 1;          //Alte Interrupts clearen
13
SRTISC_RTIE = 1;           //Timer-Interrupt enable
14
SRTISC_RTIS = 0x001b;      //Interrupt alle 8 ms
15
SRTISC_RTICLKS = 0;        //Interner 1Khz Takt wird für Timer Interrupt benutzt
16

17
}

Dann kann der Fehler fast nur noch in der Initialisierung liegen.

Ich habe auch versucht einen Breakpoint auf die ISR zu setzen, 
allerdings wurde dieser nie erreicht...
#3253403
Lesenswert?

Jan R. schrieb:
> So habe alles auskommentiert in der ISR und der Fehler kam immer noch.

Dann heißt es eben weiterschauen.
Die Technik dazu kennst du ja jetzt.


Nur der Vollständigkeit halber

> void PeriphInit(void) {
>
> //Global Interrupts aktivieren
> EnableInterrupts;
>
> ....
> }

Die Reihenfolge ist:
ERST konfiguriert man alle Bauteile und Komponenten und DANN gibt man 
die Interrupts generell frei. Nicht umgekehrt. Sonst kann es dir 
theoretisch bei mehreren Interrupt Quellen passieren, dass der erste 
Interrupt schon kommt, während das Programm die weiteren Komponenten 
noch gar nicht fertig konfiguriert hat.
OP #3253413
Lesenswert?

Hier der komplette Code:

main.c
1
#include <hidef.h> /* for EnableInterrupts macro */
2
#include "derivative.h" /* include peripheral declarations */
3
#include "MCUInit.h"     //Interrupts, IO-Ausgänge, SPI
4
#include "IOVerarbeitung.h"   //BCD-Schalter, DIL-Schalter, LEDS, SPI
5

6
#define uint8 unsigned char
7

8
uint8 key_state;  // debounced and inverted key state:
9
          
10
uint8 key_press;  // bit = 1: key pressed
11

12
volatile bool status_bcd;
13
volatile bool status_dil;
14
volatile uint8 ISR_counter;
15

16
void main(void)
17
{
18
  PeriphInit();
19
  for(;;) {
20
  __RESET_WATCHDOG();
21
    
22
    if(status_bcd == true) {
23
     status_bcd = 0;
24
     //Sende_SPI(framerate);
25
    } else{
26
     //Der BCD-Schalter wurde nicht verändert 
27
    }
28
  }
29
 }
30
 
31
 __interrupt VectorNumber_Vtpm2ovf void Timer_ISR() {
32

33
   static uint8 ct0, ct1;
34
   uint8 i;
35
 
36
   SRTISC_RTIACK= 1;   
37
   
38
    /*
39
    
40
   ISR_counter++;
41
    if(ISR_counter == 4) {
42
       ISR_counter = 0;
43
     
44

45
      i = key_state ^ ~KEY_PIN;                       // key changed ?
46
      ct0 = ~( ct0 & i );                             // reset or count ct0
47
      ct1 = ct0 ^ (ct1 & i);                          // reset or count ct1
48
      i &= ct0 & ct1;                                 // count until roll over ?
49
      key_state ^= i;                                 // then toggle debounced state
50
      key_press |= key_state & i; 
51
   
52
      status_bcd = BCD_Verarbeiten(BCD_Auslesen());
53
      //Jetzt kann ich den Status des Schalters abfragen
54

55
   } 
56
   */
57
 }
MCUInit.h
1
void PeriphInit(void) {
2

3
//Global Interrupts aktivieren
4
EnableInterrupts;
5

6
//PORT G für BCP und DIL Schalter auf Eingang stellen
7
PTGDD = 0x00;
8
//PORT D für LEDS auf Ausgang stellen
9
PTADD = 0xff;
10

11
//Timer initialisieren
12
SRTISC_RTIACK= 1;          //Alte Interrupts clearen
13
SRTISC_RTIE = 1;           //Timer-Interrupt enable
14
SRTISC_RTIS = 0x001b;      //Interrupt alle 8 ms
15
SRTISC_RTICLKS = 0;        //Interner 1Khz Takt wird für Timer Interrupt benutzt
16
}

IOVerarbeitung.h
1
typedef int bool;
2
  /*!< Definition for boolean type. */
3
#ifdef __MISRA__ /* MISRA rule #18 */
4
  #define true  1u
5
    /*!< Definitioni for TRUE. */
6
  #define false 0u
7
    /*!< Definition for FALSE. */
8
#else
9
  #define true  1
10
    /*!< Definitioni for TRUE. */
11
  #define false 0
12
    /*!< Definition for FALSE. */
13
#endif
14

15
#define uint8 unsigned char
16
#define KEY_PIN    PTGD_PTGD;
17

18

19
byte buffer; //Temporäre Veriable für Überprüfung der Schalter
20

21

22
uint8 lastresult_bcd;      //Letzter Wert des BCD-Schalters
23
uint8 lastresult_dil;      //Letzter Wert des DIL-Schalters
24
float framerate;           //Aktuelle Framerate
25

26

27
bool BCD_Verarbeiten(uint8);
28
uint8 BCD_Auslesen(void);
29
void DIL_Verarbeiten(uint8);
30
uint8 DIL_Auslesen(void);
31

32
 
33
//Wert von BCD-Schalter auslesen und in eine Dezimalzahl konvertieren 
34
uint8 BCD_Auslesen(void) {
35
  buffer = PTGD;
36
  buffer = (buffer & 0xf); 
37
  return ((((buffer & 0xF0) >> 4) * 10) + (buffer & 0x0F));   //Rückgabewert: BCD-Schalterzahl in dezimaler Form
38
}
39

40
//Wert von BCD-Schalter weiterverarbeiten => entsprechende Framerate setzen
41
bool BCD_Verarbeiten(uint8 i) {
42

43
  if(lastresult_bcd == i) {
44
   return false;
45
  } 
46
  else{
47
        if(i == 0) {
48
          framerate = 23.97;
49
        } 
50
      }  
51
      lastresult_bcd = i;
52
      return true;
53
}

Dann gibt es noch vorgefertigte Includes:
derivative.h
MC9S08AW60.h
OP #3253414
Lesenswert?

> Die Reihenfolge ist:
> ERST konfiguriert man alle Bauteile und Komponenten und DANN gibt man
> die Interrupts generell frei. Nicht umgekehrt. Sonst kann es dir
> theoretisch bei mehreren Interrupt Quellen passieren, dass der erste
> Interrupt schon kommt, während das Programm die weiteren Komponenten
> noch gar nicht fertig konfiguriert hat.

Ok, gut vielen Dank!

Ist denn ein genereller logischer fehler zu sehen.
Ist z.B. die PeDa-Lösung richtig eingebunden, auch wenn der Fehler 
auskommentiert kommt.

Es wird dann am Timer liegen, ich werde es jetzt mal schrittweise 
durchgehen.
Gast #3253418
Lesenswert?

Jan R. schrieb:
> buffer = PTGD;
>   buffer = (buffer & 0xf);
>   return ((((buffer & 0xF0) >> 4) * 10) + (buffer & 0x0F));
> //Rückgabewert: BCD-Schalterzahl in dezimaler Form

funktioniert das denn?

wenn du ein

buffer = (buffer & 0xf);

machst, dann macht es für mich wenig sinn dann später ein

(buffer & 0xF0)

zu machen.

(wenn ich mich irre, dann wegen der wärme ...)
OP #3253433
Lesenswert?

Fehler erstmal gefunden... falsche Vectornumber benutzt KopfAnWand

Jetzt muss ich logische Fehler finden.
Den Entprellcode habe ich nur eingebaut ohne mich damit näher zu 
beschäftigen. Da schaue ich jetzt mal als erstes.

Aber allen vielen Dank für die fleiíge Unterstützung hat sich gelohnt.
Gast #3253917
Lesenswert?

Rufus Τ. Firefly schrieb:
> Den üblen Pfusch mit Code und Variablendefinitionen in Headerdateien
> solltest Du Dir allerdings so schnell wie irgend möglich abgewöhnen --
> und zwar noch bevor Du irgendwas anderes machst.

Dem kann ich nur beipflichten.
In eine Headerdatei gehört nichts, was Speicher braucht.
Deklarationen, aber niemals Definitionen.
OP #3254091
Lesenswert?

Ich bin Ehrenvorsitzender dieses Vereins auf Lebenszeiten. Wir haben uns 
das Ziel gesetzt nie schönen Quellcode zu schreiben. Doch jetzt haben 
wir einen Abtrünnigen, der doch mal schönen Code schreiben wollte, habt 
ihr für ihn Tipps?
Ich schaue erstmal in der Sammlung, ob ich etwas finde....
#3254135
Lesenswert?

http://de.wikipedia.org/wiki/Modul_(Software)

Grundsätzlich:

Ein .c Datei und eine. h Datei bilden ein sog. MODUL. In der der Header 
Datei wird das Interface definiert. Auch die Datenstrukturen. In der .c 
Datei wird dieses Interface implementiert.
Um das Modul zu nutzen wird nur die .h Datei inkludiert.

Ein Modul sollte eine wohldefinierte Aufgabe haben. Klar abgegrenzt zum 
Rest.

Es bietet sich z.B. an, bestimmte Peripherie Bausteine in eigene Module 
auszulagern. Ein Modul kapselt dann die gesamte Funktionalität des 
Bausteins.

Oder bestimmte Algorithmen, wie Sortierung oder Datenstrukturen mit 
Operationen drauf (Bäume, Listen usw.).

Ob man nun IOs und Init in eigene module machen sollte, würde ich 
bezweifeln.

Module sollten über Sachverhalte und Komplexität abstrahieren, und die 
Komplexität darunter, für den nutzenden Code, verdecken.

gruß cyblord
Gast #3254147
Lesenswert?

0 - byte buffer;
1 - buffer = PTGD;
2 - buffer = (buffer & 0xf);
3 - return ((((buffer & 0xF0) >> 4) * 10) + (buffer & 0x0F));

1) buffer = ????????;
2) buffer = ???????? & 00001111 = 0000????;
3) buffer = 0000???? & 11110000 = 0 so war es zumindest früher einmal.
OP #3254163
Lesenswert?

1 - buffer = PTGD;
2 - buffer = (buffer & 0xf);

Hier wird der Wert von PTGD eingelesen.
Egal was vorher dort drinstand, ich brauche nur die vier unteren Bits.
Egal was in den oberen drinstand, die sind dann 0. Und die unteren 
behalten ihren normalen Wert.
Ja ok du hast aber Recht.

(((buffer & 0xF0) >> 4) * 10)

Ergibt dann eigentlich keinen Sinn, es funktioniert aber wie es soll.
Ich könnte dies quasi löschen. Scheinbar kann der Compiler automatisch 
von byte in uint8 konvertieren.


@cyblord
Wenn ich diese Sachen nicht auslagern würde, dann wäre die main.c aber 
so voll.
#3254212
Lesenswert?

Jan R. schrieb:
> Kannst du mir mal eine kurze sinnvolle Gliederung hierfür erstellen?
> Wäre super nett.
> Habe es einfach noch nie gemacht.

Programmieren musst du schon selber, Übung macht den Meister.

Deine IO-Verarbeitung ist ja eigentlich das Einlesen deiner 
BCD-Schalter. Die kannst du schon kapseln, mit Header Datei (dort denkst 
du dir ein praktisches Interface aus) und .c Datei wo du das 
ausprogrammierst.

Und das Verarbeiten der BCD-Daten, gehört da nicht rein. Das Modul hat 
die Aufgabe, die BCD-Stellung aus den Portpins abzufragen und ordentlich 
als Zahl wiederzugeben. Nicht mehr und nicht weniger.

Trotzdem ist deine main jetzt nicht so lang dass man das nicht auch 
einfach direkt als eine Funktion in die main.c schreiben könnte. Aber 
wenn du es auslagern willst, dann eben so.

gruß cyblord
#3254215
Lesenswert?

Normalerweise hast Du zu jeder Header-Datei (.h) eine Codedatei (.c). 
Diese beiden Dateien bilden ein Modul und sollten auch als gekapselt 
programmiert werden.

Du greifst von der main.c direkt auf Variablen in einem Modul zu. So 
etwas ist schlechter Stil und fehleranfällig (Seiteneffekte).

Ein gutes Buch (wenn nicht das C-Buch ist von Kernighan-Ritchie):
http://www.amazon.de/Programmieren-C-Reference-Manual-deutscher-Sprache/dp/3446154973/ref=sr_1_1/276-8358910-2845704?s=books&ie=UTF8&qid=1374741104&sr=1-1&keywords=3446154973

Dort steht alles drin.
#3254220
Lesenswert?

Pete K. schrieb:
> Normalerweise hast Du zu jeder Header-Datei (.h) eine Codedatei (.c).
> Diese beiden Dateien bilden ein Modul und sollten auch als gekapselt
> programmiert werden.

Es wurde schon alles gesagt, nur noch nicht von jedem?

> Ein .c Datei und eine. h Datei bilden ein sog. MODUL. In der der Header
> Datei wird das Interface definiert. Auch die Datenstrukturen. In der .c
> Datei wird dieses Interface implementiert.

> Seiteneffekte
Ohoh damit rufst du die Sprach-Nazis auf den Plan. side effects = 
Nebeneffekte. Aber sag ich auch nicht ;-)
Gast #3254334
Lesenswert?

Ich erinnere mich noch gut daran:

mein erstes C-Programmierbuch (>25 Jahre her)
Keine Erfahrung mit Programmieren mit Bibliotheken.
Und dann kam der Satz:
"Benutzen Sie diese Funktion aus der Bücherei"

Super!
Dann doch lieber in Englisch....
Gast #3254344
Lesenswert?

cyblord ---- schrieb:
>> Ja, wobei ein Buch in der richtigen Sprache schon ganz schön wäre...
>
> Sprache und Konzept sollte man trennen können. Sonst wird man nie gut.

dumm nur das Java OOP ist und C nicht. C hat dafür auch noch 
Headerdateien. Man kann halt nicht alles vergleichen. Es gibt konzepte 
die man in C anders umsetzt als bei java.
OP #3254347
Lesenswert?

"Benutzen Sie diese Funktion aus der Bücherei"
ja das sind die Klassiker....
Da wird man mehr verwirrt, als das einem diese Übersetzung hilft.
Ja nur ist das C Buch in Englisch noch zeitgemäß, ist ja schon fast 20 
Jahre alt. Nicht, dass der dann noch ältere englische Wörter benutzt...
#3254351
Lesenswert?

Peter II schrieb:
> cyblord ---- schrieb:
>>> Ja, wobei ein Buch in der richtigen Sprache schon ganz schön wäre...
>>
>> Sprache und Konzept sollte man trennen können. Sonst wird man nie gut.
>
> dumm nur das Java OOP ist und C nicht. C hat dafür auch noch
> Headerdateien. Man kann halt nicht alles vergleichen. Es gibt konzepte
> die man in C anders umsetzt als bei java.

Ja sicher gibt es da Unterschiede. Das Buch ist auch keine Anleitung für 
gutes OOP, sondern für guten Code.
Es gibt leider nichts vergleichbares für C, darum muss man halt über das 
Java hinwegsehen. Es ist auch kein Anfängerbuch, welches einem die 
Sprachen näher bringen soll.
Hast du es mal gelesen? Danach diskutiere ich mit dir gerne weiter 
darüber.
Also back to Topic!

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren