Code funktioniert nur manchmal

Hier ist ein guter Artikel zur Performance von computed goto und wieso switch langsamer ist:
https://eli.thegreenplace.net/2012/07/12/computed-goto-for-efficient-dispatch-tables

Das ist aber auch ein spezieller Fall. computed goto ist nicht das gleiche wie ein normales goto. Hier braucht man die extra Geschwindigkeit nicht (außerdem gibt eine keine branch prediction die etwas beschleunigen könnte). Und man sollte verstehen was man macht und wieso.

Und man sollte verstehen was man macht und wieso.

Eigentlich soll Computed Goto Distanzen berechnen können. Damit relative Gotos ermöglichen. Das gilt für unseren GCC, noch als broken.(soweit mir bekannt)

Also eine Warnung: Computed Gotos verwenden, ok. Aber bitte keine Rechenoperationen auf den Lable Adressen ausführen und dann anspringen. Das kann ins Auge gehen.

//----

Übrigens, eine Suche über meinen SketchOrdner, inklusive Libraries und Hardwaredefinitionen, führte über 1300 Gotos ans Licht. Auch, oder eher gerade, in namhaften Libraries.

//----

Du möchtest ja Beispiele sehen...... Hier ein Doppelblinker Beispiel, aus meiner Wühlkiste, für einen kleinen endlichen Automaten mit Goto:

const byte taster     = 2;

class Blinker
{
  protected:
  const byte ledPin;
  const unsigned long hellPhase;
  const unsigned long dunkelPhase;
  unsigned long zeitMerker = 0;
  void *sprungZiel = 0; // zustandsmerker

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

  void run(const bool blinkAnforderung)
  {
    if(!sprungZiel) sprungZiel = &&Start;
    goto *sprungZiel;

    Start:
      if(!blinkAnforderung) return;
      digitalWrite(ledPin,HIGH);
      sprungZiel = &&HellPhase;
      zeitMerker = millis();
    
    HellPhase:
      if(millis()-zeitMerker

Ob das jetzt ein gutes Beispiel ist, kann ich nicht sagen. Aber dennoch ist es mit Goto und dabei recht übersichtlich.

Bei der Verwendung von enum Switch/case würde der Compiler versuchen eine Sprungtabelle anzulegen. Dann würden sich erstmal keine Performance Vorteile, für Goto, ergeben.

Aber: Wenn die case nicht fortlaufend definiert sind, oder die Anzahl ein gewisses Maß überschreitet, dann generiert der Compiler ein Konstrukt, welches einer langen if Kette entspricht. Dann dauert es eben diese Anzahl Vergleiche, bis der betreffende Case erkannt und ausgeführt wird. Natürlich, je weiter ein Fall hinten steht, desto länger dauert es dann bis zur Ausführung.

Serenifly: Fehlerbehandlung und Resourcenfreigabe am Ende einer Funktion: https://blog.regehr.org/archives/894

Wird z.B. im Linux Kernel sehr oft verwendet

Weil der Linux Kernel in C geschrieben ist.

C++ Programmierer benutzen zumindestens für die Resourcen bevorzugt Konstruktoren/Destruktoren.

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.