Hallo,
jetzt suche ich schon mehrere Stunden nach einem hartnäckigen Bug in
meiner Entprellungsroutine und fast habe ich den Eindruck, der Compiler
baut hier Mist.
Folgende Situation:
- Günstiges Board mit STM32F103RBT
- Neueste SystemWorkbench mit StdPeriph_Driver
- Ein Taster (später werden es vier sein) zum Entprellen
- IRMP läuft mit 15 KHz in einer eigenen Timer-IRQ
Die Takte stimmen, Pins sind alle richtig initialisiert, der Systemtimer
ruft mit hoher Priorität (höher als IRMP) den Entpreller alle 10 ms auf:
1
volatileuint16_tkey_state=0;// debounced and inverted key state:
2
// bit = 1: key pressed
3
volatileuint16_tkey_press=0;// key press detect
4
5
volatileuint16_tkey_rpt=0;// key long press and repeat
6
7
voidKeyboard_IRQHandler(void)
8
{
9
// Timer Interrupt handler for TIM4 will be called every 10ms
10
staticuint16_tct0=0xffff,ct1=0xffff,rpt;
11
// KEY-PIN MACRO will be called to read in key button pressed events
12
uint16_ti;
13
14
i=key_state^~KEY_PIN;// key changed ?
15
ct0=~(ct0&i);// reset or count ct0
16
ct1=ct0^(ct1&i);// reset or count ct1
17
i&=ct0&ct1;// count until roll over ?
18
key_state^=i;// then toggle debounced state
19
key_press|=key_state&i;// 0->1: key press detect
20
21
intc=key_press;
22
23
if((key_state&REPEAT_MASK)==0)// check repeat function
24
rpt=REPEAT_START;// start delay
25
if(--rpt==0)
26
{
27
rpt=REPEAT_NEXT;// repeat delay
28
key_rpt|=key_state&REPEAT_MASK;
29
}
30
}
Man beachte die eigentlich total überflüssige Zeile
1
intc=key_press;
mit dieser Zeile funktioniert die Entprellung einwandfrei, ohne
diese Zeile reagiert die Funktion oft gar nicht oder nicht richtig auf
Tastendrücke.
Eigentlich ist "key_press" doch als volatile deklariert und sollte
dementsprechend von etwaigen Compileroptimierungen verschont bleiben.
Ich blick's gerade nicht, denn diese kleine Zeile entscheidet eindeutig
zwischen Funktion oder Nichtfunktion.
Hat da wer 'ne Erklärung parat?
Schöne Grüße,
Oliver
Oliver R. schrieb:>> Hat da wer 'ne Erklärung parat?>> Schöne Grüße,> Oliver
vielleicht, wenn Du mal mit -E compilieren würdest und beide Versionen
(mit und ohne der "überflüssigen Deklaration") als Assembler output mal
vergleichen und reinstellen würdest... über welche Compilerversion reden
wir hier?...
... Und mach doch gleich deine Variablen 32 bittig.
Beim ARM bezahlst die Optimierung nach Größe mit Geschwindigkeit.
Es werden zusätzliche Befehle eingeschoben.
Und bist du dir ganz sicher, dass ct1 hier auch static ist?
Ich würde die definition der 3 inneren Variablen in 3 zeilen machen.
Adib.
Oliver R. schrieb:> jetzt suche ich schon mehrere Stunden nach einem hartnäckigen Bug in> meiner Entprellungsroutine und fast habe ich den Eindruck, der Compiler> baut hier Mist.
Nimm einen einfachen Spiegel zum Suchen nach dem Bug.
Also mal ganz ernst:
Du hast diese Entprellroutinen unreflektiert und wohl unverstanden
kopiert.
Und du hast einen grauseligen Schreibstil.
Wenn ich deinen Quelltext lese, frag ich mich, ob wenigstens du ihn
selbst verstehst.
Bei sowas: "volatile uint16_t key_state = 0;" frage ich mich, was es
denn sein soll: definiert vorbelegt oder volatile. Lauter
selbstgebaute Fallstricke.
Eigentlich ist es doch so einfach:
wir haben folgende Zustände:
1. Taste ungedrückt und nicht beim Entprellen.
--> nix zu tun
2. Taste ungedrückt und beim Entprellen.
--> Repetierzähler auf lange Repetierzeit setzen und Entprellzähler
herunterzählen. Wenn Null, dann Zustand=1
3. Taste gedrückt und nicht beim Entprellen.
--> Aha, Tastenereignis gefunden. Also melden.
Dann Entprellzähler laden und eine lange Repetierzeit auswählen
4. Taste gedrückt und beim Entprellen.
--> Entprellzähler wieder laden, Repetierzähler herunterzählen.
Wenn Repetierzähler auf null, dann Tastenereignis melden, eine kürzere
Repetierzeit auswählen.
Sowas solltest du ja wohl ohne Probleme in C Quelle umsetzen können.
W.S.
W.S. schrieb:> Du hast diese Entprellroutinen unreflektiert und wohl unverstanden> kopiert.
Keine Ahnung ob der TO das kapiert hat, allerdings habe ich eher das
Gefühl, dass dies bei dir der Fall ist. Der Code von Peter ist nicht
kompliziert, wird allerdings auf Bit-Ebene ausgeführt und seltsamerweise
ähnelt er stark deiner Entprellbeschreibung.
Der Urspungscode ist mit sehr hoher Wahrscheinlichkeit fehlerfrei. Ich
selbst habe ihn schon mehrfach eingesetzt. Hier auf eine andere Lösung
zu verweisen ist einfach daneben.
@TO
Sinnvoll wären tatsächlich die beiden Assemblerlistings. Ansonsten kann
man auch mit dem Debugger durch die ISR steppen und nachvollziehen, an
welcher Stelle ein Fehler auftritt. Notfalls im Intrukion Stepping
Modus.
W.S. schrieb:> Nimm einen einfachen Spiegel zum Suchen nach dem Bug.>> Also mal ganz ernst:> Du hast diese Entprellroutinen unreflektiert und wohl unverstanden> kopiert.
Ich würde mit solchen Unterstellungen aber ganz vorsichtig sein. Oder
willst Du damit sagen, daß Du ihn nicht kapiert hast und deswegen auch
andere ihn nicht kapieren?
Außerdem ist nicht der Code das Problem, sondern vermutlich ein
Compiler-Bug. Sonst würde ein Write-Only einer Variablen das Verhalten
ja nicht ändern.
W.S. schrieb:> Bei sowas: "volatile uint16_t key_state = 0;" frage ich mich, was es> denn sein soll: definiert vorbelegt oder volatile.
Natürlich beides, was denn sonst.
Die Vorbelegung mit 0 muß man nicht extra hinschreiben, ist aber für das
Verständnis klarer, daher guter Schreibstil. Der Code ist der gleiche.
Das volatile habe ich früher weggelassen und es ging. Die neueren
Compilerversionen machen aber recht agressives Inlining und dann kann es
notwendig sein.
Hallo,
erst einmal danke für die Anregungen (den Unsinn von W.S. mal
ausgenommen). Ich werde mal die Assemblerlistings vergleichen, dort
müsste man in der Tat was sehen können.
Und wie gesagt: Mit dieser Zeile funktioniert die Entprellung wie
gewünscht, Peters Code funktioniert auf jeden Fall!
Gruß,
Oliver
So, ich habe mal die Assemblerlistings der Stelle verglichen. Links ist
die Zeile einkompiliert, rechts nicht.
Leider kenne ich mich mit ARM-ASM nicht gut aus...
Achja, Version der tools:
System Workbench for STM32 - C/C++ Embedded Development Tools for MCU
Version: 1.14.0.201703061529
Das scheint die neueste Version zu sein.
Gruß,
Oliver
Peter D. schrieb:> Ich würde mit solchen Unterstellungen aber ganz vorsichtig sein
OK, dann muß ich dir den Vorwurf machen, daß du eine verbuggte Quelle
geliefert hast. Möchtest du auch nicht gern hören, ja?
Ich sag's mal so: Du hast da einige recht artistische Kunststückchen
geliefert, die auf einem AVR sicherlich sehr schön funktionieren, die
aber genau dann mit größter Vorsicht zu genießen sind, wenn man selbiges
auf einer ganz anderen Architektur verwenden will.
Und jetzt fängst du nach dem TO auch noch an, von Compiler-Bugs zu
orakeln. Ach nö, sowas riecht mir gar zu sehr nach Ausrede.
Die generelle Abhilfe wäre, das Ganze einfach in der Art biederer
Hausmannskost neu zu schreiben. Wetten, daß dann der ominöse
Compiler-Bug verschwunden ist?
W.S.
Oliver R. schrieb:> So, ich habe mal die Assemblerlistings der Stelle verglichen. Links ist> die Zeile einkompiliert, rechts nicht.> Leider kenne ich mich mit ARM-ASM nicht gut aus...>> Achja, Version der tools:>> System Workbench for STM32 - C/C++ Embedded Development Tools for MCU>> Version: 1.14.0.201703061529>> Das scheint die neueste Version zu sein.>> Gruß,> Oliver
Die ganzen Befehle mit X im Namen sind der Tatsache geschuldet, daß du
6Byte sparen willst. Mach aus den uint16_t uint32_t, dann sind die weg.
Zudem würde ich die 3 globalen, volatilen Variablen am Anfang der Isr in
lokale Variablen ablegen und am Ende der Isr wieder zurückschreiben.
Wenn diese nämlich in der "mainloop" abgefragt werden, dann kann nur
dort eine plötzliche Änderung des Wertes passieren. Und in der Isr läd
der Compiler die einmal in ein Register und gut. Er muß sie dann nicht
mehr bei jeder Benutzung im RAM nachlesen. Denn das dauert!
W.S. schrieb:> OK, dann muß ich dir den Vorwurf machen, daß du eine verbuggte Quelle> geliefert hast.
Behaupten allein reicht nicht, Du mußt es auch beweisen können. Warum
soll er auf anderen Architekturen nicht laufen?
W.S. schrieb:> Du hast da einige recht artistische Kunststückchen> geliefert
Ich sehe da kein einziges Kunststückchen.
Die logischen Verknüpfungen sind alle nachvollziehbar und es ist auch
alles korrekte C-Syntax.
Das mit dem volatile habe ich doch erklärt, warum es in älterem Code
fehlte. Das wäre aber auch der einzige Kritikpunkt.
W.S. schrieb:> Und jetzt fängst du nach dem TO auch noch an, von Compiler-Bugs zu> orakeln.
Es dürfte allgemeiner Konsenz unter Programmierern sein, daß ein
Schreibzugriff auf eine Variable ohne späteres Lesen keinen Effekt haben
darf. Zumindest sehe ich keinen Lesezugriff auf "c".
Natürlich muß es kein Compiler-Bug sein, es kann auch ein Zugriffsfehler
in einem völlig anderen Programmteil sein. Wir kennen das komplette
Programm ja nicht.
Die Zeile mit .L8+16 deutet darauf hin, daß eine weitere Variable
angelegt wird und dadurch sich nachfolgende Variablen verschieben und
somit auch ein Zugriffsfehler.
Ich hab selber schonmal auf einen char-Pointer ein int gespeichert und
mir dann die Karten gelegt. Der Fehler und die Auswirkung waren in 2
völlig verschiedenen Programmteilen.
Hi,
>> Die ganzen Befehle mit X im Namen sind der Tatsache geschuldet, daß du> 6Byte sparen willst. Mach aus den uint16_t uint32_t, dann sind die weg.
Da ging es eigentlich nicht ums Sparen, sonder ist der Tatsache
geschuldet, das der gesamte C-Port 16-Bit breit ist. Das ist zumindest
für das Verständnis von Vorteil.
>> Zudem würde ich die 3 globalen, volatilen Variablen am Anfang der Isr in> lokale Variablen ablegen und am Ende der Isr wieder zurückschreiben.> Wenn diese nämlich in der "mainloop" abgefragt werden, dann kann nur> dort eine plötzliche Änderung des Wertes passieren. Und in der Isr läd> der Compiler die einmal in ein Register und gut. Er muß sie dann nicht> mehr bei jeder Benutzung im RAM nachlesen. Denn das dauert!
Das werde ich mal probieren. Aber irgendwelche Geschwindigkeitssorgen
habe ich sowieso nicht. Das Ganze würde auch auf einem 8 MHz AVR laufen,
aber ich wollte halt mal mich auch mit den STM32-Teilen beschäftigen.
Natürlich könnte so ein Phänomen auch durch fehlerhaften Code in anderen
Programmteilen verursacht werden, aber das Hauptprogramm wurde schon auf
die IRMP-Initialisierung und die Abfrage von get_key_short abgespeckt.
Gruß,
Oliver
Hi,
> W.S. schrieb:>> Ich würde mit solchen Unterstellungen aber ganz vorsichtig sein>> OK, dann muß ich dir den Vorwurf machen, daß du eine verbuggte Quelle> geliefert hast. Möchtest du auch nicht gern hören, ja?>
Diese "verbuggte" Quelle wird schon seit Jahren hier im Artikel
"Tastaturentprellung" verwendet und ist sicherlich hundertfach
erfolgreich im Einsatz. Hör endlich auf anderen pauschal und ohne
Begründung Fehler zu unterstellen. Wenn man noch nicht einmal Zeilen wie
1
volatileuint16_tkey_state=0;
versteht, dann sollte man einfach nur schweigen!
> Ich sag's mal so: Du hast da einige recht artistische Kunststückchen> geliefert, die auf einem AVR sicherlich sehr schön funktionieren, die> aber genau dann mit größter Vorsicht zu genießen sind, wenn man selbiges> auf einer ganz anderen Architektur verwenden will.>
Schon wieder so ein Quark. In der Entpreller-IRQ steht nichts
AVR-spezifisches. Es gibt überhaupt keinen Grund, warum es auf anderen
Architekturen nicht gehen sollte.
Du blickst die Funktion halt nicht und bist auch noch zu faul, Dir den
Artikel dazu durchzulesen (da wird es nämlich erklärt!).
> Und jetzt fängst du nach dem TO auch noch an, von Compiler-Bugs zu> orakeln. Ach nö, sowas riecht mir gar zu sehr nach Ausrede.>
Und Dein Geschreibsel riecht vor allen Dingen nach viel Dampfplauderei.
Pauschale Unterstellungen, gepaart mit Ahnungslosigkeit und
Gast-Account.
-> Fertig ist der Troll
Gruß,
Oliver
(normalerweise nicht so pampig, aber jetzt ist echt mal gut...)
Oliver R. schrieb:> die Abfrage von get_key_short abgespeckt.
Das kann nicht funktionieren, get_key_short muß immer zusammen mit
get_key_long benutzt werden!
Steht aber auch in der Beschreibung. Und REPEAT_MASK muß entsprechend
definiert sein.
Außerdem wäre es sinnlos, nur get_key_short zu verwenden. Braucht man
die Unterscheidung nicht, nimmt man get_key_press.
Hi,
> Oliver R. schrieb:>> die Abfrage von get_key_short abgespeckt.>> Das kann nicht funktionieren, get_key_short muß immer zusammen mit> get_key_long benutzt werden!> Steht aber auch in der Beschreibung. Und REPEAT_MASK muß entsprechend> definiert sein.> Außerdem wäre es sinnlos, nur get_key_short zu verwenden. Braucht man> die Unterscheidung nicht, nimmt man get_key_press.
Im vollständigen Code habe ich ja get_key_long und get_key_short
zusammen verwendet. Dabei sind mir gewisse Unregelmäßigkeiten
aufgefallen. REPEAT_MASK ist auch korrekt definiert...
Gruß,
Oliver
So langsam hasse ich C ... jedes Programm-Listing benutzt andere
komische Symbole, um irgendwelche Vorgänge mit einem Zeichen abzukürzen.
Um das Programmbeispiel am Anfang zu lesen muss man wohl der perfekte
C-Programmierer sein :-)
H-G S. schrieb:> Um das Programmbeispiel am Anfang zu lesen muss man wohl der perfekte> C-Programmierer sein :-)
Eigentlich nicht. Ich halte mich sicher nicht für einen perfekten
C-Porgrammierer, habs aber geblickt, als ich das ganze mal Schritt für
Schritt im AVR Simulator durchgedingst habe und den Scope auf die
verschiedenen Zwischenvariablen gelegt habe.
Ich benutze halt auch nicht jeden Tag den EXOR Operator, der zu Anfang
evtl. etwas verwirrt.
H-G S. schrieb:> So langsam hasse ich C ... jedes Programm-Listing benutzt andere> komische Symbole, um irgendwelche Vorgänge mit einem Zeichen abzukürzen.
Oh Gott, diese weinerliche Generation. 500 Pokemons und deren Skills
auswendig können aber kein C.
> Um das Programmbeispiel am Anfang zu lesen muss man wohl der perfekte> C-Programmierer sein :-)
Nein, man muss nur C können. C hat so um die 25 Schlüsselwörter und so
um die 40 "komische Symbole", bekannt als Operatoren. Andere
Programmiersprachen haben mehr Schlüsselworte und Operatoren.
Von den C Operatoren sind die meisten in einer Art Baukastensystem
konstruiert. Man muss daher nicht alle auswendig lernen.
Lernt man zum Beispiel die Operatoren
+ - * / % << >> & ^ |
(davon sind die ersten vier noch trivial und offensichtlich) und ein
Bauprinzip, weiß man sofort wofür
+= -= *= /= %= <<= >>= &= ^= |=
stehen. D.h. man kann von ca. 40 Operatoren schon mal vier triviale und
10 abgeleitete abziehen. Was bei << und >> passiert kann man sich
einfach visualisieren, daher sind sie auch einfach zu merken.
Mit einem weiteren einfachen Prinzip versteht man aus & und | heraus
&& ||
Ebenfalls trivial und einfach zu lernen sind:
== != < <= > >=
Versteht man != ist ! vielleicht nicht trivial, aber verwandt. ~ muss
man lernen.
Versteht man ++ versteht man auch --.
Die mehrfach verwendeten Symbole muss man lernen, und ein paar giftige
wie ->, ., ",", unary & oder unary * auch.
Jack schrieb:> Nein, man muss nur C können.
...und man braucht Compiler, die unter dem gleichen Quellcode auch das
Gleiche verstehen. Das ist nämlich das große Problem, von dem man hier
jeden Tag 10x lesen kann: Pustekuchen mit Portabilität! Was von dem
einen Compiler freudig gefressen wird, erzeugt beim Anderen Brechreiz,
dem der sich mit völlig unzutreffenden Fehlermeldungen Luft macht.
Nee...
C MUSS man nicht können, wenn man nicht von außen dazu gezwungen wird.
Gruß.
Peter D. schrieb:> Behaupten allein reicht nicht, Du mußt es auch beweisen können. Warum> soll er auf anderen Architekturen nicht laufen?
Weil eben ein eher unbedarfter Benutzer bei deinen Codestücken nicht
durchsieht und genau deshalb nen Fehler beim portieren macht. Darum.
Aber es glauben viel zu viele Benutzer, sie bräuchten ja bloß Peters
Code in ihr Geschreibsel hineinzukopieren und schon ist alles erledigt.
Von wegen.
Für "Maker" (also Leute, die nur benutzen wollen ohne zu verstehen) ist
dein Code ne Verführung zu unreflektierter Verwendung, das wäre
eventuell mein Vorwurf. Guck dir mal meine Art des Codeschreibens an, in
diesem Fall die Entprellung der 42 Tasten an der steinalten Lernbetty.
Das sind zwar rund 30 Zeilen und es ist ein etwas anderes Thema als
deines, aber sie sind auch für nen Laien lesbar:
1
#define Prellzeit 6
2
#define Repetier_lang 90
3
#define Repetier_kurz 10
4
5
volatileintRepCounter;
6
volatileintPrellCounter;
7
8
voidTasten_Bonze(void)
9
{int64ta;
10
11
ta=TastenZustand();
12
if(PrellCounter)/* wenn wir noch beim Entprellen sind */
13
{if(ta)/* wenn noch ne Taste gedrückt ist */
14
{PrellCounter=Prellzeit;
15
if(RepCounter)
16
{--RepCounter;
17
return;
18
}
19
else
20
{RepCounter=Repetier_kurz;
21
TasteEintragen(ta);
22
return;
23
}
24
}
25
else
26
{--PrellCounter;
27
return;
28
}
29
}
30
else
31
{if(!(ta))return;
32
TasteEintragen(ta);
33
RepCounter=Repetier_lang;
34
PrellCounter=Prellzeit;
35
}
36
}
Und nun guck dir den Eröffnungspost des TO an. Er ist genauso
vorgegangen wie ich es weiter oben geschrieben habe, es funktioniert
aber nicht wie erwartet und nun guckt er schon "mehrere Stunden" drauf,
ohne den Haken zu finden. Zum Schluß glaubt er an nen Compilerfehler.
Und warum? Weil es wohl egal ist, wie gut dein Code ist, denn der TO
konnte damit nicht umgehen und hat irgendwas übersehen - auch nach
Stunden des Draufguckens. Wie hieß das? a fool with a tool is...
Nochwas:
Flash Bum Beng schrieb:> ...und man braucht Compiler, die unter dem gleichen Quellcode auch das> Gleiche verstehen. Das ist nämlich das große Problem, von dem man hier> jeden Tag 10x lesen kann
Dazu ein großes JEIN. Es hängt nämlich sehr davon ab, WIE man seinen
Quellcode schreibt.
Wer sich nicht zusammenreißen kann beim Erdenken und Formulieren seiner
Algorithmen und alles ausreizt, was ihm so einfällt, der wird immer
Probleme haben bei Portieren.
Wenn man hingegen einen eher "braven" defensiven Programmierstil
benutzt, dann hat man durchweg keine Probleme damit. Und der Compiler
hat es dann fast immer viel leichter, das Ganze zu optimieren.
Das ist der Knackpunkt.
W.S.
@ W.S.
Du willst es einfach nicht verstehen.
Hier geht es nicht darum, irgendeinen Entpreller-Code aus dem Hut zu
zaubern und es irgendwie hinzubekommen (im genannten Artikel gibt es
genügend Ansätze und Beispiele), sondern um den Erkenntnisgewinn, warum
dieser erprobte Code hier unter diesem System Ärger macht.
Klugscheißerei, dümmliche Unterstellungen und unzutreffende Vermutungen
helfen da nicht weiter.
Bitte such Dir doch einen anderen Thread für Deine Ergüsse!
Gruß,
Oliver
Flash Bum Beng schrieb:> Jack schrieb:>> Nein, man muss nur C können.>> ...und man braucht Compiler, die unter dem gleichen Quellcode auch das> Gleiche verstehen.
Wann hast du Held der Arbeit denn mal einem C-Compiler gesehen, der zum
Beispiel bei == etwas anderes macht als andere C-Compiler?
> C MUSS man nicht können, wenn man nicht von außen dazu gezwungen wird.
Man kann sich den Realitäten natürlich dauerhaft verweigern.
> Gruß.
Ne lass mal stecken.
So, jetzt habe ich wieder etwas Zeit gefunden und habe mir die Quellen
noch einmal angesehen.
Der Fehler lag in der unzureichenden Art und Weise, wie ich die
Atomarität der get_key...-Funktionen sichergestellt habe.
Wie man es richtig macht, hat dankenswerterweise Michael K. in seinem
Post (die verlinkten Quellen) beschrieben.
Um das Posting kurz zu halten, und weil es wirklich so aussah, als ob
die Probleme von der IRQ-Routine kommen, habe ich diese Funktionen nicht
mitgepostet.
Sicherlich wäre es Euch dann schon eher aufgefallen.
Aber es gab dabei auch zwei Gemeinheiten:
1. Ohne die 15000 Interrupts von IRMP zeigte sich das Problem nicht, man
konnte also kein wirklich kleines Testprogramm erzeugen.
2. Das Ein- und Auskommentieren besagter Zeile aus meinem ersten Posting
hat wohl mehr oder weniger zufällig das Timing so geändert, dass die
Entprellung scheinbar korrekt funktioniert hat. Aber auch das war eine
Täuschung, ein paar Zeilen mehr in der IRQ-Routine, dann hat es schon
nicht mehr korrekt funktioniert...
Fazit: Nix Compilerfehler und alles bestens mit Peters Entpreller!
Gruß,
Oliver
Peter D. schrieb:> Also die Funktionskommentare sind ja eine reine Freude. Da kann man> schon neidisch werden.
Danke! Das will ich mal leicht modifiziert an dich zurück geben:
Also die Funktionalität ist ja eine reine Freude. Da kann man schon
neidisch werden.