Code funktioniert nur manchmal

C++ Programmierer

Richtige C++ Programmierer würden wohl Exceptions nutzen.

Aber das dürfen wir AVRler leider nicht.

Das würde die Fehlerbehandlung abdecken, was zumindestens auf den kleinen Arduinos nicht geht, weil die einfach zu wenig Speicher haben.

Auf einem ESP32 dürften Exceptions durchaus benutzbar sein.

Auf Microcontrollern ist OOP auf alle Fälle nützlich und sinnvoll einsetzbar.

Doc_Arduino: Nur goto halte ich hier für völlig fehl am Platz. Schon weil sich die Lesbarkeit in Größenordnungen verschlechtert. Ein sauberes switch-case mit klaren enum Namen und gut ist. Dann ist der Code auch Jahre später einfach wartbar.

Dem kann ich nur zustimmen. :D

Bei geeigneten (zusammenhängenden) Auswahlwerten erzeugt der Kompiler ein computed goto. Das ist bei enums überwiegend der Fall. Weiterhin kann der Kopiler feststellen, wenn man einen der Fälle vergessen hat.

Hallo,

combie, meinste nicht auch das dein goto mit einem switch case nicht deutlich klarer lesbar wäre? Bist du nicht der der lesbaren Code vorzieht … :slight_smile: duck und weg :slight_smile: Ist nur Spass, ist dein Code.

Computed goto kenne ich nicht und befinde mich damit außerhalb meines Wissensstandes. Interessant zu wissen.

if hatte ich schon einmal gegen switch case antreten lassen. Wenn ich mich recht erinnere wurde if mit jedem Vergleich langsamer und switch case war immer konstant gleich schnell/langsam, nur abhängig von der Summe der case Vergleiche. if kann man dafür wiederum von Hand optimieren in dem man die Reihenfolge der Vergleiche vertauscht. Welcher Vergleich wahrscheinlich öfterer zuschlägt wie andere.

Meine Bedenken sind das die Gefahr besteht, wenn das hier Neulinge lesen, die gewöhnen sich goto an weil es bequem ist und dann endet das in Unübersichtlichkeit. Selbst mit einer Batchdatei (Windows) erlebt. Zwangsweise goto zur Ablaufsteuerung verwendet, bin bald blöde gewurden im wilden Gehüpfe.

Ich sage mal so. Ich weiß jetzt das es hier und da sinnvoll sein kann. Nur selbst habe ich es noch nie benötigt und ich denke ich werde es auch nicht benötigen. :slight_smile: Stand heute. Ich befasse mich dann doch lieber immer weiter mit leserlicheren Code zu schreiben für mich und für andere. Auch meine switch case kann ich hier und da noch aufräumen. Es hat eben jeder seine Herangehensweise. Ich nehme gern neue Dinge zur Kenntnis, schau mir das an und entscheide dann für mich alleine ob ich das anwende/verwende oder auch nicht. In dem Fall hier wie man sieht nicht.

Zitat aus “Shooter”. “langsam ist präzise und präzise ist schnell” :slight_smile:

In diesem Sinne allen einen ruhigen Sonntag.

Doc_Arduino: if hatte ich schon einmal gegen switch case antreten lassen. Wenn ich mich recht erinnere wurde if mit jedem Vergleich langsamer und switch case war immer konstant gleich schnell/langsam,

Selbstverständlich. Die Zeit ist konstant weil switch/case i.d.R. als Sprungtabelle umgesetzt wird. Allerdings gehen die schnellen RJMPs (relative jump) "nur" +/- 2kB.

Hallo,

noch einer wach. Mit 2kB meinst du den Speicherbereich in dem gesprungen wird oder darf die angelegte Sprungtabelle nicht 2kB belegten Speicher überschreiten?

combie, meinste nicht auch das dein goto mit einem switch case nicht deutlich klarer lesbar wäre? Bist du nicht der der lesbaren Code vorzieht ... :) duck und weg :) Ist nur Spass, ist dein Code.

Dann zeige doch mal wie du einen solchen Doppelblinker bauen würdest... Dann können wir das nebeneinander halten und an der Sache die "Schönheit", "Eleganz", "Wartungsfreundlichkeit" und "Übersichtlichkeit" bewerten.

Vielleicht kann ich ja von dir noch was lernen!

Ich finde eine Version mit switch und enum ist besser zu lesen, aber schau selbst

void run(const bool blinkAnforderung) {
  switch (aktuell) {
    case Start:
      if (!blinkAnforderung)
        return;
      digitalWrite(ledPin, HIGH);
      aktuell = Hell;
      zeitMerker = millis();
    case Hell:
      if (millis() - zeitMerker < hellPhase)
        return;
      digitalWrite(ledPin, LOW);
      aktuell = Dunkel;
      zeitMerker = millis();
    case Dunkel:
      if (millis() - zeitMerker >= dunkelPhase)
        aktuell = Start;
  }
}

Hier noch mal alles zusammen

enum Zustand {
  Start,
  Hell,
  Dunkel,
};

const byte taster = 2;

class Blinker
{
  protected:
    const byte ledPin;
    const unsigned long hellPhase;
    const unsigned long dunkelPhase;
    unsigned long zeitMerker = 0;
    Zustand aktuell = Start;

  public:
    Blinker(const byte ledPin, const unsigned long hellPhase, const unsigned long dunkelPhase)
      : ledPin(ledPin), hellPhase(hellPhase), dunkelPhase(dunkelPhase) {}

    void run(const bool blinkAnforderung) {
      switch (aktuell) {
        case Start:
          if (!blinkAnforderung)
            return;
          digitalWrite(ledPin, HIGH);
          aktuell = Hell;
          zeitMerker = millis();
        case Hell:
          if (millis() - zeitMerker < hellPhase)
            return;
          digitalWrite(ledPin, LOW);
          aktuell = Dunkel;
          zeitMerker = millis();
        case Dunkel:
          if (millis() - zeitMerker >= dunkelPhase)
            aktuell = Start;
      }
    }

    void init() {
      pinMode(ledPin, OUTPUT);
    }
};

Blinker blinkGruppe[] = { // {pin,hellzeit,dunkelzeit}
  {13, 100, 500},
  {12, 500, 1000},
};

void setup() {
  pinMode(taster, INPUT_PULLUP);
  for (Blinker &blinker : blinkGruppe)
    blinker.init();
}

void loop() {
  bool blinkAnforderung = !digitalRead(taster);
  for (Blinker &blinker : blinkGruppe)
    blinker.run(blinkAnforderung);
}

Ich habe so wenig wie möglich geändert. Was nicht heissen soll, dass ich "einen solchen Doppelblinker" so schreiben würde...

Was nicht heissen soll, dass ich "einen solchen Doppelblinker" so schreiben würde...

Wie würdest du ihn schreiben?

Und ja, so wie deins sehen eigentlich alle meine hier je geposteten Blink Klassen aus.

Mit einer Ausnahme: Den enum würde ich nicht global machen, weil er nicht global gebraucht wird.

Und vermutlich würde ich meine "neue" CombieLib für IO und Timer einsetzen. Aber das Argument gilt noch nicht, weil die Lib noch nicht fertig ist.

Also die von mir "verbesserte" Version:

const byte taster = 2;

class Blinker
{
  protected:
    enum Zustand {  Start,  Hell,  Dunkel,};
    const byte ledPin;
    const unsigned long hellPhase;
    const unsigned long dunkelPhase;
    unsigned long zeitMerker = 0;
    Zustand aktuell = Start;

  public:
    Blinker(const byte ledPin, const unsigned long hellPhase, const unsigned long dunkelPhase)
      : ledPin(ledPin), hellPhase(hellPhase), dunkelPhase(dunkelPhase) {}

    void run(const bool blinkAnforderung) {
      switch (aktuell) {
        case Start:
          if (!blinkAnforderung)
            return;
          digitalWrite(ledPin, HIGH);
          aktuell = Hell;
          zeitMerker = millis();
        case Hell:
          if (millis() - zeitMerker < hellPhase)
            return;
          digitalWrite(ledPin, LOW);
          aktuell = Dunkel;
          zeitMerker = millis();
        case Dunkel:
          if (millis() - zeitMerker >= dunkelPhase)
            aktuell = Start;
      }
    }

    void init() {
      pinMode(ledPin, OUTPUT);
    }
};

Blinker blinkGruppe[] = { // {pin,hellzeit,dunkelzeit}
  {13, 100, 500},
  {12, 500, 1000},
};

void setup() {
  pinMode(taster, INPUT_PULLUP);
  for (Blinker &blinker : blinkGruppe)
    blinker.init();
}

void loop() {
  bool blinkAnforderung = !digitalRead(taster);
  for (Blinker &blinker : blinkGruppe)
    blinker.run(blinkAnforderung);
}

Die Schachtelungstiefe ist allerdings um eins höher, als bei der Goto Version. Auch werden zwei Sprachkonstrukte genutzt, statt nur eins. Aber beides ist kein KO Kriterium, für diese Variante.

Ansonsten, wie schon gesagt:

Wie würdest du ihn schreiben?

Jetzt frei, ohne dich an meine Vorlage zu halten. Nur, identische Funktion, also gleiche Aufgabe.

Denn:

Vielleicht kann ich ja von dir noch was lernen!

Deine run Funktion benutzt mehr unsaubere und/oder exotische Sprachelemente als eine Version mit enum und switch.

    void *sprungZiel = 0; // zustandsmerker
      if (!sprungZiel) sprungZiel = && Start;
      goto *sprungZiel;
      sprungZiel = 0;

Wenn das deinem Kodestil entspricht, kannst du noch viel lernen, egal von wem. ;)

du noch viel lernen, egal von wem.

Genau!

Damit ich dich richtig verstehe... Kritik ja. Zeigen nein.

combie: Damit ich dich richtig verstehe... Kritik ja. Zeigen nein.

Ich habe dir gerade gezeigt wie man es einfacher verständlich und weniger exotisch formulieren kann.

Du sprachest:

Was nicht heissen soll, dass ich "einen solchen Doppelblinker" so schreiben würde...

Und ich fragte darauf hin:

Wie würdest du ihn schreiben?

Aber ist ok, wenn du nicht magst, dann lass es...

Ich habe schon verstanden, dass du vom Thema ablenken willst.

Es geht hier um goto, &&, void* versus struct und enum.

Da ist - meiner Ansicht nach - enum und switch zu bevorzugen, was ich mit meinem Beispiel versucht habe deutlich zu machen.

Wenn du anderer Ansicht bist, von mir aus.

Hallo,

goto umgewandelt in switch case, ich finde das ist besser lesbar, jetzt könnte man noch die Logikwechsel entwirren, mal sehen ob ich das auch noch hinbekomme ...

Vergleichstest ergeben beide Sketche verhalten sich gleich. Zufällig, war keine Absicht, ist der switch case Sketch um 14 Byte kleiner. Ehrlich gesagt dachte ich der wird etwas größer. Egal.

const byte taster = 2;

class Blinker
{
  protected:
  const byte ledPin;
  const unsigned long hellPhase;
  const unsigned long dunkelPhase;
  unsigned long zeitMerker = 0;
  typedef enum {START, HELLPHASE, DUNKELPHASE} state;    // Steuerzustände
  state sprungZiel = START;

  public:
  Blinker(const byte ledPin,const unsigned long hellPhase,const unsigned long dunkelPhase): ledPin(ledPin),hellPhase(hellPhase),dunkelPhase(dunkelPhase){}

  void run(const bool blinkAnforderung)
  {
    switch (sprungZiel) {
      case START:
                  if(!blinkAnforderung) break;
                  digitalWrite(ledPin,HIGH);
                  zeitMerker = millis();
                  sprungZiel = HELLPHASE;
                  break;
   
      case HELLPHASE:
                  if(millis()-zeitMerker

Edit: ihr habt ja schon ohne mich weitergemacht, habt ihr die breaks in den cases absichtlich weggelassen? Der Code rammelt doch ohne break durch? Gleich einmal testen ... Edit 2: okay, wegen der jeweiligen if Abfrage wird das durchrammeln verhindert ;) Gefällt mir auch noch nicht so richtig, der Status darf erst bei Bedarf weitergeschalten werden und sollte dann auch ausgeführt werden ohne zusätzliche Hinderung, mal sehen ob mir da nochwas einfällt ...

Doc_Arduino: ihr habt ja schon ohne mich weitergemacht, habt ihr die breaks in den cases absichtlich weggelassen? Der Code rammelt doch ohne break durch? Gleich einmal testen ...

Ich habe versucht möglichst wenig zu verändern und der originale Kode fiel durch, also habe ich das so gelassen. Wenn man breaks benutzt wiirde es wahrscheinlich einen Hauch schneller, aber auch grösser werden.

Hallo,

so wenig wie möglich ändern war auch meine erste Herangehensweise. Man muss sich erstmal reindenken. Zufrieden war ich mit dem switch case noch nicht. ;) Meine neue Version ist nun auch 6 Byte größer. Egal. Ich finde es noch besser lesbar. Man sieht was wann passiert. Da man naturgemäß mit positiver Logik besser klar kommt, habe ich die positive Logik, die combie in loop mit der Tasterabfrage schon macht in switch case weitergeführt. Die Zustände namentlich machen jetzt auch das was sie aussagen. Das war mein Ziel. Bsp. der Zustand AUS bleibt aus, solange wie nicht erneut geblinkt werden soll, erst dann ändert sich der Zustand. EINSCHALTEN schaltet wirklich nur ein und übergibt in den nächsten Zustand. Danach bleibt der Zustand HELLPHASE solange erhalten wie er gültig ist, macht demnach auch hier genau das was der Name sagt usw.

Alle wieder versöhnt?

switch default könnte man hier weglassen, da alle Zustände behandelt werden, das prüft ja der Compiler.

const byte taster = 2;

class Blinker
{
  protected:
  const byte ledPin;
  const unsigned long hellPhase;
  const unsigned long dunkelPhase;
  unsigned long zeitMerker = 0;
  typedef enum {AUS, EINSCHALTEN, HELLPHASE, DUNKELPHASE} state;    // Steuerzustände
  state sprungZiel = AUS;

  public:
  Blinker(const byte ledPin,const unsigned long hellPhase,const unsigned long dunkelPhase): ledPin(ledPin),hellPhase(hellPhase),dunkelPhase(dunkelPhase){}

  void run(const bool blinkAnforderung)
  {
    
    switch (sprungZiel) {
      case AUS:    
                  if(blinkAnforderung == true) {
                    sprungZiel = EINSCHALTEN;
                  }
                  break;
                  
      case EINSCHALTEN: 
                  digitalWrite(ledPin,HIGH);
                  zeitMerker = millis();
                  sprungZiel = HELLPHASE;
                  break;
                  
      case HELLPHASE:
                  if(millis()-zeitMerker > hellPhase) {
                    digitalWrite(ledPin,LOW);
                    zeitMerker = millis();
                    sprungZiel = DUNKELPHASE;
                  }
                  break;
    
      case DUNKELPHASE:
                  if(millis()-zeitMerker > dunkelPhase) {
                    sprungZiel = AUS;
                  }  
                  break;
                  
      default:    digitalWrite(ledPin,LOW);         // sollte nie eintreten
                  break;          
    }            
  }

  void init()
  {
    pinMode(ledPin,OUTPUT);
  }
};


Blinker blinkGruppe[] = { // {pin,hellzeit,dunkelzeit}
                          {30,100, 500},
                          {31,500,1000},
                        };




void setup()
{
  pinMode(taster,INPUT_PULLUP);
  for(Blinker &blinker:blinkGruppe) blinker.init();
}

void loop()
{
  bool blinkAnforderung = !digitalRead(taster);
  for(Blinker &blinker:blinkGruppe) blinker.run(blinkAnforderung);
}

habt ihr die breaks in den cases absichtlich weggelassen?

Ich, nicht absichtlich. Eher Faulheit.

Ein break bzw. return ist bei dem Ablauf, an der Stelle, nicht nötig. (Edit: wie du ja gerade selber bemerkt hast)

Und was nicht nötig ist, lasse ich gerne weg.... Mein Fokus liegt da eher auf sprechende Bezeichner, möglichst flache Struktur, einfache Bedingungen. u.s.w.

Hallo,

sprechende Bezeichner ...

genau das macht laut meiner Meinung die neue Version. Die alte Version war schon immer einen Zustand weiter der dann wiederum künstlich aufgehalten wurde. Das musste ich entknoten. ;)

wünsche allen noch einen ruhigen sonnigen Sonntag.

PS: wenn jemand noch die Frage zu #47 (46) beantworten kann, bin ich mit allen glücklich und zufrieden

habe ich die positive Logik,... in switch case weitergeführt.

Und dir damit eine weitere Einrückungsebene eingehandelt.

Ich will damit nicht sagen, dass Blockklammer Schachtelungstiefen per se böse sind. Das sind sie nicht. Aber der Mensch kann nicht viel auf einen Blick erfassen. Bis zu einer Tiefe von 3 mag einfach so gehen, und da bist du jetzt angekommen. Mit der Klassenklammer auch schon bei 4. Ab 7 wirds kritisch, würde ich mal so sagen...

Meiner Erfahrung nach: Je flacher ein Code ist, desto flüssiger ist er von oben herab zu lesen.

Je tiefer verschachtelt, desto schwieriger gestaltet sich die Fehlersuche. Um so schlimmer, wenn noch komplizierte Bedingungen dazu kommen.

if(blinkAnforderung == true) Dafür fehlt mir allerdings etwas das Verständnis. Klar ist das richtig so.... Aber if(blinkAnforderung) würde meinem Hang zur Faulheit eher entsprechen.

Das ist hier alles noch lange nicht im schlimmen Bereich. (soweit ich das beurteilen mag)

Serenifly: Selbstverständlich. Die Zeit ist konstant weil switch/case i.d.R. als Sprungtabelle umgesetzt wird. Allerdings gehen die schnellen RJMPs (relative jump) "nur" +/- 2kB.

Doc_Arduino: Hallo,

noch einer wach. Mit 2kB meinst du den Speicherbereich in dem gesprungen wird oder darf die angelegte Sprungtabelle nicht 2kB belegten Speicher überschreiten?

siehe: RJMP Die Sprungtabelle kannst du so groß machen, wie du willst. Aber der Kompiler tut das nicht. Ab wenige Dutzend case im Switch stellt er auf eine if Artige Entscheidungsliste um. Mit den bekannten Laufzeit Nachteilen.

RJMP und Sprungtabelle haben übrigens nicht unbedingt was miteinander zu tun.

Bei Sprungtabellen liegen oftmals nur die Zieladressen im Speicher Bei RJMP ist die Zieladresse fest im Kommando einkompiliert.

Legst du allerdings RJMP Kommandos in die Tabelle, dann darf die Tabelle wirklich nicht größer sein, denn man möchte ja meist ein Ziel außerhalb der Tabelle anspringen.