millis() laufen im Kreis

Hallo miteinand,

ich habe hier eine Relais Schaltung und bekomme die Schleife nicht hin.

const uint8_t OPENER_INPUT_PIN 21 //Wire from small keypad relay, switched to GND while active
const uint8_t OPENER_RELAY_PIN 27 //Switch door opener relay on if LOW

unsigned long opener_checkInterval  = 5000;  //5 seconds in millis
unsigned long opener_lastInterval   = 0;

typedef struct  {
  char relay_name[17] = {'\0'};
  uint8_t relay_pin = 0;	//Relay GPIO
  uint8_t relay_type = 0; 	//0=Active_Low, 1=Active_High, 2=WiFi
  uint8_t relay_state = 0;	//0=off, 1=on
  uint8_t relay_mode = 0;	//0=automatic, 1=manual
} Relay;
Relay opener_relay;

void setup() {
  opener_relay.relay_pin = OPENER_RELAY_PIN;
  pinMode(OPENER_INPUT_PIN, INPUT_PULLUP);
  pinMode(opener_relay.relay_pin, OUTPUT);
  digitalWrite(opener_relay.relay_pin, HIGH);
  opener_relay.relay_state = 0;
}

void loop() {
  ...
 
  //Mini Relais der Steuerung zieht OPENER_INPUT_PIN auf LOW
  //Relais an GPIO27 zieht an

  if (digitalRead(OPENER_INPUT_PIN) == LOW) {

    digitalWrite(opener_relay.relay_pin, LOW);
    opener_relay.relay_state = 1;

    //Im Notfall soll das Relais spätestens nach 5 Sekunden abschalten,
    //um den angeschlossenen Verbraucher nicht zu überlasten
    //danach aber wieder ganz normal zur Verfügung stehen

    if (millis() - opener_lastInterval > opener_checkInterval && (opener_relay.relay_state == 1)) {
        digitalWrite(opener_relay.relay_pin, HIGH);
        opener_relay.relay_state = 0;
        opener_lastInterval = millis();
    }
  }
  else {
    digitalWrite(opener_relay.relay_pin, HIGH);
    opener_relay.relay_state = 0;
  }
}

Die Problemstellung steht als Kommentar im loop(). Ich kriege es einfach nicht hin.

Hallo
Sorry ich kann's mir nicht verkneifen :wink:
Die Antwort zu dem Tittel des Post ist

Ja

Heinz

Hallo,

dein Code kompiliert nicht. Syntaxfehler der globalen Variablen.

Ich weiß nicht was ansonsten mit dem Relais passieren soll.
Habe dir aber ein Bsp. mit 2 Varianten fertig gemacht.
Der Taster wird mittels Funktionen abgehandelt.
Die Led (Relais) mittels Klasse hat ihre eigenen Methoden.

Wenn Taster gedrückt, wird sich die Einschaltzeit gemerkt und die Led eingeschalten.
update() muss ständig aufgerufen werden.
Darin wird gepüft ob die Einschaltdauer abgelaufen ist oder nicht.
Wenn ja, wird die Led automatisch ausgeschalten.
Die Einschaltzeit wird durch Taster halten nicht verlängert.
Zwischendurch geht die Led immer aus.
Beim Taster dauerdrücken sieht man nur nicht das die Led zwangsweise ganz kurz ausgeht.

Verstehen und anwenden. Entweder machst für beide eine Klasse oder machst für beide nur Funktionen.
Im Grunde brauchst du einen nicht retriggerbaren Monoflop. Danach kannste auch googeln.

/*
  Doc_Arduino - german Arduino Forum
  IDE 1.8.13
  avr-gcc 10.2.0
  Arduino Mega2560
  15.12.2020
  License: GNU GPLv3
  https://forum.arduino.cc/index.php?topic=718340.0
*/
  
const byte pinTaster {2};

class AutomaticOff
{
  private:
    const byte pinLed;
    unsigned long onTime;
    unsigned long lastTime;
    bool stateLed;

  public:
    AutomaticOff(byte pin, unsigned long onT) :
      // Initialisierungsliste
      pinLed{pin},
      onTime{onT},
      lastTime{0},
      stateLed{false}
    { }

    void init()
    {
      pinMode(pinLed, OUTPUT);
      digitalWrite(pinLed, stateLed);
    }

    void update()
    {
      if (stateLed && (millis() - lastTime >= onTime) )
      {
        digitalWrite(pinLed, LOW);         // LED ausschalten nach x ms
        lastTime = millis();
        stateLed = LOW;
      }
    }

    void switchLedOn()
    {
      if (!stateLed)                      // Retriggersperre
      {
        digitalWrite(pinLed, HIGH);       // LED einschalten
        lastTime = millis();              // Einschaltzeit merken
        stateLed = HIGH;
      }
    }
};



AutomaticOff led (28, 2000);          // Pin, onTime ms


void setup (void)
{
  pinMode(pinTaster, INPUT_PULLUP);
  led.init();
}

void loop (void)
{
  if (updateTaster(pinTaster))
  {
    led.switchLedOn();
  }

  led.update();
}


// ****** Funktionen ******

bool updateTaster (const byte pin)
{
  static unsigned long lastMillis {0};
  static bool state {false};
  unsigned long ms {millis() };

  if (ms - lastMillis >= 30)
  {
    lastMillis = ms;
    state = !digitalRead(pin);    // liefert true wenn Taster gedrückt
  }

  return state;
}

(deleted)

Wozu das "typedef struct Relay" wenn nur mit einem Relais gearbeitet wird ?

Wofür das C Type typedef da verwendetet wird, frage ich mich auch, das stammt bestimmt aus einem C Buch. Wirklich modernes C++ ist das nicht.

Aber kapseln....
Da gibt es nun wirklich gar nichts gegen zu sagen.
Davon kann man gar nicht genug haben.

freddy64:
ich habe hier eine Relais Schaltung und bekomme die Schleife nicht hin.
.....

Und welche Schleife meinst du genau ?

combie:
Wofür das C Type typedef da verwendetet wird, frage ich mich auch, das stammt bestimmt aus einem C Buch.

combie verwendet lieber using. Da ich als alter C - Hase typedef schon kannte, war ich froh, als ich gelernt hatte, dass er eigentlich dasselbe meint. (Sicher gibt es Unterschiede, und ob mehr Kapseln immer besser sind, haben wir schon paarmal diskutiert :slight_smile: )

  if (digitalRead(OPENER_INPUT_PIN) == LOW) {

digitalWrite(opener_relay.relay_pin, LOW);

Das ist doch genau das, was du nicht (immer) willst. Warum machst du es dann so?

Deine Beschreibung ist auch (für mein Verständnis) etwas verworren:
Nach 5 s soll das Relais auf jeden fall wieder abfallen, ok, aber wie lang?

Sinnvoller wäre, den Flankenwechsel des Tasters als Trigger für das Relais zu nehmen.
Taster losgelassen oder Zeitablauf wäre dann der Trigger fürs Abschalten.

combie verwendet lieber using. Da ich als alter C - Hase typedef schon kannte, war ich froh, als ich gelernt hatte,

In diesem Fall nicht.

Mit using kann man schön einen Typealias bauen. Ist eher kein Ersatz für struct.

Hier würde ich eher sowas machen:

// original
typedef struct  {
  char relay_name[17] = {'\0'};
  uint8_t relay_pin = 0;	//Relay GPIO
  uint8_t relay_type = 0; 	//0=Active_Low, 1=Active_High, 2=WiFi
  uint8_t relay_state = 0;	//0=off, 1=on
  uint8_t relay_mode = 0;	//0=automatic, 1=manual
} Relay;


// C++ Stil
struct Relay {
  char relay_name[17] = {'\0'};
  uint8_t relay_pin = 0;	//Relay GPIO
  uint8_t relay_type = 0; 	//0=Active_Low, 1=Active_High, 2=WiFi
  uint8_t relay_state = 0;	//0=off, 1=on
  uint8_t relay_mode = 0;	//0=automatic, 1=manual
} ;

Ist aber nicht wirklich wichtig, da quasi gleichwertig.
Und dennoch gibt es in C++ kaum noch eine Notwendigkeit typedef einzusetzen.

(Sicher gibt es Unterschiede, und ob mehr Kapseln immer besser sind, haben wir schon paarmal diskutiert :slight_smile: )

Wie man zur verbesserung der Kapselung da Methoden einbaut, muss ich nicht wiederholen, da Doc_Arduino das schon vorgeführt hat.

Mantra:

Fasse zusammen, was zusammen gehört.

// Forumsketch
// angelehnt an Ausgangspost: https://forum.arduino.cc/index.php?topic=718340.msg4826018#msg4826018

/* dort steht:
    //Im Notfall soll das Relais spätestens nach 5 Sekunden abschalten,
    //um den angeschlossenen Verbraucher nicht zu überlasten
    //danach aber wieder ganz normal zur Verfügung stehen
*/

// die PIN sind anzupassen!
const uint8_t OPENER_INPUT_PIN = 2;
const uint8_t OPENER_RELAY_PIN = 13;

unsigned long opener_checkInterval  = 5000;
unsigned long opener_lastInterval   = 0;

bool opener_state_marker = false;

void setup()
{
  pinMode(OPENER_INPUT_PIN, INPUT_PULLUP);
  pinMode(OPENER_RELAY_PIN, OUTPUT);
  digitalWrite(OPENER_RELAY_PIN, HIGH);
}

void loop()
{
  //Mini Relais der Steuerung zieht OPENER_INPUT_PIN auf LOW
  //Relais an OPENER_RELAY_PIN zieht an

  if ((!digitalRead(OPENER_INPUT_PIN)) && (!opener_state_marker))
  {
    digitalWrite(OPENER_RELAY_PIN, LOW);
    opener_state_marker = true;
    opener_lastInterval = millis();
  }
  if ((digitalRead(OPENER_INPUT_PIN)) && (opener_state_marker))
  {
    opener_state_marker = false;
  }
  if (((millis() - opener_lastInterval) > opener_checkInterval) || (!opener_state_marker))
  {
    if (!digitalRead(OPENER_RELAY_PIN))
    {
      digitalWrite(OPENER_RELAY_PIN, HIGH);
    }
  }
}

ohne Kommentare und unkommentiert.
PIN anpassen, einspielen, testen - Feedback geben.

Wozu das "typedef struct Relay" wenn nur mit einem Relais gearbeitet wird ?

Es werden bis zu 8 in einem Array verwendet.
Relay relays[NUM_OF_RELAYS];
NUM_OF_RELAYS ist abhängig vom Relay Board 1,2,4 oder 8

Ist

// C++ Stil
struct Relay {
  char relay_name[17] = {'\0'};
  uint8_t relay_pin = 0;	//Relay GPIO
  uint8_t relay_type = 0; 	//0=Active_Low, 1=Active_High, 2=WiFi
  uint8_t relay_state = 0;	//0=off, 1=on
  uint8_t relay_mode = 0;	//0=automatic, 1=manual
} ;

der richtige C++ code?

Ich lasse mich hier gerne für weitere Anwendungen belehren.

Ist das nur ein Unterschied in der Schreibweise oder arbeitet dadurch das Programm schneller oder zuverlässiger.

Es kommt halt immer wieder noch MSC von 1994 durch.

Ich habe noch nicht alle Posts bis zum Ende durchexerziert.
Wenn meine Lösung dabei ist, werde ich es berichten.

der richtige C++ code?

Nicht ganz.....

Solche Initialisierungen in Strukturen funktionieren nicht unter allen Umständen.
Ist z.B. abhängig von der C++ Version.

Wie schon gesagt:
Doc_Arduino zeigt die etwas bessere Variante.
Mit Konstruktor und Initialisierungsliste und eingeschränkter Sichtbarkeit.

Er geht allerdings noch deutlich weiter, und verankert auch die Funktionen, hier Methoden, in der Klasse.
Klassen und Strukturen sind in C++ identisch, bis auf die default Sichtbarkeitsregeln.

Ist das nur ein Unterschied in der Schreibweise oder arbeitet dadurch das Programm schneller oder zuverlässiger

Erstmal nur in der Schreibweise.

Sicherer und stabiler, wird es dadurch, dass man die die Dinge möglichst einschränkt, so dass der Compiler eine Chance hat, dir den Kram schon beim kompilieren um die Ohren zu hauen.

uint8_t relay_pin = 0; //Relay GPIO
Ich nehme mal an, dass sich der Pin zur Laufzeit nie ändern wird/darf
Also:

private: // oder protected:
const uint8_t relay_pin = 0;  //Relay GPIO

Die Initialisierungsliste kann const Elemente besetzen, bevor die Klasse/Struktur konstruiert wird.

uint8_t relay_type = 0; //0=Active_Low, 1=Active_High, 2=WiFi
3 Zustände

private: // oder protected:
enum RelayType:byte {Active_Low,Active_High,WiFi};
RelayType relay_type = RelayType::Active_Low;

Der Versuch relay_type = 42; zu setzen, wird gnadenlos vom Kompiler bestraft.

uint8_t relay_state = 0; //0=off, 1=on
2 Zustände

private: // oder protected:
bool relay_state = false;  //false=off, true=on

... war jetzt nur so ein Ansatz ....

Allerdings werden auf diese Art viele Programmierfehler, welche sonst schwer zu finden sind, früh gemeldet.
Das ist mehr Schreibarbeit, aber das Programm wird stabiler.

(deleted)