Moin Moin,
Auch nach lesen von diversen Threads werde ich nicht gnaz schlau daraus
ob ich mich darauf verlassen kann, das ein Element in einem Struct oder
Elemente eines Array wirklich volatile sind wenn ich diese
folgendermaßen definiere:
MCU: AT90CAN128
gcc version 4.3.3 (WinAVR 20100110)
Optimizer: -Os
1
//variant a
2
volatileuint8_tactive=0;
3
4
// variant b
5
typedefstruct{
6
volatileuint8_tactive;
7
can_buffer_tbuffer;
8
can_tlist[CAN_TX_BUFFER_SIZE];
9
}txbuf_t;
10
11
//variant c
12
volatileactive[SJA_MAX];
13
14
txbux_ttxbuf[SJA_MAX];
15
16
17
// later in code
18
ISR(INT0_vect){
19
20
...
21
22
// This is working
23
active=0;
24
25
// This two variant´s do not work
26
active[0]=0;
27
txbuf.active[0]=0;
28
....
29
}
Sobald ich die Variable "active" als Element eines Array oder einer
Struktur nutze funktioniert anscheinend die Deklaration volatile nicht,
da in beiden Fällen der code nicht mehr funktioniert.Natürlich könnte es
auch nur ein "seiteneffekt" sein. Ich möchte mir aber wirklich sicher
sein, dass die deklaration und der Zugriff 100% richtig sind bevor ich
anfange wilder fiese Geister zu suchen.
Wo liegt hier der Haken in der Deklaration oder Nutzung der Variablen?
Arrrrghhhh!
Ich habe den code abgeschrieben weil es sonst zu viel wäre. Tippfuhler!
Mea maxima culpa!
Richtig ist natürlich:
1
//variant c
2
volatileuin8_tarray=0;
3
4
5
// This two variant´s do not work
6
active[0]=0;
7
txbuf[0].active=0;
Compilieren tut es ( wenn man den originalen code nutzt, aua!). die
Funktion die ich mit der variablen "active" steuere funktionier jedoch
nur, wenn ich die variante "c" nutze. Sie zeigt an, ob ein
CAN-Controller ( SJA1000 ) noch gerade dabei ist die CAN nachricht zu
senden und somit weitere Daten in einen Buffer zwischengespeichert
werden müssen.
Danke für´s aufmerksame lesen.
Gruß
Karsten
OK, jetzt sieht es besser aus.
Meine Erfahrungen mit Variante b waren bis jetzt ganz gut.
Zeigst Du uns auch noch die andere Stelle, wo Du den Wert liest?
Hallo,
Damit nun nicht noch einmal Karstens Krummfinger dazwischen kommen der
ganze code für den SJA1000 im Anhang.
Es sind zwei SJA1000 anzusteuern, die jeweils mit einem TX buffer
ausgestattet sind und diesen über interrupt "selbst" abarbeiten. Dabei
wird aber nur eine sende Funktion genutzt die als Parameter den
entsprechenden SJA übergeben bekommt der die message versenden soll.
Alles nicht ganz einfach, die Funktion _send_message() reentrant zu
halten...
Gruß
Karsten
Ob Member oder separate Variable ist bei "volatile" egal. Nur ist die
Programmlogik hier nicht identisch, denn in einem Fall ist es eine
Variable für alle, im anderen Fall eine pro Puffer.
Hallo,
Damit die Funktion sja_send_message() für beide SJA´s funktioniert muss
natürlich die Variable "active" abhängig vom Übergabewert ( x ) pro
buffer gesetzt werden. txbuf[x].active = Y
Weil das aber nicht funktioniert, habe ich für den einen SJA eine feste
Variable ausprobiert ( volatile uint8_t active ). Im Code ist der
ursprüngliche Versuch auskommentiert.
Weil es mit der direckten volatile variable active funktioniert, gehe
ich davon aus, dass ich etwas falsch mache wenn ich ein volatiles
Element aus der struct txbuf_t nutze. Zum Test hatte ich auch einfach
mal nur ein arra versucht wie oben beschrieben. Aber auch dort
funktioniert der code nicht mehr.
Jetzt könnte ich zwei "active" variablen nutzen und mit "wilden if()"
testen ob nun für das setzten und lesen die Eine oder Andere nutzen
muss. Das ist aber wieder alles nicht atomic und macht dann sicher
Probleme.
gruß
Karsten
Karsten K. schrieb:> Hallo,>> Damit die Funktion sja_send_message() für beide SJA´s funktioniert muss> natürlich die Variable "active" abhängig vom Übergabewert ( x ) pro> buffer gesetzt werden. txbuf[x].active = Y
Jo, grundsätzlich richtig
> Alles nicht ganz einfach, die Funktion _send_message() reentrant zu halten...
Teil deines 'alles nicht ganz einfach' dürfte darin liegen, dass du eine
globale sja_add Variable hast, die angibt auf welchen SJA die Low-Level
Funktion operieren sollen.
Hättest du dir die Variable gespart und statt dessen den Low-Level
Funktionen ein zusätliches Argument gegeben, welches ihnen jeweil
angibt, welcher SJA gemeint ist, dann hätte sich dein 'alles nicht so
einfach' in Luft aufgelöst. Solche 'Umschaltvariablen' inklusive
Sichern/Rücksetzen an allen möglichen und unmöglichen Codestellen sieht
nur auf den ersten Blick einfach aus. Tatsächlich hat so ein Vorgehen
ungemeines Sprengpotential. Hat sich in der Vergangenheit oft genug
bewiesen. Übergib einer Funktion alle Parameter, die sie zur Arbeit
benötigt - das kostet dich einmalig ein paar Minuten um die vorhandenen
Funktionen (die mit nur einem Gerät arbeiten) entsprechend aufzubohren
aber auf lange Sicht ist das die wartungstechnisch vernünftigste Lösung.
> Weil es mit der direckten volatile variable active funktioniert, gehe> ich davon aus, dass ich etwas falsch mache wenn ich ein volatiles> Element aus der struct txbuf_t nutze.
Ich hab ihn (den Fehler) zwar noch nicht entdeckt, ich denke aber, dass
den sja_add irgendwo den falschen Wert hat.
PS: Bei Code in diesen Größenordnungen ist ein vergessenes volatile
'nicht so schlimm'. Natürlich ist es nach wie vor ein Fehler, aber
alleine durch die Codegröße und den Datenumfang schafft es der Compiler
sowieso nicht, die entsprechendn Variablen ständig in Registern zu
halten, so dass sich ein fälschlicherweise vergessenes volatile nicht
auswirkt, weil der Compiler sowieso keine andere Wahl hat, solange das
ganze nicht in Schleifen vorkommt.
Hallo Karl Heinz,
Ja, du hast recht! Ich war(bin) auch nicht so sehr mit sja_add
zufrieden, hatte aber gescheut die sja_write() und sja_read() umzubauen.
O.K. ich werde deinen Vorschlag beherzigen und das ganze ohne globales
Schalten realisieren. Das wird dann hoffentlich betriebssicherer.
Das Problem mit der Variablen "active" wird aber bestehen bleiben. Es
sei denn, der Fehler liegt tatsächlich am globalen sja_add.
By the Way:
Von der "Schönheit" her würde ich die Makros
ENTER_CRITICAL_SECTION und LEAVE_CRIRICAL_SECTION für die Funktion
_send_message() gern innerhalb der Funktion definieren.
Mir ist nur nicht klar, ob dies negative Auswirkungen hat.
1
uint8_t
2
_send_message(constcan_t*msg){
3
4
ENTER_CRITICAL_SECTION
5
...
6
LEAVE_CRIRICAL_SECTION
7
}
8
9
// same as ??????
10
LEAVE_CRIRICAL_SECTION
11
_send_message(&msg);
12
LEAVE_CRIRICAL_SECTION
Herzlichen Dank für deine Einschätzung und deinen Tipp!
Gruß
Karsten
Karsten K. schrieb:> Ja, du hast recht! Ich war(bin) auch nicht so sehr mit sja_add> zufrieden, hatte aber gescheut die sja_write() und sja_read() umzubauen.
Das sind 2 Funktionen, denen du ein zusätzlicxhes Argument gibst.
Dann jagst du den Compiler drüber und der sucht dir alle Aufrufe (in
Form von Fehlermeldungen) raus, an denen beim Aufruf dieses Argument
ergänzt werden muss. Es wird Fälle geben, in denen der Aufruf in einer
Funktion steckt, die dann selbst wieder ein zusätzliches Argument
bekommt und es einfach nur weitergibt.
Alles in allem ist der Umbau (beim gezeigten Code) in 10 Minuten
abgeschlossen. Alles kein Problem.
> Schalten realisieren. Das wird dann hoffentlich betriebssicherer.> Das Problem mit der Variablen "active" wird aber bestehen bleiben.
Ich denke, dass das ein Nebensachauplatz ist.
Dein 'funktioniert nicht' ist nicht unbedingt auf diese active Variable
zurückzuführen. Wie schon gesagt: du hast (im gezeigten Code) keine
Warteschleifen, die auf eine Veränderung dieser Variablen warten würden.
Durch die Codegröße und die Anzahl der Variablen sowie die Aufteilung in
Funktionen hat der Compiler sowieso keine Wahl - er kann diese Active
Variable nicht die ganze Zeit in einem Register halten.
// SJA ist frei. die msg kann direkt abgesetzt werden
24
_send_message(msg,sja);
25
}
26
LEAVE_CRITICAL_SECTION;
27
28
return(2);
29
}
PS: jetzt seh ich hier auch, was mich noch stört.
Der Member 'buffer' hat einen schlechten Namen. Das ist offenbar ein
Member, den die Queue Routinen benötigen, und in denen die sich
INformation darüber ablegen, welche Einträge in
1
can_tlist[CAN_TX_BUFFER_SIZE];
benutzt sind und welche nicht.
Also eine Control-Variable.
Dann würde ich die Member so benennen
1
typedefstruct{
2
3
volatileuint8_tactive;// Flag set if transmission is in progress
4
can_buffer_tQueueControl;
5
can_tQueueBuffer[CAN_TX_BUFFER_SIZE];
6
7
}txbuf_t;
das macht dann den Code beim reinen Lesen logischer
ich finde diese Version leichter zu lesen, weil man nicht dauernd um die
Ecke denken muss. Mit dem Begriff 'buffer' verbindet man ja intuitiv
etwas - nämlich ein Array in dem etwas gespeichert wird. Nur ist hier
der 'buffer' eben kein Buffer in diesem Sinne, sondern einfach nur ein
Hilfselement der Queue Routinen.
-> man kann durch geschickte Wahl von Variablennamen sich das Leben
leichter oder schwerer machen.
Das hier
1
can_buffer_enqueue(&txbuf[sja].QueueControl);
sieht auch eigenartig aus, aber ok.
Das Prinzip ist mir, denk ich klar.
Zuerst besorgt man sich mit
can_buffer_get_enqueue_ptr
einen Pointer auf ein Queue Element, das man benutzen kann.
can_buffer_enqueue 'aktiviert' dann dieses Element in der Queue. Das
geht deswegen, weil sich die Queue Routinen irgendwo merken, welchen
Pointer sie als letztes beim get_ptr rausgerückt haben.
Finde ich nicht gut gemacht. Da ist schon wieder eine Race Condition
drinnen. Zwischen der get_ptr Funktion und enqueue darf kein Interrupt
auftreten, der seinerseits etwas in diese Queue zu stellen versucht.
Solche implizten Zusammenhänge, quer über Funktionsaufrufe, sind
gefährlich! Da ist schon so mancher reingefallen. Es ist verführereisch
zb in der Programmierung einer linearen Liste ein sog. activeElement in
die Datenstruktur mit aufzunehmen. Auf lange Sicht gesehen ist das aber
parktisch immer ein 'pain in the ass'.
Hallo KarlHeinz,
Eine menge Info die ich ersteinmal nachvollziehen muss. Den Umbau habe
ich tatsächich in 10 Minuten geschafft, herzlichen Dank.
Weiters sehe ich mir jetzt an ( war zwischendurch beim Bäcker :-). Im
Augenblick kracht das ganze sogar ab und zu mal. In sofern, dass der
AT90CAN128 einen Reset ausführt! Die Resetleitung ist´s nicht; da hatte
ich schon mal den reset-IC aus dem OLIMEX Testboard erwischt als der
sporatisch 500ns lang Reset erzeugt hat!
O.K. Schritt für Schritt nach dem alten Cäsar: Teile und herrsche!
Ich melde mich gern zurück.
Gruß
Karsten
Hallo, nochmal ich :-)
Ich habe gerade beschlossen die Steckbrettvariante komplett über den
haufen zu schmeissen; irgendwie muss ich mich zumindest auf eine
variable in diesem projekt verlassen können ( Wackelkontackte, lange
leitungen usw. ). Unberechenbare Resets sind ja kein Spass mehr.
Also ein kleines reboot :-(
Zu den Buffern:
Nun, index geführte Buffer listen sind vielleicht nicht der letzte
schrei, aber auch eine (doppelt) gelinkte Liste muss beim Zugriff
"gesichert" werden. Der verwendete Code ist in allen entscheidenen durch
ATOMIC_BLOCK makros gesichert.
Falls aber eine benutzbare Bufferwverwaltung für die kleinen AVR´s zur
"Hand" ist und ich diese nutzen darf, wäre ich höchst begeistert diese
zu bekommen.
Herzlichsten Dank nochmals für die geniale Hilfe und den Anstoß nochmals
am Grunde zu beginnen. Am Ende ist es dann doch einfacher als ewiges
rungefrickel mit labilem Code.
Gruß
Karsten