Variablen werden überschrieben (ESP8266/NodeMCU)

Hallo zusammen,

vermutlich eine Newbie-Frage, aber ich komme hier einfach nicht weiter. Nach jedem MQTT-Callback passiert hier etwas merkwürdiges:

Der Wert des jeweiligen Topics wird korrekt zugewiesen, dennoch halten beide Variablen (temperature & pressure) denselben Wert, und danach sind sie leer bzw sogar zerschossen.

Kann mir da jmd weiterhelfen? Bin Anfänger...

Danke & BG!

Ausgabe Serial:

Received message [home-assistant/utils/temperature] 
23.63
Assigning value to temperature...

...LOOP:::
Temperature:
23.63
Pressure:
23.63

Received message [home-assistant/utils/pressure] 
1016.188
Assigning value to pressure...

...LOOP:::
Temperature:
1016.188
Pressure:
1016.188

...LOOP:::
Temperature:

Pressure:


...LOOP:::
Temperature:

Pressure:


...LOOP:::
Temperature:
⸮
Pressure:
⸮

Sketch:

#include <Wire.h>
#include <ESP8266WiFi.h>
#include <PubSubClient.h>


const char* ssid = "XXX";
const char* password = "XXX";

const char* mqtt_auth_user = "XXX";
const char* mqtt_auth_password = "XXX";
const char* mqtt_server = "XXX";
const char* mqtt_topic_temperature = "home-assistant/utils/temperature";
const char* mqtt_topic_pressure = "home-assistant/utils/pressure";
const char* mqtt_client_id = "ESP8266Client772";

long lastMsg = 0;
char msg[50];
int value = 0;

char* temperature = "22.8";
char* pressure = "99";

WiFiClient espClient;
PubSubClient client(espClient);

void setup() {
  Serial.begin(9600);

  setup_wifi();
  client.setServer(mqtt_server, 1883);
  client.setCallback(callback);
}

void loop() {

  if (!client.connected()) {
    reconnect();
  }
  client.loop();

  delay(3000);
  
  Serial.println();
  Serial.println("...LOOP:::");
  Serial.println("Temperature:");
  Serial.println(temperature);

  Serial.println("Pressure:");
  Serial.println(pressure);
}

void setup_wifi() {
  delay(10);

  Serial.println();
  Serial.print("Connecting to ");
  Serial.println(ssid);

  WiFi.begin(ssid, password);

  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }

  Serial.println("");
  Serial.println("WiFi connected");
  Serial.println("IP address: ");
  Serial.println(WiFi.localIP());
}

void reconnect() {
  while (!client.connected()) {
    Serial.print("Attempting MQTT connection...");
    if (client.connect(mqtt_client_id, mqtt_auth_user, mqtt_auth_password)) {
      client.subscribe(mqtt_topic_temperature);
      client.subscribe(mqtt_topic_pressure);
      Serial.println("connected");
    } else {
      Serial.print("failed, rc=");
      Serial.print(client.state());
      Serial.println(" try again in 5 seconds");

      delay(5000);
    }
  }
}

void callback(char* topic, byte* payload, unsigned int length) {
  
  Serial.print("Received message [");
  Serial.print(topic);
  Serial.print("] ");
  char msg[length + 1];
  for (int i = 0; i < length; i++) {
    msg[i] = (char)payload[i];
  }
  Serial.println();
  msg[length] = '\0';
  Serial.println(msg);

  if (strcmp(topic, mqtt_topic_temperature) == 0) {
    temperature = msg;
    Serial.println("Assigning value to temperature...");
  }

  if (strcmp(topic, mqtt_topic_pressure) == 0) {
    pressure = msg;
    Serial.println("Assigning value to pressure...");
  }
}

Hi

Variablen, Die sowohl innerhalb wie auch außerhalb einer ISR benutzt werden, benötigen das Wörtchen volatile.
Damit wird sichergestellt, daß die Variable JEDES MAL aus dem Speicher gelesen und auch geschrieben wird.
Der Zugriff MUSS atomar sein - bei Datentypen, Die nicht in einem Rutsch verarbeitet werden können, müssen Interrupts unterbunden werden, da sich auch während des Auslesen/abspeichern ein Interrupt-Aufruf dazwischen mogeln kann und Der macht ganz eigene Dinge mit den gespeicherten Daten.

Dann nutzt Du msg[] als char-Array.
Mit temperatur=msg solltest Du der Variable temperatur die Adresse der Variable msg[] zugewiesen haben.
Das erklärt auch, warum beide 'Messwerte' identisch sind - der Speicherort von msg[] ist identisch (hier sogar auch, obwohl die Variable bei jedem Aufruf erneut erstellt wird).

Denke, Du willst irgendwas aus dem char-Array auslesen?

MfG

Du kopierst hier nur einen Pointer:

temperature = msg;
pressure = msg;

Und msg ist ausserhalb von void callback() ungültig. Was dann da steht ist reine Glückssache.

Mach an besten aus temperature und pressure ein char-array das groß genug ist und benutze dann strncpy().

Ich danke euch beiden ganz herzlich, läuft nun. Da muss ich mich wohl noch etwas einlesen wie ich feststellen muss.
Macht alles Sinn was ihr schreibt. & ja, es soll msg (Payload) weiterverarbeitet werden, im konkreten Fall Ausgabe auf kleinem TFT, jeweils Luftdruck & Temperatur.

Danke für die schnelle Hilfe & BG!

Hallo,

in Sachen Interrupts bin ich noch nicht so bewandert. Was ich bis jetzt darüber gelesen habe ist:
Die Abarbeitung der Interrupt-routine so schnell wie möglich halten. Eine callback-funktion wird - wenn ich das richtig verstehe - auch von einem Interrupt aus aufgerufen? Ist das so? Es sieht ja schon ein bisschen anders aus wie ein AttachInterrupt.....

Wenn es denn interrupt ausgelöst ist. Wäre es dann besser in der Callback-function nur die Daten wegzuspeichern und ein Flag zu setzen "neueDatenEingetroffen" und dann in der loop die weitere Datenverarbeitung und serielle Ausgabe usw. zu machen?

viele Grüße Stefan

Callback != ISR

Gruß Tommy

Naja du willst ISRs so kurz wie möglich halten, da in einem ISR alles andere steht, wie z.B. millis() werden nicht weitergezählt.

Da callback() vermutlich durch client.loop() aufgerufen wird, dürfte das hier egal sein.

Aber kommen wir mal zum delay() im loop()... das blockiert und verzögert damit den Aufruf von client.loop()...

Eine callback-funktion wird - wenn ich das richtig verstehe - auch von einem Interrupt aus aufgerufen? Ist das so?

Nööö ....
Kann in Ausnahmefällen so sein.
Ist aber hier nicht der Fall.

Hier ist das Problem, dass Zeiger kopiert werden, aber der Speicherplatz auf dem sie zeigen nach der Funktion verfällt und sicherlich auch wieder überschreiben wird.
Drum steht da auch Müll an den Stellen.

Der klassische Zeiger, welcher in die Wiese zeigt.
Mit den ebenso üblichen Effekten.

achja das liebe delay(). Das wird im Forum so lange immer wieder neu als Problem auftauchen bis sich die
Leute die festlegen was in den Beispielen für Befehle drinstehen dazu durchringen den Befehl delay() durch etwas nicht-blockierendes zu ersetzen.

In dieser Code-version ist das delay durch eine nicht blockierende Timing-Funktion ersetzt.
Es wird auch nur alle 3 Sekunden einmal etwas ausgegeben aber die loop-Schleife wird nicht drei Sekunden lang angehalten

hier die Variablen die benötigt werden

unsigned long startMillis;  
unsigned long currentMillis;
const unsigned long period = 3000;

hier der geänderte Ausschnitt:

  currentMillis = millis();  //get the current "time" (actually the number of milliseconds since the program started)
  if (currentMillis - startMillis >= period)  //test whether the period has elapsed
  {
    Serial.println();
    Serial.println("...LOOP:::");
    Serial.println("Temperature:");
    Serial.println(temperature);
  
    Serial.println("Pressure:");
    Serial.println(pressure);
    startMillis = currentMillis;  //IMPORTANT to save the start time of the last serial output
  }

hier das komplette Programm

#include <Wire.h>
#include <ESP8266WiFi.h>
#include <PubSubClient.h>


const char* ssid = "XXX";
const char* password = "XXX";

const char* mqtt_auth_user = "XXX";
const char* mqtt_auth_password = "XXX";
const char* mqtt_server = "XXX";
const char* mqtt_topic_temperature = "home-assistant/utils/temperature";
const char* mqtt_topic_pressure = "home-assistant/utils/pressure";
const char* mqtt_client_id = "ESP8266Client772";

long lastMsg = 0;
char msg[50];
int value = 0;

char* temperature = "22.8";
char* pressure = "99";

unsigned long startMillis;  
unsigned long currentMillis;
const unsigned long period = 3000; 


WiFiClient espClient;
PubSubClient client(espClient);

void setup() {
  Serial.begin(9600);

  setup_wifi();
  client.setServer(mqtt_server, 1883);
  client.setCallback(callback);
}

void loop() {

  if (!client.connected()) {
    reconnect();
  }
  client.loop();

  currentMillis = millis();  //get the current "time" (actually the number of milliseconds since the program started)
  if (currentMillis - startMillis >= period)  //test whether the period has elapsed
  {
    Serial.println();
    Serial.println("...LOOP:::");
    Serial.println("Temperature:");
    Serial.println(temperature);
  
    Serial.println("Pressure:");
    Serial.println(pressure);
    startMillis = currentMillis;  //IMPORTANT to save the start time of the last serial output
  } 
}

void setup_wifi() {
  delay(10);

  Serial.println();
  Serial.print("Connecting to ");
  Serial.println(ssid);

  WiFi.begin(ssid, password);

  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }

  Serial.println("");
  Serial.println("WiFi connected");
  Serial.println("IP address: ");
  Serial.println(WiFi.localIP());
}

void reconnect() {
  while (!client.connected()) {
    Serial.print("Attempting MQTT connection...");
    if (client.connect(mqtt_client_id, mqtt_auth_user, mqtt_auth_password)) {
      client.subscribe(mqtt_topic_temperature);
      client.subscribe(mqtt_topic_pressure);
      Serial.println("connected");
    } else {
      Serial.print("failed, rc=");
      Serial.print(client.state());
      Serial.println(" try again in 5 seconds");

      delay(5000);
    }
  }
}

void callback(char* topic, byte* payload, unsigned int length) {
 
  Serial.print("Received message [");
  Serial.print(topic);
  Serial.print("] ");
  char msg[length + 1];
  for (int i = 0; i < length; i++) {
    msg[i] = (char)payload[i];
  }
  Serial.println();
  msg[length] = '\0';
  Serial.println(msg);

  if (strcmp(topic, mqtt_topic_temperature) == 0) {
    temperature = msg;
    Serial.println("Assigning value to temperature...");
  }

  if (strcmp(topic, mqtt_topic_pressure) == 0) {
    pressure = msg;
    Serial.println("Assigning value to pressure...");
  }
}

hier kann man nachlesen wie das funktioniert:

Using millis() for timing. A beginners guide

viele Grüße Stefan

Hallo Tommy,
CodeLineUnderstood = true;

Hallo Combie,
ich danke dir fürs erklären. Wieder was dazugelernt.

Die Daten sind ASCII-codes. Ich habe heute die SafeString-library (für mich) entdeckt.
SafeString

Wie würde man jetzt am besten die Daten die von der Callback-funktion angeliefert werden in einen SafeString kopieren?
Die Definition der Callback hat diese Parameter:

(void callback(char* topic, byte* payload, unsigned int length) {

würde da

MySafeString.print(topic);

funktionieren?
viele Grüße Stefan

StefanL38:
achja das liebe delay(). Das wird im Forum so lange immer wieder neu als Problem auftauchen bis sich die
Leute die festlegen was in den Beispielen für Befehle drinstehen dazu durchringen den Befehl delay() durch etwas nicht-blockierendes zu ersetzen.

Naja, wenn du die Wahl zwischen einem auf das wesentliche reduzierten 4-Zeiler hast oder auch gleich eine Statemachine miterklären musst, was machst du?

Ich habe nix gegen delay().
Tipp: Es gibt auch ein Leben neben delay()

Habe auch mittlerweile gelernt den Begriff "Blockadefreies Programmieren" unangebracht zu finden.
(die Begründung habe ich hier sicherlich schon ein paar mal runter gebetet)

Selbst Schleifen, gern auch Endlosschleifen, tun mir nicht mehr weh.

Rintin:
Naja, wenn du die Wahl zwischen einem auf das wesentliche reduzierten 4-Zeiler hast oder auch gleich eine Statemachine miterklären musst, was machst du?

Mir Mühe geben es für Newbees mit einer Alternative die einfacher ist als eine statemachine zu erklären.
Bei so Standardbefehlen wie digitalWrite wird auch nicht erklärt was da im Hintergrund für Portregister gesetzt werden.
In der Arduino-IDE gibt es Arduino-spezifische Sachen. Also könnte man auch einen Standardbefehl für nicht-blockierendes bedingtes Aufrufen von Code definieren.
Was ich auf die Schnelle coden würde ist:
ist dieses Beispiel

unsigned long DemoTimerA = 0; // variables that are used to store timeInformation
unsigned long DemoTimerB = 0; 
unsigned long DemoTimerC = 0; 
unsigned long DoDelayTimer = 0; 

boolean TimePeriodIsOver (unsigned long &expireTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - expireTime >= TimePeriod )
  {
    expireTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  } 
  else return false;            // not expired
}

void setup() 
{
  Serial.begin(115200);
  Serial.println("Program started");
}

void loop() 
{
  if (  TimePeriodIsOver(DemoTimerA,1000)  )
    {
      Serial.println(" TimerA overDue");
    }
       
  if (  TimePeriodIsOver(DemoTimerB,2000)  )
    {
      Serial.println("                TimerB overDue");
    }

  if (  TimePeriodIsOver(DemoTimerC,3000)  )
    {
      Serial.println("                               TimerC overDue");
    }

  if (  TimePeriodIsOver(DoDelayTimer,20000)  )
    {
      Serial.println("every 20 seconds execute delay(3500)... to make all other timers overdue");
      delay(3500);
    }    
}

Die function

boolean TimePeriodIsOver (unsigned long &expireTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - expireTime >= TimePeriod )
  {
    expireTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  } 
  else return false;            // not expired
}

wird einfach per copy&paste eingefügt. Damit hat man die Stolperfalle unsigned long **&**expireTime
im Griff. Und der Aufruf ist dann fast so schnell geschrieben wie "delay()"

Und im blink-Beispiel genauso

boolean TimePeriodIsOver (unsigned long &expireTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - expireTime >= TimePeriod )
  {
    expireTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  } 
  else return false;            // not expired
}

unsigned long BlinkTimer = 0; // variables that are used to store timeInformation
const int OnBoardLED = 13;
int LEDState = LOW;

void setup() 
{
  pinMode(OnBoardLED, OUTPUT);
  digitalWrite(OnBoardLED, HIGH);
  Serial.begin(115200);
  Serial.println("Program started");
}

void loop() 
{
  if (  TimePeriodIsOver(BlinkTimer,500)  )
    {
      if (LEDState == LOW) 
        { LEDState = HIGH; }

      else 
        { LEDState = LOW; }
        
    digitalWrite(OnBoardLED,LEDState);
    }
}

viele Grüße Stefan

#include <INTERVAL.h>

const byte LED = 13;

void setup() 
{
  pinMode(LED,OUTPUT);
}

void loop() 
{
   INTERVAL(500UL)  // ms
   {
     digitalWrite(LED,!digitalRead(LED));
   }
}