Mein erstes Programm, bin ich auf dem Holzweg?

Hallo Allerseits
Ich bin neu hier im Forum und möchte mich deshalb mal kurz vorstellen:
Mein Name ist Erwin, ich komme aus der Schweiz, in meiner Freizeit bin gerne mal was am basteln und experimentieren. Programmiertechnisch habe ich bisher einige Projekte in VBA (Excel , Access) umgesetzt, in Python(RaspberryPi) habe ich auch schon ein zwei kleine Projekte umgesetzt.

Mit Arduino habe ich mich jedoch bisher nie beschäftigt. Da dies doch eine neue Programmiersprache für mich ist, habe ich mal versucht ein kleines Programm (LED-Blinker Taster-gesteuert) zu schreiben und das so aufgebaut, wie ich es es für Sinnvoll erachte.

Jetzt wüsste ich gerne von euch Profis, ob es Sinn macht auf Arduino so zu programmieren oder entstehen da früher oder später bei komplexeren Programmen Probleme?
Ich möchte mir nicht von Anfang falsche Methoden angewöhnen.
Vielen Dank schon mal für eure Bemühungen mir den Einstieg in die Arduino-Welt zu erleichtern.

Hier mein erster Code den ich geschrieben habe, läuft auf einen Arduino Uno (China Kopie)

const int led_Pin[] = { 13 };       //Pin Nummern für Led
const int taster_Pin[] = { 2, 8 };  //Pin Nummern für Taster
const int entprellZeitMs = 40;      //Wert für Taster-Entprellung

int intervall = 500;        //Wert für blinker_stabil
int ledOnMs = 20;           //Wert für blinker_instabil
int ledOffMs = 980;         //Wert für blinker_instabil
int modus = 1;              //Wert wird durch schalter2 gesetzt

bool led_Status[] = { LOW };                //Wert sollte gleiczeitig mit digitalWrite() gesetzt
bool taster_Status[] = { HIGH, HIGH };      //Werte werden gelesen
bool schalter_Status[] = { LOW };           //Wert wird von schalter1 gesetzt
bool taster_Gedrueckt[] = { LOW, LOW };     //Wert für Taster-Entprellung

unsigned long letzteSchaltungMs[] = { 0, 0 };     //Wert für Taster-Entprellung
unsigned long letzerWechselMs = 0;                //Wert für blinker_stabil & blinker instabil
unsigned long aktuelleMs = 0;                     //Wert für blinker_stabil & blinker instabil



void setup() {
  // put your setup code here, to run once:
  pinMode(led_Pin[0], OUTPUT);
  pinMode(taster_Pin[0], INPUT_PULLUP);
  pinMode(taster_Pin[1], INPUT_PULLUP);
}

void loop() {
  // put your main code here, to run repeatedly:

  tasterCheck(0);
  tasterCheck(1);
  lichtmodi();
  //schalter1_aktion();
}




//____________________________
//
//* * * Taster Aktionen * * *
//____________________________

void tasterAktion(int ArrNr) {  //Auswahl welche aktion durgeführt werden soll(welcher Taster wurde gedrückt)
  switch (ArrNr) {
    case 0:
      taster1_aktion();
      break;
    case 1:
      taster2_aktion();
      break;
      // case 2:
      //   taster3_aktion();
      //   break;
  }
}


void taster1_aktion() {
  schalter2();          //Taster als umschalter aktivieren
}

void taster2_aktion() {
  modus = 1;            //LED wird ausgeschalten
}

// void taster3_aktion() {

// }

// _____________________________
//
// * * * Taster entprellen * * *
// _____________________________

void tasterCheck(long taster_Nr) {

  taster_Status[taster_Nr] = digitalRead(taster_Pin[taster_Nr]);        //Pin auslesen
  if (taster_Status[taster_Nr] == LOW) {
    letzteSchaltungMs[0] = millis();        //solange der Taster gedrückt wird, wir diesert Wert angepasst
    taster_Gedrueckt[taster_Nr] = HIGH;     //solange der Taster gedrückt wird, wir diesert Wert angepasst
  }
  //Schaltung für Taster wird beim loslassen aktiviert (Taste lange drücken wird nur einfach gewertet)
  if (((millis() - letzteSchaltungMs[0]) > entprellZeitMs) && (taster_Gedrueckt[taster_Nr] == HIGH)) {
    taster_Gedrueckt[taster_Nr] = LOW;  //Variable zurücksetzen
    tasterAktion(taster_Nr);            //Aktion nach Tastendruck auslösen
  }
}

//_____________________________
//
//* * * Software Schalter * * *                 z.B. zum umwandeln eines Tasters in einen Schalter
//_____________________________

void schalter1() {                                      //einfacher umschalter
  if (schalter_Status[0] == LOW) {
    schalter_Status[0] = HIGH;
  } else {
    schalter_Status[0] = LOW;
  }
}

void schalter2() {                                    //mehrfach Umschalter (verschiedene Modi)
  if (modus < 6) {
    modus++;
  } else {
    modus = 1;
  }
}

//____________________________
//
//* * * Schalter Aktionen * * *
//____________________________

void schalter1_aktion() {                 //es wird zwischen zwei Funktionen umgeschalten
  if (schalter_Status[0] == HIGH) {
    blinker_stabil(intervall);
  } else {
    blinker_instabil(ledOnMs, ledOffMs);
  }
}

//________________________________________________________________
//
//* * * Gleichmässiger Blinker On und Off Zeiten sind gleich * * *
//________________________________________________________________

void blinker_stabil(long wechsel) {             //LED-Blinker on/off Zeiten sind gleich
  aktuelleMs = millis();
  if ((aktuelleMs - letzerWechselMs) > wechsel) {
    letzerWechselMs = aktuelleMs;
    if (led_Status[0] == LOW) {
      led_Status[0] = HIGH;
    } else {
      led_Status[0] = LOW;
    }
    digitalWrite(led_Pin[0], led_Status[0]);
  }
}

//______________________________________________________________________
//
//* * * Ungleichmässiger Blinker On und Off Zeiten unterschiedlich * * *
//______________________________________________________________________

void blinker_instabil(long OnMs, long OffMs) {
  aktuelleMs = millis();
  if (led_Status[0] == HIGH) {
    if ((aktuelleMs - letzerWechselMs) > OnMs) {
      letzerWechselMs = aktuelleMs;
      led_Status[0] = LOW;
    }
  } else {
    if ((aktuelleMs - letzerWechselMs) > OffMs) {
      letzerWechselMs = aktuelleMs;
      led_Status[0] = HIGH;
    }
    digitalWrite(led_Pin[0], led_Status[0]);
  }
}

//_______________________________________________
//
//* * * Unterschiedliche Lichtmodi schalten * * *
//_______________________________________________

void lichtmodi() {
  if (modus == 1) {
    lichtmodus1();
  }
  if (modus == 2) {
    lichtmodus2();
  }
  if (modus == 3) {
    lichtmodus3();
  }
  if (modus == 4) {
    lichtmodus4();
  }
  if (modus == 5) {
    lichtmodus5();
  }
  if (modus == 6) {
    lichtmodus6();
  }
}

//______________________________________
//
//* * * Unterschiedliche Lichtmodi * * *
//______________________________________
void lichtmodus1() {
  digitalWrite(led_Pin[0], LOW);
  led_Status[0] = LOW;
}

void lichtmodus2() {
  digitalWrite(led_Pin[0], HIGH);
  led_Status[0] = HIGH;
}

void lichtmodus3() {
  blinker_stabil(intervall);
}

void lichtmodus4() {
  blinker_instabil(ledOnMs, ledOffMs);
}

void lichtmodus5() {
  blinker_stabil(100);
}

void lichtmodus6() {
  blinker_instabil(800, 200);
}

Ich freue mich auf eure Anregungen :slight_smile:

Freundliche Grüsse aus der Schweiz Erwin

Diese if-Kette würde ich durch ein switch/case-Konstrukt ersetzen.

Gruß Tommy

Hallo Tommy
Vielen Dank für deine Anregung.
Wie du vieleicht gesehen hast, habe ich weiteroben so ein switch/case Konstrukt verwendet, war auch die Idee, zu erfahren, was euch besser gefällt.
Macht es technisch einen unterschied oder ist es rein zum lesen übersichtlicher? Code ist es in beiden Fällen 3 Zeilen

Gruss Erwin

Beim switch/case wird nach dem 1. Treffer aufgehört, bei Deinem if-Konstrukt werden alle geprüft.
Anders wäre es bei

if () {

} else if () {

} ...

Das wird aber schnell unübersichtlich.

Gruß Tommy

aus diesem Grund habe ich einzelne if's verwendet, aber ja stimmt, daran hatte ich gar nicht gedacht, dass so alle Bedingungen geprüft werden.
Ich werde dies sofort anpassen und ein Switch/Case Konstrukt daraus erstellen

Grüsse Erwin

Durchnummerierte Funktionsnamen und Variablennamen in Kombination mit Arrays und Index-Zählern machen das Ganze unnötig kompliziert, finde ich.

Die zwei Funktionen können so nur einmal und nicht gleichzeitig verwendet werden.

... hättest du auch einfacher haben können, und dadurch dass du es so verschachtelt baust, wird es leider nicht wirklich universell verwendbar.

Statt vieler kleiner Arrays würde ich dir empfehlen, objektorientiert zu denken. Ob du dann alle Daten der zwei Taster in zwei getrennten Objekten oder in einem Array aus zwei Objekten hast, ist weniger wichtig.

Wenn dir (durch Arrays) wichtig ist, alle gleichen Elemente gleich zu behandeln,
dann mach das doch auch:

struct Taster { ... };
Taster taster[] {
  {...},
  {...}
};
void loop() {
  for ( Taster t: taster)  t.check(); // oder: tasterCheck(t);
  lichtmodi();
}

Hallo Michael
Vielen Dank für die Inputs.

Das bin ich mir bewusst, es wird ja im Programm auch nur zwischen diesen zwei Funktionen umgeschalten, aus diesem Grund habe ich mich entschieden die gleiche Variable zu verwenden.

Ich hatte das Gefühl, dass ich durch die Auflösung in einzelne Funktionen mehr universelle Lösungen realisieren lassen (mehr Möglichkeiten offen bleiben), da ich dann jede einzelne Funktion oder auch mehrere gleichzeitig aufrufen kann, wenn ich sie gerade brauche.

wie geschrieben, ich bin neu in der Arduino-Welt, Ich habe das Programm mit den Grundfunktionen, welche eigentlich in jeder Programmiersprache ähnlich und gleich sind geschrieben, einfach um mal zu testen ob ich überhaut ein Programm hinkriege, das trotz laufendem Programm noch auf Taster reagiert. Bei vba(Excel, Access) ist dies ziemlich schwierig zu realisieren. Mir geht es wirklich in erster Linie darum ob ich auf dieser Art Grundkonstrukt weitermachen soll oder ob dies komplett überdenken muss. Ich habe auch schon bemerkt dass es bestimmt noch optimierungspotenzial bezüglich Verschachtelung gibt, ich hatte aber teilweise Probleme dass das Programm nicht lief mit weniger Verschachtelung (Weil ich die richtigen Methoden noch nicht kenne). Der Grund dass ich einige Variablen in Arrays angelegt habe, ist dass ich zu einem späteren Zeitpunkt in komplexeren Varianten beispielsweise in Loops verschiedene Elemente nacheinander mit der gleichen Funktion abprüfen/verändern kann.

Diese Methode kenne ich noch nicht, kannst du mir diese näher erklären? Sieht interessant aus.

Grüsse Erwin

@wisi82

wenn das dein "erster" Code ist und macht was er tun soll - na dann Gratuliere.

An ein paar Ecken würde ich da ansetzen.

A.) in den IDE Settings kannst du dir die Ausgabe der Compilier Warnings einschalten. Das bringt dir dann

C:\Daten\myrepository\Arduino\Forum no SVN\sketch_apr03a\sketch_apr03a.ino: In function 'void blinker_stabil(long int)':
C:\Daten\myrepository\Arduino\Forum no SVN\sketch_apr03a\sketch_apr03a.ino:132:38: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if ((aktuelleMs - letzerWechselMs) > wechsel) {
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
C:\Daten\myrepository\Arduino\Forum no SVN\sketch_apr03a\sketch_apr03a.ino: In function 'void blinker_instabil(long int, long int)':
C:\Daten\myrepository\Arduino\Forum no SVN\sketch_apr03a\sketch_apr03a.ino:151:40: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if ((aktuelleMs - letzerWechselMs) > OnMs) {
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
C:\Daten\myrepository\Arduino\Forum no SVN\sketch_apr03a\sketch_apr03a.ino:156:40: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if ((aktuelleMs - letzerWechselMs) > OffMs) {
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~

Das könntest du beheben.

B) du verwendest schon einige Arrays - das ist gut. Dann verstehe ich nicht dass du bei den Funtkionen wieder mit 1,2,3,... durchzählen beginnst. Entweder mach die Funktion wiederverwendbar, oder bennene sie sprechend.

C) es wurde schon genannt: Geh den nächsten Schritt richtung Objektorientierter Programmierung. Z.B. schreib dir eine Blinkled, lege dir eine LED Objekt an and und übergib der Blinkled einfach die neuen Blinkmuster (on/off Zeiten) als Parameter. Ebenso kannst du mit deinen Tasten vorgehen. Das wird den Code drastisch kürzen und verständlicher machen - und noch besser - viel einfacher erweiterbar machen.

D) Der Compiler hat es für dich vermutlich schon wegoptimiert aber für pins brauchst du normalerweise kein int. Da reicht ein uint8_t auch

z.B.:

const uint8_t taster_Pin[] = { 2, 8 };  //Pin Nummern für Taster

oder gleich ein constexpr ... dann brauchst du dich nicht auf den Compiler verlassen sonst bestimmst vorab dass das ein Konstanter Ausdruck ist:

constexpr uint8_t taster_Pin[] = { 2, 8 };  //Pin Nummern für Taster

Du hast gefragt....

const int led_Pin[] = { 13 };       //Pin Nummern für Led

Wenn Du Pindefinitionen machst, dann nimm byte als Variablengröße. Ein int braucht 2 bytes. Und ein Pin kann nicht negativ werden :wink:
Siehe auch nächster Ausschnitt.

Mach sowas nicht:

  pinMode(taster_Pin[0], INPUT_PULLUP);
  pinMode(taster_Pin[1], INPUT_PULLUP);

bzw.

  tasterCheck(0);
  tasterCheck(1);

Wenn Du Aufzählungen hast, dann zähle auch.

const byte tasterNums = sizeof(taster_Pin) / sizeof(taster_Pin[0]);
[...]

  for (byte b = 0; b < tasterNums; b++)
  {
    pinMode(taster_Pin[b], INPUT_PULLUP);
  }

Wenn Du die Pins als byte deklarierst, kannst Du ggfls. auf die Berechnung aus sizeof(Elemente)/sizeof(Element[0] verzichten und nur das einfache sizeof() nehmen :wink:

Hier hat sich ein Fehler eingeschlichen:

  if (taster_Status[taster_Nr] == LOW)
  {
    letzteSchaltungMs[0] = millis();        //solange der Taster gedrückt wird, wir diesert Wert angepasst
    taster_Gedrueckt[taster_Nr] = HIGH;     //solange der Taster gedrückt wird, wir diesert Wert angepasst
  }

Die letzteSchaltungMs muss zum taster passen.
Und im weiteren Verlauf natürlich auch darauf richtig auswerten.

Die Funktion mit zwei if-Abfragen, in der die zweite das selbe macht wie die erste macht es unübersichtlich und rechenintensiv.
Wenn dann reicht ein else if und Du kannst auf die &&-Verknüpfung verzichten.

  if (modus < 6)
  {
    modus++;
  }
  else
  {
    modus = 1;
  }

Das gewöhne Dir garnicht erst an. Wenn, dann bekommst Du es nur schwer wieder weg und Du wirst ständig daran scheitern, wenn Du mit Elementen arbeitest. Schönes Beispiel: Das erste Element in einem Array ist immer das mit der Nummer 0.
Auch funktioniert darum die for-Schleife oben im setup so schön. Du hast 2 Elemente, prüfst aber auf < 2. das erste Element ist 0 das zweite 1.
Wenn Du Zähler nutzt, fange nicht an Deine eigenen Konventionen einzubringen.
Das macht es für Aussenstehende ungleich schwerer den Code dann zu verstehen.

Das hier:

  if ((aktuelleMs - letzerWechselMs) > wechsel)

wirft Dir der Compiler mit einer Warnung vor die Füsse.
Bitte stelle die IDE mal anders ein. (DATEI-Voreinstellungen)

Dann siehst Du, das der Wertebereich nicht passt...

Das:

    if (led_Status[0] == LOW)
    {
      led_Status[0] = HIGH;
    }
    else
    {
      led_Status[0] = LOW;
    }
    digitalWrite(led_Pin[0], led_Status[0]);

geht auch kürzer:

    digitalWrite(led_Pin[0], !digitalRead(led_Pin[0]));

Das zum switch/case spar ich mir.... :slight_smile:

Aber das mit dem status:

void lichtmodus1()
{
  digitalWrite(led_Pin[0], LOW);
  led_Status[0] = LOW;
}

void lichtmodus2()
{
  digitalWrite(led_Pin[0], HIGH);
  led_Status[0] = HIGH;
}

kannst Du vollständig weglassen - an jeder anderen Stelle auch.
Wenn der led_Status[x] die led abbilden soll, dann kannst Du den ledPin abrufen.
Das vermeidet dann auch Missverständnisse... Du veränderst led_Status[0] an den unterschiedlichsten Stellen im Code.
Und es spart eine Variable...

Zu den magischen Zahlen war auch schon gesagt.
Von daher wars für mich erstmal.

ist keine "Methode" sondern der Einstieg ins objektorientierte Denken.
Erstmal ein benutzerdefinierter (zusammengesetzter) Datentyp, dem man im nächsten Schritt auch Methoden mitgeben kann.

Herzlichen Dank, aber nicht denken dass ich das einfach getippt habe und dann hats funktioniert, die meisten Nerven hat es mich gekostet eine Reaktion vom Schalter zu erhalten (Ich dachte schon, dass es an dem Klon liegt, auf keinem Pin habe ich eine reaktion erhalten, ausser Reset (taster funktioniert also) bis ich endlich gefunden hatte dass ich an einer Stelle als Vergleichsoperator nur = anstelle von == verwendet hatte :joy: hatte ich den Code bereit fast auf Null reduziert.

UUUUPS :see_no_evil: unterschiedliche Datentypen, habe ich behoben

Die Idee war dass ich auch die Funktionen in loops ansprechen kann, aber soweit bin ich noch nicht gekommen.

Sobald ich weiss wie ich Objekte anlegen und verwenden kann, werde ich so etwas bestimmt in Betracht ziehen, aber momentan weiss ich von Objekten nur dass es sie gibt, da macht es noch keinen Sinn sowas zu verwenden. Da muss vorher meine Lernkurve noch etwas an Höhe gewinnen :wink:

Ja was den nun? :thinking: Ich habe mich für Byte entschieden. :grinning:

Danke für den Hinweis, «0» durch «taster_Nr» ersetzt

Auch hier gilt:
UUUUPS :see_no_evil: unterschiedliche Datentypen, habe ich behoben

Nein, so wie du es beschrieben hast, setze ich die LED auf den gleichen Status wie sie ihn momentan hat, so wie ich diese Zeile im Zusammenhang mit dem vorhergehenden if verwende, wird die Variable umgeschaltet wenn notwendig und dann anhand der Variable die LED geschalten, oftmals hat also in dieser Zeile die Variable led_Status nicht den gleichen Wert wie !digitalRead(led_Pin[0]) wird aber dann genau mit dieser Zeile angepasst.
Oder macht das ! etwas was ich noch nicht verstehe? :thinking:

Ich dachte es sei besser eine Variable auszulesen als jedes mal den pin abzufragen, wenn dies jedoch keine Rolle spielt, werde ich diese Variabel gerne weglassen, auch im vorherigen Hinweis

Grüsse und herzlichsten Dank für eure Bemühungen
Erwin

JA!
(Ich vermute es mal. Denn sonst würdest Du nicht fragen :slight_smile: )

Ja. Nein.
Du verwendest die Variable um unterschiedliche Aktionen auszulösen und beschreibst die Variable auch an unterschiedlichen Stellen.
Da die Variable bisher IMMER den tatsächlichen Zustand abbilden soll, ist das lesen des Pin zuverlässiger

Dann werde ich mich wahrscheinlich doch früher oder später mal mit objektorientiertem Programmieren beschäftigen, könnte aber gut sein dass der Sommer dazwischen kommt :joy:

Grüsse Erwin

Das muss Dich nicht entmutigen, da sind wir alle schon drüber gestolpert und es ist auch nicht auszuschließen, dass es wieder passiert.
Das den eigenen Code lesen hat einen entscheidenden Nachteil: Der Mensch liest fehlertolerant. D.h. Du liest nicht wirklich das, was dort steht, sondern was Du meinst dass es dort stehen müsste. Das macht die Fehlersuche so mühsam.
Der erste fremde, der vorbei kommt, sieht den Fehler sofort. Das habe ich oft genug erlebt.

Gruß Tommy

1 Like

Bitte entschuldige in diesem Fall meine vorherige Antwort.
Tatsächlich ist mir das ! erst aufgefallen, als ich den Rest der Antwort bereits geschrieben hatte.
Dürfte ich vielleicht fragen, was genau es mit diesem Ausrufezeichen auf sich hat?

Grüsse Erwin

Das ist die Negation.
!true ist false und umgekehrt.
bzw. a = !a; schaltet a um

Gruß Tommy

Hauptsache etwas mit 8 bit. Ich würde uint8_t nehmen, weil das Arduino intern auch verwendet:

void digitalWrite(uint8_t, uint8_t);
int digitalRead(uint8_t);

Keine Bange, das kenne ich ja bereits aus anderen Sprachen, ist da nicht anders :slight_smile:

Das würde ja bedeuten, dass mit dieser einen Zeile einfach zwischen on und off umgeschalten wird, was bedeuten würde, dass ich mir auch die if-Abfrage sparen kann. Ich dachte, dass dies nur die letzte Zeile ersetzen soll. :see_no_evil:

@my_xy_projekt Vielen herzlichen Dank dafür, das wird mir in Zukunft einige Zeilen an Code sparen

Grüsse Erwin

Das war auch der Plan, habe es bloss noch nicht umgesetzt, natürlich wird noch eine schöne Schleife drumgebunden, :ribbon:

1 Like

byte und uint8_t ist dasselbe.
Da gibt es interessantere Themen, wo man diskutieren könnte,was besser ist.

Z.B. ob man tatsächlich,statt einer Variablen mit dem aktuellen Led-Status, besser digitalRead auf einen OUTPUT-Pin machen sollte.

Das hätte ich für eher abwegig gehalten. Aber tatsächlich ist lediglich die Umrechnung der ArduinoPinNummer in das tatsächliche Hardware-Register und Bit der irrelevante Overhead.

Auch von mir, meine Anerkennung für dein "erstes C++ - Arduino-Programm". :+1:

Dass es bei C/C++ bis auf Leerzeichen auf jeden Buchstaben ankommt, hast du ja schon öfters gemerkt. Willkommen!