Hi,
die erste Variante ist deswegen unglücklich (und von MISRA angezählt),
da es kein guter Stil ist, die Zählvariable einer for Schleife im Rumpf
nochmal schreibend zu verändern. Das ganze Konstrukt mit dem
busy-waiting auf ein externes Ereignis (Spannungslevel) ist insgesamt
eher schlecht gewählt, da es - auch so wie es geteilt ist - sinnlos
Rechenzeit verbraucht. Das Ganze sollte man eher von der Anwending
entkoppeln und in einen Treiber auslagern. Der könnte das busy-weiting
dann über einen Zustandsautomaten erledigen und somit rechenzeitneutral
bleiben. Rückmeldung an die Anwendung erfolgt dann über den Zustand des
Automaten.
Grüsse
Frank
Bauform B. schrieb:> Using a restricted form of loop makes code easier> to review and to analyse.
Lese mal den gesamten Absatz:
Rule 14.2 - In automatically generated code...
-
https://www.misra.org.uk/app/uploads/2021/06/MISRA-C-2012-Permits-First-Edition.pdf
Wobei deine Gebilde meiner Meinung nach schon arg an code obfuscation
grenzen. Mit deinem vs_ok = 0; baust du ja quasi eine Endlosschleife und
führst die for-schleife ad absurdum. Von daher ist die zweite Version
schon ein Stück besser. Da kommt man eher darauf das, dass gar keine
Schleife mit 16 Durchläufen ist so wie es die erste Version suggeriert.
Bei so Gebilden wäre ein vorangestellter Kommentar mit "Die Spannung
muss 16 mal hintereinander größer als x sein damit die Endlosschleife
verlassen wird" doch ganz hilfreich:-)
Homer schrieb:> Das ganze Konstrukt mit dem busy-waiting auf ein externes Ereignis> (Spannungslevel) ist insgesamt eher schlecht gewählt
so allgemein ist das natürlich richtig, aber in diesem speziellen Fall
darf das Programm nichts tun, solange die Spannung nicht ausreicht.
Trotzdem finde ich die for-Variante viel schöner!
Bauform B. schrieb:> Ist sowas wirklich leserlicher?
Bei MISRA geht es nicht darum dass irgendwas leserlich wird, im
Gegenteil, MISRA macht vieles unübersichtlich. Bei MISRA geht es nur
darum, einen Satz völlig willkürlich gewählte Regeln zu erfüllen, damit
man dann sagen kann, der Code sei "konform". Was zwar nichts aussagt
außer dass niemand jemals wieder diesen Code anfassen will, aber
irgendwie für gutes Karma wichtig ist.
wäre ja mit ausgelagertem Automaten kein Problem: Die Anwendung schaut
ob der Automat den Zustand "Spannung erreicht" erreicht hat (was erst
nach entsprechend entprellter Überschreitung der Schwelle erfolgt
(Hysterese wäre hier vielleicht auch sinnvoll)), dann geht es erst
weiter. Damit kann dann auch der Überwachungsautomat in einer beliebigen
von der Anwendung unabhängigen Zeitscheibe laufen und sogar während der
Anwendungslebenszeit die Spannung weiter überwachen, falls auch ein
Wiederunterschreiten problematisch wäre...
Irgend W. schrieb:> Wobei deine Gebilde meiner Meinung nach schon arg an code obfuscation> grenzen. Mit deinem vs_ok = 0; baust du ja quasi eine Endlosschleife und> führst die for-schleife ad absurdum. Von daher ist die zweite Version> schon ein Stück besser. Da kommt man eher darauf das, dass gar keine> Schleife mit 16 Durchläufen ist so wie es die erste Version suggeriert.Das ist ein Argument, danke. Ich glaube schon, dass in den meisten
Regeln so ein Körnchen Wahrheit versteckt ist.
Und danke für den Link!
Programmierer schrieb:> MISRA (...) irgendwie für gutes Karma wichtig ist.
na, wenn das kein guter Grund ist :)
Bauform B. schrieb:> aber MISRA erlaubt das nicht.
Natürlich nicht.
Weil es grober Schrott wäre.
Da misst man nie 24V, weil es nur 23 sind oder die Referenzspannung
schwankt, und dein (Steuergerät?) bleibt noch in der
Initialisierungsphase hängen, sogar wenn nur alle 100ms ein kurzer
Einbruch passiert, ohne dem Benutzer zu sagen, warum, und ohne eine
sinnvolle Aktion anzuleiern.
Sagte ich schon, das ist Schrott.
Man erwartet z.B. dass die 24V in 0.1 Sekunde anliegen, erlaubt 0.25
Sekunden durch einen Zähler oder Timerabfrage und macht dann auch bei
13V weiter um Fehlermeldung auszugeben.
Oder man startet gleich und misst dauernd und wenn die 24V mal zu wenig
sind, speichert man das in einem Fehlerflag das dann (auf Anforderung?)
gesendet wird.
Zudem ist deine Schleife übel gestaltet: erst erweckt sie den Eindruck,
nach 16 Durchläufen zu terminieren, dann mogelt sie sich doch zur
Endlosschleife. Solche Programmierhansel würde ich rausschmeissen. Man
manipuliert Schleifenzähler nicht.
Bauform B. schrieb:> hallo c-hater und Freunde,>> wir warten, bis die 24V halbwegs stabil sind. Das könnte doch so> aussehen:>
1
for(intvs_ok=0;vs_ok<16;vs_ok++){
2
>sleep_ms(16);
3
>if(adc_read_24V()<8192){
4
>vs_ok=0;
5
>}
6
>}
Was MISRA hier natürlich leider nicht anmerkt, sind andere Dinge:
* Was bedeutet adc_read_24V() ? Der ADC liest immer 24V?
* Was ist 8192?
* Der Name vs_ok suggeriert ein Prädikat: true/false, warum dann DT int?
* Und natürlich eine potentiell blockierende Operation (wurde schon
genannt).
> Ist sowas wirklich leserlicher?>
1
intvs_ok=0;
2
>
3
>do{
4
>sleep_ms(16);
5
>if(adc_read_24V()>8192){
6
>vs_ok+=1;
7
>}else{
8
>vs_ok=0;
9
>}
10
>}while(vs_ok<16);
Nein, weil die Schleifentransformation, die Du gemacht hast formell
falsch ist: die Negation der Bedingung ist falsch (macht in der Praxis
aber wohl weinig Unterschied ...). Ansonsten treffen weiterhin alle
Probleme von oben zu, und zudem erweiterst Du unnötig den Scope von
vs_ok.
Insgesamt liefert MISRA hier (wie auch sonst sehr häufig), keinen
Beitrag zur Verbesserung. Die wirklich wichtigen Dinge werden von MISRA
sehr oft nicht erfasst, weil sie oft in der unklaren Semantik von
Konstrukten liegt. Wie in diesem Beispiel.
Wilhelm M. schrieb:> * Was ist 8192?
Ein Versuch, ein #define zu vermeiden ;) Ist MIN_STARTUP_VOLTAGE
wirklich besser? Damit müsste ich erst irgendwo suchen, wie viel Volt
das sind. Diese Konstante wird ja garantiert nur hier gebraucht.
> * Der Name vs_ok suggeriert ein Prädikat: true/false, warum dann DT int?
Einverstanden, aber der Name ist falsch, int passt schon.
> * Und natürlich eine potentiell blockierende Operation
Nicht nur potentiell, die muss hier blockieren. Solange der Stecker
nicht in der Steckdose steckt, läuft die Kiste ja auch nicht. Und mit zu
niedrigen 24V eben genauso wenig. Realistischer ist natürlich eine
Schwelle oberhalb von 20V, drunter darf man z.B. kein Relais
einschalten.
> und zudem erweiterst Du unnötig den Scope von vs_ok.
Tja, was soll ich machen, wenn ich kein for benutzen darf?
Also gut, neuer Versuch.
MaWin schrieb:> Da misst man nie 24V, weil es nur 23 sind oder die Referenzspannung> schwankt, und dein (Steuergerät?) bleibt noch in der> Initialisierungsphase hängen, sogar wenn nur alle 100ms ein kurzer> Einbruch passiert
Ja, das ist Absicht. Lieber keine Funktion als eine halbe.
> ohne dem Benutzer zu sagen, warum
Immerhin leuchtet eine rote LED, mehr geht leider nicht.
> und ohne eine sinnvolle Aktion anzuleiern.
Ich kann da keine Ersatzteile bestellen weil ich kein Internet hab ;)
MaWin schrieb:> Man manipuliert Schleifenzähler nicht.
Das sehe ich ja ein, das ist ja der Grund meines Besuchs.
Bauform B. schrieb:> Wilhelm M. schrieb:>> * Was ist 8192?>> Ein Versuch, ein #define zu vermeiden ;) Ist MIN_STARTUP_VOLTAGE> wirklich besser?
Bei reinem C ist das schon mal besser.
Oder eine Umrechnungsfunktion: volt_to_raw(24).
Oder die Goodies von C++ nutzen: 24_V
>> * Der Name vs_ok suggeriert ein Prädikat: true/false, warum dann DT int?>> Einverstanden, aber der Name ist falsch, int passt schon.
Warum soll ein DT, der ein Anzahl repräsentiert negativ werden können?
>> * Und natürlich eine potentiell blockierende Operation>> Nicht nur potentiell, die muss hier blockieren.
Potentiell blockierende Operation ist eine Operation die unendlich lange
blockieren kann, wenn das zugehörige Ereignis nicht eintritt. Genau das
ist hier der Fall.
> Solange der Stecker> nicht in der Steckdose steckt, läuft die Kiste ja auch nicht. Und mit zu> niedrigen 24V eben genauso wenig. Realistischer ist natürlich eine> Schwelle oberhalb von 20V, drunter darf man z.B. kein Relais> einschalten.
Das ist Mist: der Anwender weiß nicht, warum die Kiste nicht läuft.
>>> und zudem erweiterst Du unnötig den Scope von vs_ok.>> Tja, was soll ich machen, wenn ich kein for benutzen darf?> Also gut, neuer Versuch.
1
uint32_tt0;
2
>
3
>t0=uptime_ms;
4
>for(;;){
5
>if(adc_read_mV(VS_24V)<21000){
6
>t0=uptime_ms;
7
>}else{
8
>if((uptime_ms-t0)>128){break;}
9
>}
10
>}
Wieder voll von magischen Konstanten und Variablen im umgebenden Scope.
Das ist noch schlechter.
Ich denke mal, Dein Gerät kennt mehrere Globalzustände: Startup, Init,
Run, Error, ...
Dann gehört das ganze in eine StateMachine, und dann ist das Verharren
im "Init"-Zustand auch wesentlich klarer.
Bauform B. schrieb:> Ein Versuch, ein #define zu vermeiden ;) Ist MIN_STARTUP_VOLTAGE> wirklich besser?
Natürlich!
magic numbers im code sind immer Mist.
Ich würde genauso den Delay-Wert und den Schleifen-Endwert per #define
festlegen.
> Damit müsste ich erst irgendwo suchen, wie viel Volt das sind.
Das ist kein Argument, weil jede auch nur halbwegs gescheite
Entwicklungsumgebung dir den Wert eines #define beim drüberhovern mit
dem Mauszeiger anzeigt.
Das ließe sich sogar noch wartbarer schreiben, wenn du nicht einfach
8192 ins #define OPERATING_THRESHOLD_24V_ADC reinschreibst, sondern den
Wert aus dem gewünschten Spannungs-Schwellwert und dem LSB-Spannungswert
des ADCs (natürlich als weiteres #define festgelegt) ausrechnen läßt.
Wenn sich dann z.B. die Vref für den ADC mal ändern sollte, mußt du
nicht alle magic numbers für ADC-Spannungswerte neu berechnen, sondern
paßt nur das eine #define für den ADC-LSB-Wert an.
Wilhelm M. schrieb:> Das ist Mist: der Anwender weiß nicht, warum die Kiste nicht läuft.
Natürlich, die Natur ist grausam, besonders, wenn Hardware nicht richtig
funktioniert.
>> Ein Versuch, ein #define zu vermeiden ;) Ist MIN_STARTUP_VOLTAGE>> wirklich besser?>> Bei reinem C ist das schon mal besser.> Oder eine Umrechnungsfunktion: volt_to_raw(24).
Die Umrechnung steckt ja in adc_read_24V() bzw. jetzt umbenannt in
adc_read_mV(int channel). Solche Funktionen liefern die Daten natürlich
in mV. Insofern ist 8192 (jetzt 21000) garnicht soo magisch. Man könnte
das Programm sogar auf Papier lesen ohne Umblättern zu müssen ;)
> Wieder voll von magischen Konstanten und Variablen im umgebenden Scope.> Das ist noch schlechter.
Wie wäre es, wenn ich die Schleife in eine Funktion packe, so
wait_for_24V()? Anders wird das Scope-Problem kaum zu lösen sein.
Thorsten S. schrieb:> Das ließe sich sogar noch wartbarer schreiben, wenn du nicht einfach> 8192 ins #define OPERATING_THRESHOLD_24V_ADC reinschreibst, sondern den> Wert aus dem gewünschten Spannungs-Schwellwert und dem LSB-Spannungswert> des ADCs (natürlich als weiteres #define festgelegt) ausrechnen läßt.
Wie gesagt, die adc_read_ Funktionen liefern mV.
> Wenn sich dann z.B. die Vref für den ADC mal ändern sollte, mußt du> nicht alle magic numbers für ADC-Spannungswerte neu berechnen, sondern> paßt nur das eine #define für den ADC-LSB-Wert an.
Und das gehört ins adc-Modul. Den Benutzer der adc-Funktionen geht das
garnichts an.
Will man professionell eine Spannung überwachen, dann prüft man, ob
Standardabweichung und Mittelwert innerhalb eines festgelegten
Toleranzfensters sind. Nur dann kann man feststellen, ob die Messung
glaubwürdig ist.
Weiterhin ist ein Timeout sinnvoll, um eine Fehlermeldung auszugeben.
Einfach nur endlos zu warten, läßt den Anwender ratlos dreinblicken.
Bauform B. schrieb:> Die Umrechnung steckt ja in adc_read_24V() bzw. jetzt umbenannt in> adc_read_mV(int channel).
Ich hoffe, diese Funktion weiß mit adc_read_mV(-42) umzugehen ;-)
> Solche Funktionen liefern die Daten natürlich> in mV. Insofern ist 8192 (jetzt 21000) garnicht soo magisch.
Gut, dann hast Du ja schonmal einen Ratschlag übernommen ;-)
>> Wieder voll von magischen Konstanten und Variablen im umgebenden Scope.>> Das ist noch schlechter.>> Wie wäre es, wenn ich die Schleife in eine Funktion packe, so> wait_for_24V()? Anders wird das Scope-Problem kaum zu lösen sein.
Congrats: Du hast es erkannt.
Man könnte es auch eleganter machen, aber dafür hast Du mit reinem C die
falsche Sprache gewählt. Wobei Du sicher auch nur zum Nutzen der kleinen
(großen) Goodies auch C++ einsetzen könntest (etwa hier: ein Closure).
Und dann denke über den expliziten Zustandsautomaten für die
Globalzustände nach. Das macht ggf. vieles klarer. Und das wird sicher
nicht der einzige Zustandsautomat sein ...
Wilhelm M. schrieb:> Ich hoffe, diese Funktion weiß mit adc_read_mV(-42) umzugehen ;-)
Dagegen gibt es assert() oder return -1; dann muss eben der Aufrufer
damit umgehen.
> Und dann denke über den expliziten Zustandsautomaten für die> Globalzustände nach.
Ich hätte einen mit knapp 1000 Zeilen im Angebot, aber damit fangen wir
jetzt lieber nicht an ;) Hier passt das irgendwie nicht so richtig. Der
Ablauf so total linear, auf Zustand n kann nur n+1 folgen und zurück
geht es nur über Reset. Ja, auch aus dieser Endlosschleife (außer, man
spendiert mehr Volts).
Jedenfalls hab' ich was gelernt, danke dafür!
Bauform B. schrieb:> Wilhelm M. schrieb:>> Ich hoffe, diese Funktion weiß mit adc_read_mV(-42) umzugehen ;-)>> Dagegen gibt es assert()
Da bist Du einer der wenigen (neben mir), die das so sehen. Du kommst
allerdings mit weniger Vorbedingungen aus, wenn Du gleich deinen DT mit
einem besser eingeschränkten Wertebereich nimmt.
> oder return -1; dann muss eben der Aufrufer> damit umgehen.
Da schießt Du Dir im obigen Code allerdings auch selbst ins Knie, denn
das wird dort nicht behandelt.
>> Und dann denke über den expliziten Zustandsautomaten für die>> Globalzustände nach.>> Ich hätte einen mit knapp 1000 Zeilen im Angebot, aber damit fangen wir> jetzt lieber nicht an ;) Hier passt das irgendwie nicht so richtig. Der> Ablauf so total linear, auf Zustand n kann nur n+1 folgen
Auch dafür sind SM wunderbar, und Du könntest für den Anwender noch eine
LED anschalten, ohne das der Code noch unverständlicher wird.
und zurück
> geht es nur über Reset.> Ja, auch aus dieser Endlosschleife (außer, man> spendiert mehr Volts).
Mit mehr Spannung ist dass dann wohl eher ein unumkehrbarer Reset für
immer ;-)
Wilhelm M. schrieb:> Du kommst allerdings mit weniger Vorbedingungen aus, wenn Du> gleich deinen DT mit einem besser eingeschränkten Wertebereich nimmt.
Traditionell gibt es für die Kanalnummern ein typedef enum. Das spielt
mit einem switch schon ganz gut zusammen, nur beim Funktionsaufruf muss
C (oder doch wieder ich?) noch üben.
>> oder return -1; dann muss eben der Aufrufer damit umgehen.> Da schießt Du Dir im obigen Code allerdings auch selbst ins Knie, denn> das wird dort nicht behandelt.
Manchmal ergibt sich das ganz natürlich: -1mV ist kleiner als 21V :) Da
ist C wieder im Vorteil, eine richtige Sprache würde so eine Mischung
aus mV und Status garnicht erlauben.
> Du könntest für den Anwender noch eine LED anschalten
Wenigstens das mache ich schon. Aber jetzt, wo du es sagst:
normalerweise sorgt die Hardware für das passende Licht aus der LED,
warum ging das auf dieser Platine eigentlich nicht?
> Mit mehr Spannung ist dass dann wohl eher ein unumkehrbarer Reset für> immer ;-)
Dass diese Benutzer auch immer alles übertreiben müssen. Vielleicht hält
ja die gute SMA6T ihr Werbeversprechen und macht rechtzeitig einen
Kurzschluss.
Bauform B. schrieb:> Wilhelm M. schrieb:>> Du kommst allerdings mit weniger Vorbedingungen aus, wenn Du>> gleich deinen DT mit einem besser eingeschränkten Wertebereich nimmt.>> Traditionell gibt es für die Kanalnummern ein typedef enum. Das spielt> mit einem switch schon ganz gut zusammen, nur beim Funktionsaufruf muss> C (oder doch wieder ich?) noch üben.
In dem Fall musst Du noch üben ;-)
>>> oder return -1; dann muss eben der Aufrufer damit umgehen.>> Da schießt Du Dir im obigen Code allerdings auch selbst ins Knie, denn>> das wird dort nicht behandelt.>> Manchmal ergibt sich das ganz natürlich: -1mV ist kleiner als 21V :)
Ja, leider. Und schon wieder ist der Leser des Codes maximal an der Nase
herum geführt.
> Da> ist C wieder im Vorteil, eine richtige Sprache würde so eine Mischung> aus mV und Status garnicht erlauben.
Wie ich schon so oft sagte: nutze einfach die Goodies von C++, auch wenn
Du sonst rein prozedural in C-Style programmierst.
Bauform B. schrieb:> Manchmal ergibt sich das ganz natürlich: -1mV ist kleiner als 21V :) Da> ist C wieder im Vorteil, eine richtige Sprache würde so eine Mischung> aus mV und Status garnicht erlauben.
Welche "richtige" Sprache verbietet dir denn einen "signed integer" in
dieser Form zu benutzen?
Wilhelm M. schrieb:> Ja, leider. Und schon wieder ist der Leser des Codes maximal an der Nase> herum geführt.
Jetzt sei nicht so negativ und freu dich mal an den kleinen Geschenken
der Natur ;)
mh schrieb:> Bauform B. schrieb:>> Manchmal ergibt sich das ganz natürlich: -1mV ist kleiner als 21V :) Da>> ist C wieder im Vorteil, eine richtige Sprache würde so eine Mischung>> aus mV und Status garnicht erlauben.>> Welche "richtige" Sprache verbietet dir denn einen "signed integer" in> dieser Form zu benutzen?
Na, mal nur geträumt: von Ada würde ich sowas erwarten. Eine Funktion
kann entweder einen Status (eine endliche Anzahl definierter Werte) oder
eine Spannung (bipolar oder unipolar) liefern, aber nicht beides
abwechselnd. Wenn die Spannung den definierten Wertebereich
überschreitet, gibt es innerhalb der Funktion einen Laufzeitfehler.
Dank vollständiger Tests kann der im Feld ja nicht auftreten. Oder so.
Bauform B. schrieb:> Na, mal nur geträumt: von Ada würde ich sowas erwarten. Eine Funktion> kann entweder einen Status (eine endliche Anzahl definierter Werte) oder> eine Spannung (bipolar oder unipolar) liefern, aber nicht beides> abwechselnd. Wenn die Spannung den definierten Wertebereich> überschreitet, gibt es innerhalb der Funktion einen Laufzeitfehler.> Dank vollständiger Tests kann der im Feld ja nicht auftreten. Oder so.
In Ada hält dich auch niemand davon ab einen integer mit range [-1,
2^16] zu nehmen und den Fehlerfall in der -1 zu codieren.
mh schrieb:> In Ada hält dich auch niemand davon ab einen integer mit range [-1,> 2^16] zu nehmen und den Fehlerfall in der -1 zu codieren.
Das ist ja beruhigend, dass man sich in jeder Sprache in den Fuß
schießen kann :)
Wilhelm M. schrieb:> Nimmst halt C++ und std::optional<>: fertig!
Und er nun wieder, hartnäckig wie ein Versicherungsverkäufer ;)
Ich hab's probiert und konnte meine struct nicht statisch
initialisieren, also sowas
Bauform B. schrieb:> Ich hab's probiert und konnte meine struct nicht statisch> initialisieren, also sowas
1
info_structinfo={
2
>.name="foo",
3
>.version=1
4
>};
Und was soll das sein?
Was soll eine "statische" Initialisierung sein?
Ein struct braucht nicht den Typenamen xxx_struct zu bekommen.
Zeige die Definition der Klasse.
Wilhelm M. schrieb:> Ein struct braucht nicht den Typenamen xxx_struct zu bekommen.
Was sonst? xxx_t ist verboten.
> Zeige die Definition der Klasse.
Klassen gibt's nicht, nur C
The requirement that additional types defined in this section end
4
in "_t" was prompted by the problem of name space pollution. It is
5
difficult to define a type (where that type is not one defined by
6
POSIX.1-2017) in one header file and use it in another without adding
7
symbols to the name space of the program. To allow implementors to
8
provide their own types, all conforming applications are required
9
to avoid symbols ending in "_t", which permits the implementor to
10
provide additional types. Because a major use of types is in the
11
definition of structure members, which can (and in many cases must)
12
be added to the structures defined in POSIX.1-2017, the need for
13
additional types is compelling.
> Was willst Du mit den ganzen zusammenhanglosen Bemerkungen eigentlich> erreichen?
C Lernen; jede Bemerkung hilft. Ich finde, wir sind garnicht so weit vom
Thema abgekommen.
Bauform B. schrieb:> Wilhelm M. schrieb:>>> Was sonst? xxx_t ist verboten.>> Wer sagt das?>> Der C-Standard reserviert erstmal nur int*_t und uint*_t, aber auch> Sachen wie tss_t oder wctrans_t. Das alleine würde mir schon reichen.> POSIX reserviert sich dann gleich alles, was auf _t endet:> https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html
Wieder Salamitaktik: jetzt kommt auch noch Posix-conformance dazu.
Auf einem µC-Board?
>> Was willst Du mit den ganzen zusammenhanglosen Bemerkungen eigentlich>> erreichen?>> C Lernen; jede Bemerkung hilft. Ich finde, wir sind garnicht so weit vom> Thema abgekommen.
Dann schau wenigstens einmal über den Tellerrand hinaus!
Bauform B. schrieb:> Der C-Standard reserviert erstmal nur int*_t und uint*_t, aber auch> Sachen wie tss_t oder wctrans_t. Das alleine würde mir schon reichen.
Da gibts noch einiges mehr, unter anderem alles, was auf is, to oder str
gefolgt von einem Kleinbuchstaben anfängt, wie z.B.
Bauform B. schrieb:> uint32_t *top_of_stack; // 00 initial SP - Offset
Rolf M. schrieb:> Da gibts noch einiges mehr, unter anderem alles, was auf is, to oder str> gefolgt von einem Kleinbuchstaben anfängt, wie z.B.
Das ist gemein, das gilt nicht! Aber warte, bis ich das kleingedruckte
gelesen hab'.
Bauform B. schrieb:> Rolf M. schrieb:>> Da gibts noch einiges mehr, unter anderem alles, was auf is, to oder str>> gefolgt von einem Kleinbuchstaben anfängt, wie z.B.>> Das ist gemein, das gilt nicht! Aber warte, bis ich das kleingedruckte> gelesen hab'.
Nimm einen C++ namespace für Deine eigenen Bezeichner ;-)
>> Und was soll das sein?>> Was soll eine "statische" Initialisierung sein?>> Ein struct braucht nicht den Typenamen xxx_struct zu bekommen.>> Zeige die Definition der Klasse.
Er meint designated initializers. C only. Sehr hilfreich. Einer der
Gründe, weswegen wir unseren Code ebenfalls nicht einfach so mit C++
übersetzen können, um die "kleinen goodies" zu benutzen.
Jan K. schrieb:> Er meint designated initializers.
Habe ich schon verstanden ...
> C only.
Nicht mehr: ist auch in C++ verfügbar (weiß grad nicht ob 17 oder 20)
Sonnenkönig schrieb:> Wilhelm M. schrieb:>> typedef struct image_struct { ... } image_struct;>> Maximal degeneriert.
Völlig Standard in C. Auch wenn ich es so schreiben würde (und auch
tue):
typedef struct _module_image_t { ... } module_image_t;
Vorteile:
- man sieht, dass es ein Typ sein soll
- struct muss nicht mehr geschrieben werden
- intellsisense/memory watch kann die structs auflösen und anzeigen
- man keine Namenskonflikte durch den module "namespace"
Wilhelm M. schrieb:> Jan K. schrieb:>>> Er meint designated initializers.>> Habe ich schon verstanden ...
Sorry, alles klar :)
>>> C only.>> Nicht mehr: ist auch in C++ verfügbar (weiß grad nicht ob 17 oder 20)
Guter Hinweis, danke! Gibt's leider kaum zertifizierte Compiler für,
Keil ist zumindest bei C++14. Ist sicherlich ne andere Baustelle als
hier gezeigt.
Ich habe keinen MISRA-Compiler zur Hand. Trotzdem würde ich das
Original-Konstrukt in einem Code-Review rauswerfen.
Nur so aus Jux, hier mal das Original umgeschrieben. Zum Busy-Waiting,
fehlender Durchschnittsbildung usw. wurde eigentlich alles gesagt, ich
habe das trotzdem mal so gelassen. Und ohne das durch einen Compiler
gesendet zu haben:
1
// Header with pseudo units - mainly for documentation purposes
Wilhelm M. schrieb:> Jan K. schrieb:>>> Er meint designated initializers.>> Habe ich schon verstanden ...>>> C only.>> Nicht mehr: ist auch in C++ verfügbar (weiß grad nicht ob 17 oder 20)
20, allerdings mit einer Einschränkung: Man muss die Initializer in der
Reihenfolge angeben, in der die Member-Variablen definiert sind. Das ist
für mich ein erheblicher Nachteil.
Wilhelm M. schrieb:> Nicht mehr: ist auch in C++ verfügbar (weiß grad nicht ob 17 oder 20)
g++ kann es irgendwie schon länger, sogar mit der unschönen
C++20-Einschränkung, daß die Reihenfolge der Elemente eingehalten werden
muß.
Oliver
Oliver S. schrieb:> Wilhelm M. schrieb:>> Nicht mehr: ist auch in C++ verfügbar (weiß grad nicht ob 17 oder 20)>> g++ kann es irgendwie schon länger, sogar mit der unschönen> C++20-Einschränkung, daß die Reihenfolge der Elemente eingehalten werden> muß.
Ja, viele Sachen kann der GCC schon viel länger, weil er oft die
Referenzimplementierungsplattform ist:
https://en.cppreference.com/w/cpp/compiler_support
Hannes J. schrieb:> Nur so aus Jux, hier mal das Original umgeschrieben. Zum Busy-Waiting,> fehlender Durchschnittsbildung usw. wurde eigentlich alles gesagt, ich> habe das trotzdem mal so gelassen. Und ohne das durch einen Compiler> gesendet zu haben:
In Zeile 15 fehlt ein "L", da ansonsten die Error-Direktive immer
zuschlägt: REQUIRED_SAMPES => REQUIRED_SAMPLES
;-)
Und MISRA-C2012 sagt folgendes zur while-loop:
1
The right-hand operand of a logical && or || operator shall not contain side effects - MISRAC2012-Rule-13.5
Eventuell ist das ja genau so gewollt, wenn nicht, kann man aber mit
solchen Konstrukten einiges an "Spaß" haben...
Gruß,
Michael
Vielen Dank für die Aufmerksamkeit -- auch wenn mir mache Vorschläge ein
wenig abwegig erscheinen.
Michael F. schrieb:> In Zeile 15 fehlt ein "L", da ansonsten die Error-Direktive immer> zuschlägt: REQUIRED_SAMPES => REQUIRED_SAMPLES>> ;-)
Und deswegen vermeiden wir #define. Ja, es gibt auch 1 bis 2 nützliche
Anwendungen, aber in diesem Beispiel wären sogar die magischen Zahlen
besser gewesen :)
Jan K. schrieb:> typedef struct _module_image_t { ... } module_image_t;
wir haben doch gerade festgestellt, dass manche Namen reserviert sind.
Das "t" am Ende mag ja noch als Gewohnheitsrecht durchgehen, aber "_" am
Anfang? Echt jetzt?
OK, das ist ja alles nicht tragisch, aber das?!
Rolf M. schrieb:> Bauform B. schrieb:>> Der C-Standard reserviert erstmal nur int*_t und uint*_t, aber auch>> Sachen wie tss_t oder wctrans_t. Das alleine würde mir schon reichen.>> Da gibts noch einiges mehr, unter anderem alles, was auf is, to oder str> gefolgt von einem Kleinbuchstaben anfängt, wie z.B.>> Bauform B. schrieb:>> uint32_t *top_of_stack; // 00 initial SP - Offset
top_of_stack als struct member dürfte nicht betroffen sein, aber als
globale Variable? Bei Funktionen wie torque_limiter() ist es dann ganz
vorbei. Natürlich gibt es den Namen nicht in der libc, aber er ist
"potentially reserved" und darf jederzeit eingeführt werden. Was auch
noch nicht schlimm wäre, wenn es nur nicht kompilieren würde -- aber
meine Verwendung ist ab dann undefined behavior!
Der einzige Trost: ein uC-Programm kann man i.d.R. mit -ffreestanding
-nostdlib übersetzen (auch mit FreeRTOS?). Dann gibt es toupper(),
strcmp() usw. nicht und Macros benutzt man ja nicht. Also bleiben
praktisch nur die Einschränkungen der Sprache selbst. Und ich muss auf
assert() verzichten, das gibt's nicht im freestanding environment :(
https://en.cppreference.com/w/c/language/identifier
> The right-hand operand of a logical && or || operator shall not contain
2
> side effects - MISRAC2012-Rule-13.5
3
>
Och wie gemein.
> Eventuell ist das ja genau so gewollt,
Das war in der Tat so gewollt. Der Schleifenabbruch wenn needed_samples
0 ist sollte Vorrang vor einem weiteren Decrement von
remaining_total_samples haben, damit man bei einem erfolgreichen Warten
nicht in letzter Sekunde in das folgende if(remaining_total_samples <=
0) läuft.
Na ja, muss man halt die Schleife etwas umschreiben. Das if() sollte man
sowieso auf if(needed_samples > 0) ändern.
Bauform B. schrieb:> Und deswegen vermeiden wir #define. Ja, es gibt auch 1 bis 2 nützliche> Anwendungen, aber in diesem Beispiel wären sogar die magischen Zahlen> besser gewesen :)
Ehrlich? Kindergarten-Privatmeinung. Man kann sich natürlich alle Arten
von abstrusen Begründungen basteln wenn man keine #defines mag.
Ich hätte übrigens eher gedacht das sich jemand über den als Köder
reingequetschten ?:-Operator aufregt. Na ja, so kann man sich irren.
Bauform B. schrieb:> top_of_stack als struct member dürfte nicht betroffen sein, aber als> globale Variable?
Richtig, die Namen sind für mögliche Funktionen vorgesehen und sind
daher nur dort problematisch, wo es es mit einer Funktion zum
Namenskonflikt kommen kann.
> Bei Funktionen wie torque_limiter() ist es dann ganz> vorbei. Natürlich gibt es den Namen nicht in der libc, aber er ist> "potentially reserved" und darf jederzeit eingeführt werden. Was auch> noch nicht schlimm wäre, wenn es nur nicht kompilieren würde -- aber> meine Verwendung ist ab dann undefined behavior!
Der Standard behält sich vor, in bestimmten Teilen der
Standardbibliothek neue Funktionen mit solchen Namen einzuführen. Für
die besagten mit 'to' wären das z.B. <ctype.h> und <wctype.h>. Nun ist
es recht unwahrscheinlich, dass je eine Funktion namenes
torque_limiter() ihren Weg dorthin findet, aber ja, streng genommen ist
das Verhalten dann undefiniert.
Wilhelm M. schrieb:> Das ist jetzt ja noch schlimmer:
Vor allem ist das Grundproblem nicht behoben:
Keine - gar keine - Funktion wenn bloss die Spannungsmessung nicht geht.
Dabei lauft der Prozessor, hat seinen RESET und brown out hinter sich,
er hat seine Betriebsspannung und könnte kommunizieren und dort deutlich
sagen "ich bin da aber die 24V liegen erst bei 23V"
Stattdessen darf der Anwender nun fluchen weil er nicht weiss was der
Grund für die Fehlfunktion ist.
MaWin schrieb:> Vor allem ist das Grundproblem nicht behoben:>> Keine - gar keine - Funktion wenn bloss die Spannungsmessung nicht geht.
Immerhin leuchtet ab 4 bis 5 Volt eine rote LED. Und ansonsten ist das
so gewollt, ich sehe einfach das Problem nicht.
Wie wahrscheinlich ist denn ein Fehler der Messung selbst? Da sind außer
dem uC selbst 2 Widerstände, ein C und eine Durchkontaktierung
beteiligt. Ja, ich benutze die 3.3V als Referenz. Wenn also der
3.3V-Regler 4.2V liefert wird eine zu niedrige Spannung gemessen. Wenn
deshalb nicht eingeschaltet wird, ist das durchaus kein Nachteil. Aber
mit 6V läuft der uC sowieso nicht mehr.
Meinetwegen benutze zusätzlich ich die interne Referenz. Allerdings wird
dadurch die Wahrscheinlichkeit größer, dass das Teil nicht startet.
> Das ist jetzt ja noch schlimmer:
Jetzt sind wir uns wieder einig :)
Bauform B. schrieb:> ein uC-Programm kann man i.d.R. mit -ffreestanding -nostdlib> übersetzen> Und ich muss auf assert() verzichten, das gibt's nicht im> freestanding environment :(
Hast du evt. dazu eine Empfehlung? Das ist mir jetzt wegen der
reservierten Namen aufgefallen. -nostdlib benutze ich schon von Anfang
an, aber das verhindert nicht, dass beliebige libc-Header benutzt
werden. Bei der rundum-sorglos Toolchain ist natürlich newlib und
assert.h dabei.
Einzelne Funktionen wie logf() oder memset() hab' ich irgendwo geklaut,
warum sollte ich assert.h nicht auch kopieren?
Bauform B. schrieb:> Einzelne Funktionen wie logf() oder memset() hab' ich irgendwo geklaut,> warum sollte ich assert.h nicht auch kopieren?
Das assert()-Macro musst Du Dir eh selbst basteln, denn in einem µC
Projekt wird Dir ein terminate() wenig nutzen. Du wirst wohl mit einem
Pin wackeln müssen oder was immer Du willst.
Bauform B. schrieb:> ich sehe einfach das Problem nicht.
Das ist ja das Problem.
Dein Ding geht erst los, wenn die 24V erfolgreich gemessen werden.
Wenn während der Benutzung die 24V ein Problem haben, interessiert das
offenbar kein Schwein.
Wir wissen nicht, in welchem Umfeld, aber MISRA klingt nach automotive
und 24V nach LKW, also ein Steuergerät.
Man will keine Steuergeräte die nicht starten aus obskuren Gründen. Es
können dutzende Gründe sein und man ist bei Fehlfunktion am Rätselraten.
Man will, so bald man das Steuergerät erreichen kann, eine anständige
Fehlerdiagnose, ublicherweise einen per OBD abfragbetes Fehlerflag für
jede Möglichkeit.
Wenn dein Ding angestöpselt wird, in der Hand gehalten wird, und für
jede Fehlermöglichkeit eine LED hat
grün Power ist da
gelb uC hat RESET geschafft
rot die 24V sind nicht da
blau alles ok
Mag man ja feedback haben, aber mir scheint es müssten viele weitere LED
sein.
MaWin schrieb:> Wenn während der Benutzung die 24V ein Problem haben, interessiert das> offenbar kein Schwein.
Naja, Schwein hat halt andere Prioritäten. Das Programm ist durchaus
dran interessiert.
> Wir wissen nicht, in welchem Umfeld, aber MISRA klingt nach automotive> und 24V nach LKW, also ein Steuergerät.
MISRA ist mehr eine Art Hobby. In jeder Regel steckt ein Körnchen
Wahrheit und jeder Tipp hilft. LKW trifft es schon fast: Diesellok, aber
nur ein abgesetztes Display.
> Mag man ja feedback haben, aber mir scheint es müssten> viele weitere LED sein.
Ja, die hätte ich auch gerne. Aber aufpassen: grüne LED könnten mit
einem grünen Signal verwechselt werden.
Wie wäre denn das:
rot: uC hat Reset nicht geschafft
rot blinkend: 24V sind nicht da
aus: alles gut (mit Text im Display)
Bauform B. schrieb:> Jan K. schrieb:>> typedef struct _module_image_t { ... } module_image_t;>> wir haben doch gerade festgestellt, dass manche Namen reserviert sind.> Das "t" am Ende mag ja noch als Gewohnheitsrecht durchgehen, aber "_" am> Anfang? Echt jetzt?
Jo, ist völlig standard. "_" ist nicht verboten, "__" hingegen schon.
Machen auch richtig große Libs so, z.B. GTK
https://github.com/GNOME/gtk/blob/main/gtk/gtkactionmuxerprivate.h, oder
ganz viele ARM libs, siehe z.B.
https://github.com/ARM-software/CMSIS_5/blob/develop/CMSIS/Driver/Include/Driver_CAN.h
Es mag sein, dass "_t" in POSIX verboten ist, dann nimmt man halt "_T"
oder "_type"... POSIX ist jetzt aber auch nicht soo verbreitet bei µCs.
Was stört dich daran? Sonst müsste man sich doch noch einen Namen für
die struct ausdenken. Ich weiß, dass es unnamed structs gibt, aber wie
erwähnt hilft es immens, wenn der debugger die struct auch auflösen kann
und ich sehe, wie diese heißt.
Jan K. schrieb:> Jo, ist völlig standard. "_" ist nicht verboten,
Doch, wenn darauf ein Großbuchstabe folgt, schon:
"All identifiers that begin with an underscore and either an uppercase
letter or another underscore are always reserved for any use."
und außerdem:
"All identifiers that begin with an underscore are always reserved for
use as identifiers with file scope in both the ordinary and tag name
spaces."
Na, wurde jetzt schon die MISRA-Konforme Warteschleife gefunden?
Eine klitzekleines Randproblemchen sorgt für Stunden sinnloser
Beschäftigung, und am Ende kommt was völlig verkrüppeltes heraus, nur
damit es Regeln zur vermeintlichen Qualitätssteigerung entspricht.
Herrlich, wie diese Idiotie für ausufernde Kosten sorgt...
Bauform B. schrieb:> Diesellok, aber nur ein abgesetztes Display.
Display heisst, das Ding kann selber anzeigen, was ihm nicht behagt.
Lass es hochfahren, sich initialisieren, und frage dann in einer
Schleife ständig alles wissenswerte ab. Und wenn eben die 24V ausser
Toleranz sind, kann sie das anzeigen, aber trotzdem auch die anderen
Messwerte.
Bauform B. schrieb:> aber MISRA erlaubt das nicht.
MISRA erlaubt das durchaus. Es empfiehlt sich, die gesamten
MISRA-Guidelines durchzulesen, und nicht nur das Kapitel mit den
eigentlichen Regeln.
Bei der beanstandeten Regel geht es (wahrscheinlich) um die "Rule 14.2:
A for loop shall be well-formed". Diese ist in der Required-Kategorie,
d.h. um MISRA-konform zu sein muss man das Nichteinhalten der Regel
dokumentieren (entweder für das gesamte Projekt oder für die konkrete
Stelle im Code).
Jan K. schrieb:> Jo, ist völlig standard. "_" ist nicht verboten, "__" hingegen schon.> Machen auch richtig große Libs so, z.B. GTK> oder ganz viele ARM libs
Die beiden, und die CMSIS libs besonders, meinen wahrscheinlich, dass
sie zur "implementation" gehören wie die libc. Wenn wir das mal fein
unterteilen:
1) gcc
2) libgcc
3) newlib
4) CMSIS
5) meine eigene lib
6) mein Programm
wo ist dann die Grenze? 1 und 2 dürfen natürlich alles und 6 natürlich
nichts davon. Wenn ich unbedingt den _ am Anfang benutzen wollte, könnte
man höchstens über 5 verhandeln.
> Es mag sein, dass "_t" in POSIX verboten ist, dann nimmt man halt "_T"> oder "_type"...
oder eben _struct und _enum; kaum länger als _type und ein klein wenig
mehr Information. Was soll xxx_struct schon anderes sein als ein type
(da, wo es benutzt wird)?
> POSIX ist jetzt aber auch nicht soo verbreitet bei µCs.
Naja, eine ausgewachsene shell eher nicht, write() und sbrk() gibt's
auch in der newlib-nano. Und es gibt ein paar bewährte Regeln. Warum
soll man das auf einem uC nicht nutzen?
> Ich weiß, dass es unnamed structs gibt
Die braucht man im inneren einer union, aber sonst will die doch keiner
haben.
MaWin schrieb:> Display heisst, das Ding kann selber anzeigen, was ihm nicht behagt.
Jein, nicht mit weniger als 7 Volt. Die magischen 8192mV aus meiner
ursprünglichen Frage waren direkt aus dem Programm. Dann dachte ich, es
wird verständlicher, wenn ich hier 21V schreibe. War wohl nichts ;)
Also noch weiter weg von Thema ;) Die Idee dabei ist, kurze Ausfälle der
24V abzufangen. Dabei sinkt die Spannung nicht bis auf 0 und manche
Chips laufen normal weiter und andere (besonders das Display) nicht.
Deshalb erzeuge ich gleich einen Reset, sobald die Spannung unter 7V
sinkt. Nach einem Reset wird kaum mehr initialisiert als der ADC, dann
kommt die berüchtigte Warteschleife und erst danach die restliche
Initialisierung.
Ich finde, das passt gut zu solchen Start-Verweigerungen wie bei einem
Flash-CRC-Fehler oder nach dem 3. Watchdog-Reset oder so.
Christian F. schrieb:> Bei der beanstandeten Regel geht es (wahrscheinlich) um die "Rule 14.2:> A for loop shall be well-formed". Diese ist in der Required-Kategorie
Ja und ja, das ist schon klar. MISRA ist hier kein Muss, mir geht es nur
um besser lesbare Programme. Da ist jeder Hinweis willkommen, wie z.B.
die 14.2. Nur fand ich die Begründung dazu etwas dünn.
In dem Moment, wenn ich etwas schreibe, sieht das für mich natürlich
total eindeutig und übersichtlich aus. Und sobald es funktioniert, ist
es auch schon wieder vergessen. C bietet so viele Möglichkeiten, etwas
unleserlich zu schreiben. Welche Sprache hat schon einen Obfuscated Code
Contest?
Bauform B. schrieb:> (besonders das Display) nicht. Deshalb erzeuge ich gleich einen Reset,> sobald die Spannung unter 7V sinkt
Na ja, man könnte auch das Display neu initialisieren, wenn man meint
das hätte es nötig (reagiert nicht, jede Minute, Spannung war mal
niedriger wobei man dabei kurze Einbruche mitbekommen muss) und nicht
gleich mit Allem von vor beginnen. Der uC sollte durch brown out
detection selbst neu starten wenn er es nötig hat.
Ich initialisiere Peripherie auch öfter neu als nötig, einfach damit es
nicht vergessen wird.
Mich nervt z.B. Arduino 4 bit LCD 16x2 das nach Reset aber ohne power
loss nicht korrekt re-initialisiert wird, weil es schon auf 4 bit
initialisiert war, und die Leute das nicht behandeln sondern glauben es
wäre 8 bit.
Rolf M. schrieb:> MISRA kann Hirn nicht ersetzen, auch wenn viele das meinen.
MISRA ist wie Diätpillen. Nutzlos für die Verwender, aber eine Goldgrube
für die Verkäufer.
Thread beobachten
Seitenaufteilung abschalten