Going round in circles trying to understand whats wrong

Hi all

I'm going around in circles trying to understand why a variable changes unexpectedly.

I have some code that sends data to a remote wi-fi Arduino device. Its copied form other projects online.
I am busy changing it ready to use to for DMX.
I have added loads of serial prints so I can try to understand why the variable changes.

If you look at the serial output, the value of opMode is 7 in setup routine.
Then before the If its still 7.
Then inside the else, its suddenly 0.

I just cannot understand why it changes and what i am doing wrong.

Thanks in advance for advice and suggestions, even if its pointing out I'm being stupid :slight_smile:

The point of having op_Mode is that this will be the value sent to the remote device.
the old_opMode value is so that I can check if this value has already been sent on the previous loop and not send it again. The original code would keep re-sending the same value which just causes unnecessary traffic.

failed_loop.ino (4.39 KB)

  if (opMode = old_opMode){ //if loop has run before, do nothing.

The assignment operator is NOT the correct operator to use here.

You’re a star!

I will try it again tomorrow. Sometimes it’s the obvious things you miss. That explains everything.

I've made the changes and was very hopeful, however I am still getting issues.
old_opMode is somewhere being reset to zero.

It runs through loop once, correctly. Then it runs the routine the first time (case 7), then it sets old_opMode = opMode. Sends this to the serial port so I know its correct.

Then the loop runs again, and somehow old_opMode is back at zero...

What have I still missed?

#include <FastLED.h>
#include <ESP8266WiFi.h>
#include <WiFiUDP.h>
#include "reactive_common.h"

#define BRIGHTNESS          50 //up to 255

#define READ_PIN 0
#define BUTTON_PIN D1

#define NUMBER_OF_CLIENTS 1

const int checkDelay = 5000;
const int buttonDoubleTapDelay = 200;
const int numOpModes = 11;

unsigned long lastChecked;
unsigned long buttonChecked;
// bool buttonClicked = false;
bool queueDouble = false;
// bool clickTrigger;
// bool doubleTapped;
WiFiUDP UDP;

struct led_command {
  uint8_t opmode;
  uint32_t data;
};

bool heartbeats[NUMBER_OF_CLIENTS];

const int opMode = 7;
const int old_opMode = 0;


void setup()
{
  pinMode(READ_PIN, INPUT);
  pinMode(BUTTON_PIN, INPUT );

  /* WiFi Part */
  Serial.begin(115200);
  Serial.println();
  Serial.println();
  Serial.print("------------------------------------------------");
  Serial.println();
  Serial.print("Setting soft-AP ... ");
  WiFi.persistent(false);
  WiFi.mode(WIFI_AP);
  WiFi.softAP("sound_reactive", "123456789");
  Serial.print("Soft-AP IP address = ");
  Serial.println(WiFi.softAPIP());
  UDP.begin(7171); 
  resetHeartBeats();
  waitForConnections(); //wait here until we have a connection, the resume
  lastChecked = millis();
  buttonChecked = 0;

    //Serial.print("In Setup: opMode = ");
    //Serial.print(opMode);
    //Serial.print(",  old_opMode = ");
    //Serial.print(old_opMode);
    //Serial.println();
    //delay(5000);
}


void loop()
{
  uint32_t analogRaw;
  
  if (millis() - lastChecked > checkDelay) {
    if (!checkHeartBeats()) {
      waitForConnections();
    }
    lastChecked = millis();
  }
  
    Serial.print("BEFORE IF: opMode = ");
    Serial.print(opMode);
    Serial.print(",  old_opMode - ");
    Serial.print(old_opMode);
    Serial.println();
    delay(5000);

  if (opMode != old_opMode){ //if loop has not run before

    Serial.print("Inside else: opMode = ");
    Serial.print(opMode);
    Serial.print(",  old_opMode - ");
    Serial.print(old_opMode);
    Serial.println();
    delay(5000);
    
    switch (opMode) {
      case 1:
        analogRaw = analogRead(READ_PIN);
        if (analogRaw <= 3)
          break;
        sendLedData(analogRaw, opMode);
        break;
      
      case 2:
        sendLedData(0, opMode);
        delay(10);
        break;
      
      case 3:
        sendLedData(0, opMode);
        delay(10);
        break;
        
      case 4 ... 11:
        Serial.print("Sending Brightness = ");
        Serial.print(BRIGHTNESS);
        Serial.print(",  Sending opMode = ");
        Serial.print(opMode);
        Serial.print(",  old_opMode - ");
        Serial.print(old_opMode);
        sendLedData(BRIGHTNESS, opMode);
        delay(1000); //was 10
        int old_opMode = opMode;
        Serial.println();
        Serial.print("old_opMode  after reset - ");
        Serial.print(old_opMode);
        Serial.println();
        Serial.println();
        break;
    }
    
    delay(4);
  }
 
}


void sendLedData(uint32_t data, uint8_t op_mode) 
{
 struct led_command send_data;
 send_data.opmode = op_mode; 
 send_data.data = data; 
 for (int i = 0; i < NUMBER_OF_CLIENTS; i++) 
 {
    IPAddress ip(192,168,4,2 + i);
    UDP.beginPacket(ip, 7001); 
    UDP.write((char*)&send_data,sizeof(struct led_command));
    UDP.endPacket();
 }
}

void waitForConnections() {
  while(true) {
      readHeartBeat();
      if (checkHeartBeats()) {
        return;
      }
      delay(checkDelay);
      resetHeartBeats();
  }
}

void resetHeartBeats() {
  for (int i = 0; i < NUMBER_OF_CLIENTS; i++) {
   heartbeats[i] = false;
  }
}

void readHeartBeat() {
  struct heartbeat_message hbm;
 while(true) {
  int packetSize = UDP.parsePacket();
  if (!packetSize) {
    break;
  }
  UDP.read((char *)&hbm, sizeof(struct heartbeat_message));
  if (hbm.client_id > NUMBER_OF_CLIENTS) {
    Serial.println("Error: invalid client_id received");
    continue;
  }
  heartbeats[hbm.client_id - 1] = true;
 }
}

bool checkHeartBeats() {
  for (int i = 0; i < NUMBER_OF_CLIENTS; i++) {
   if (!heartbeats[i]) {
    return false;
   }
  }
  resetHeartBeats();
  return true;
}


//void buttonCheck()
//{
//  int but = digitalRead(BUTTON_PIN);
//  if (but == 0) {
//    if (millis() - buttonChecked < buttonDoubleTapDelay && buttonClicked == false ) {
//      doubleClicked();
//      doubleTapped = true;
//    }
//    clickTrigger = true;
//    buttonClicked = true; 
//    buttonChecked = millis();
//  }
//
//  else if (but == 1) {
//    if (millis() - buttonChecked > buttonDoubleTapDelay && clickTrigger) {
//      if (!doubleTapped) {
//        clicked();
//      }
//      clickTrigger = false;
//      doubleTapped = false;
//    }
//    buttonClicked = false;
//  }
//}

//void clicked() {
//  if (opMode == numOpModes)
//    opMode = 1;
//  else
//    opMode++;
//  Serial.printf("Setting opmode %d \n", opMode);
//}

//void doubleClicked() {
//
//}

PT_led_master.ino (5.02 KB)

You missed posting your code, in code tags.

I attached it as a file, how do I embed it in the post?

You click on the "</>" icon and paste your code between the tags.

Added code above now, thanks.

You have a scope issue.

This “variable” old_opMode is ALWAYS 0 (it’s a const):

const int old_opMode = 0;

This redefinition of the scope of the same variable name is a real variable:

int old_opMode = opMode;

However, note that because you have redeclared it (by proceeding with the type name “int”) it is a completely different copy of the variable with its own storage.

It’s “scope” (where it is seen and used from) exists only within the code block (inside the { }) where it is defined. Outside that any reference to “old_opMode” accesses the global const version (always 0).

  1. If you want to change the global old_opMode remove the “int” and “const“ from the above code snippets.
  2. Never ever declare global and local variables with the same names. C allows it and understands it, but from a humans point of View it results in a huge amount of potential misunderstanding.

You're a star!

I'm still learning the programming and totally misunderstood that.
I didn't want 2 different variables, I just wanted the same one used everywhere.

Many, many thanks for your help :slight_smile: :slight_smile:
All working as expected!