I2C Wire library clobbering my switch statement

This is on the Mega 2560. After the call to the wire libraries in the attached code, the switch statement doesn't work.

I reported this as a bug and they closed it and told me to post here, so I am. I have verified that this does not happen in if statements.
wfwtestboxtest.zip (3.7 KB)

Where is call to the wire libraries in your code?
What do you mean switch statement doesn't work?

The call to wire libraries is inside of I2CScanner. This is code I got from the web.

I mean the switch statement doesn't seem to get executed.

The problem has nothing to do with the I2C wire library, but is instead caused by declaring and initializing the variable fromAddress inside a case. Enclose the statements in that case within curly brackets to limit the scope of the variable and you will be ok.

The declaration of fromAddress is odd anyway, since it will go out of scope at the end of the switch case, and you never use it anywhere else.

  switch (currentState) {
    case PRESSURE_SENSOR:
      Serial.println("PRESSURE_SENSOR");
      UINT8 fromAddress = i2CScanner.findFirst();
      currentState = DONE;
      break;

    case DONE:
      Serial.println("Yes!!!");
      break;

    default:
      Serial.println("Default");
      break;
  }

Hi, @thomashehl
Welcome to the forum.

To add code please click this link;

Your code will then be in a scrolling window that makes it easy to read.

Thanks... Tom... :smiley: :+1: :coffee: :australia:

  1. Paste your code according to the above recommendations.
  2. Your code works exactly as you wrote it.

In a nutshell, your code does the following

currentState = PRESSURE_SENSOR;
if (currentState == PRESSURE_SENSOR)
  currentState = DONE;

And nothing more.

currentState:20
PRESSURE_SENSOR
currentState:21
Yes!!!
...

Yes, if you use the if statement it works, that's why I posted code with the switch statement, which does not work. You also skipped the read of the I2C component which is what appears to be causing the problem. I would assume if there were a problem with switch statements that don't call I2C, it would have been found long ago.

Also, I was under the impression you could define a variable anywhere and use it. I usually define variables close to their usage as a best practice.

Here is wfwtestboxtest.ino

#include <WB5000-pins.h>
#include <I2cScanner.h>

const int LOOP_DELAY = 1000;
const int SUCCESS_COUNT = 50;

unsigned int dispenseCount = 0;
unsigned int inputCount = 0;
unsigned int processWaterCount = 0;
unsigned int serviceWaterCount = 0;

enum TestState {
    INPUT_TEST, DEGAS_HI_TEST, DEGAS_LO_TEST, TRANSFER_TEST, SERVICE_WATER_TEST, STORAGE_HI_TEST, STORAGE_LO_TEST, // 6
    DISPENSE_ENABLE_TEST, DISPENSE1_TEST, DISPENSE5_TEST, DISPENSE20_TEST, PROCESS_WATER_TEST, // 11
    DISPENSE_LIGHT, DISPENSE_FLOW, INPUT_LIGHT, INPUT_FLOW, PROCESS_WATER_LIGHT, PROCESS_WATER_FLOW, // 17
    SERVICE_WATER_LIGHT, SERVICE_WATER_FLOW, PRESSURE_SENSOR, // 20
    DONE, ERROR_CONDITION
};

TestState currentState;

bool ledOn;
UINT8 ledValue;
bool switchTested;

I2cScanner i2CScanner;

void doSwitch();

void turnLedOff(UINT8 ledId) {
    digitalWrite(ledId, LED_OFF);
    ledOn = false;
}

void turnLedOn(UINT8 ledId) {
    digitalWrite(ledId, LED_ON);
    ledOn = true;
}

void flashLed(UINT8 ledId) {
    if (ledOn) {
        turnLedOff(ledId);
    } else {
        turnLedOn(ledId);
    }
}

void setup() {
    pinMode(DEGAS_TANK_HI_SWITCH, INPUT);
    pinMode(DEGAS_TANK_LO_SWITCH, INPUT);
    pinMode(INPUT_PUMP_SWITCH, INPUT);
    pinMode(TRANSFER_PUMP_SWITCH, INPUT);

    pinMode(DISPENSE_DONE_LED, OUTPUT);
    pinMode(DISPENSE_PROCEED_LED, OUTPUT);
    pinMode(DISPENSE_VALVE, OUTPUT);
    pinMode(INPUT_PUMP, OUTPUT);
    pinMode(INPUT_VALVE, OUTPUT);
    pinMode(PROCESS_WATER_VALVE, OUTPUT);
    pinMode(RECYCLE_VALVE, OUTPUT);
    pinMode(SERVICE_WATER_VALVE, OUTPUT);
    pinMode(TRANSFER_PUMP, OUTPUT);

    pinMode(DISPENSE_FLOWMETER, INPUT);
    attachInterrupt(digitalPinToInterrupt(DISPENSE_FLOWMETER), onDispenseFlowmeter, RISING);

    pinMode(INPUT_FLOWMETER, INPUT);
    attachInterrupt(digitalPinToInterrupt(INPUT_FLOWMETER), onInputFlowmeter, RISING);

    pinMode(PROCESS_WATER_FLOWMETER, INPUT);
    attachInterrupt(digitalPinToInterrupt(PROCESS_WATER_FLOWMETER), onProcessWaterFlowmeter, RISING);

    pinMode(SERVICE_WATER_FLOWMETER, INPUT);
    attachInterrupt(digitalPinToInterrupt(SERVICE_WATER_FLOWMETER), onServiceWaterFlowmeter, RISING);

    Serial.begin(115200);
    while (!Serial) {}

    currentState = PRESSURE_SENSOR;
    switchTested = false;
}

void loop() {
    Serial.print("currentState:");
    Serial.println(currentState);

    switch (currentState) {
        case PRESSURE_SENSOR:
            Serial.println("PRESSURE_SENSOR");
            UINT8 fromAddress = i2CScanner.findFirst();
            currentState = DONE;
            break;

        case DONE:
            Serial.println("Yes!!!");
            break;

        default:
            Serial.println("Default");
            break;
    }

    delay(500);
}

/**
 * check the switch state and operate the LED accordingly
 * @return true if test completed
 */
bool checkSwitchState(const UINT8 switchId, const UINT8 led) {
    bool result = false;

    Serial.print("switchId:");
    Serial.println(switchId);
    UINT8 val = digitalRead(switchId);
    Serial.print("val:");
    Serial.println(val);
    bool switchOn = val == HIGH;
    Serial.print("switchOn:");
    Serial.println(switchOn);
    if (switchOn) {
        digitalWrite(led, HIGH);
        switchTested = true;
    } else {
        digitalWrite(led, LOW);
        if (switchTested) {
            switchTested = false;
            result = true;
        }
    }

    Serial.print("result:");
    Serial.println(result);
    return result;
}

void onDispenseFlowmeter() {
    dispenseCount++;
}

void onInputFlowmeter() {
    inputCount++;
}

void onProcessWaterFlowmeter() {
    processWaterCount++;
}

void onServiceWaterFlowmeter() {
    serviceWaterCount++;
}

Here is I2CScanner.h


#ifndef TEPROGRAMMER_I2CSCANNER_H
#define TEPROGRAMMER_I2CSCANNER_H
#include <Arduino.h>
#include <Wire.h>
#include <mega2560types.h>

class I2cScanner {
    public:
    /**
     * Find the first address on the I2C bus that has a component attached and return it.
     * @return the address or -1 if not found.
     */
    UINT8 findFirst();
    UINT8 checkForI2c();

    void scanPorts() {
        for (UINT8 ix = 0; ix < sizeof(portArray); ix++) {
            Serial.print("Scanning (SDA) - ");
            Serial.println(portMap[ix]);
            Wire.begin(portArray[ix]);
            check_if_exist_I2C();
        }
    }

    private:
    UINT8 portArray[8] = {16, 5, 4, 0, 2, 14, 12, 13};
    char portMap[8][7] = {"GPIO16", "GPIO5", "GPIO4", "GPIO0", "GPIO2", "GPIO14", "GPIO12", "GPIO13"};

    void check_if_exist_I2C() {
        byte error, address;
        int nDevices;
        nDevices = 0;
        for (address = 1; address < 127; address++) {
            // The i2c_scanner uses the return value of
            // the Write.endTransmission to see if
            // a device did acknowledge to the address.
            Wire.beginTransmission(address);
            error = Wire.endTransmission();

            if (error == 0) {
                Serial.print("I2C device found at address 0x");
                if (address < 16)
                    Serial.print("0");
                Serial.print(address, HEX);
                Serial.println("  !");

                nDevices++;
            } else if (error == 4) {
                Serial.print("Unknown error at address 0x");
                if (address < 16)
                    Serial.print("0");
                Serial.println(address, HEX);
            }
        } //for loop
        if (nDevices == 0)
            Serial.println("No I2C devices found");
        else
            Serial.println("**********************************\n");
        //delay(1000);           // wait 1 seconds for next scan, did not find it necessary
    }
};


#endif //TEPROGRAMMER_I2CSCANNER_H

And I2cScanner.cpp

#include "I2cScanner.h"

UINT8 I2cScanner::findFirst() {
    UINT8 result = -1;
    for (int ix = 0; ix < sizeof(portArray); ix++) {
        Serial.print("Scanning (SDA) - ");
        Serial.println(portMap[ix]);
        Wire.begin(portArray[ix]);
        result = checkForI2c();
        if (result >= 0) {
            Serial.print("Found:0x");
            Serial.print(result, HEX);
            Serial.println(" Stopping.");
            break;
        }
    }

    return result;
}

UINT8 I2cScanner::checkForI2c() {
    INT8 result = -1;
    UINT8 error;
    UINT8 address = 16;
    while (address < 127 && result <= 0) {
        // The i2c_scanner uses the return value of
        // the Write.endTransmission to see if
        // a device did acknowledge to the address.
        Wire.beginTransmission(address);
        error = Wire.endTransmission();

        if (error == 0) {
            result = address;
        } else if (error == 4) {
            Serial.print("Unknown error at address 0x");
            if (address < 16) {
                result = 0;
            }
            Serial.println(address, HEX);
        }
        address++;
    } //for loop

    return result;
}

As far as comments about how this doesn't do anything with some of the variables, I have followed best practices and trimmed the program down as much as possible to just show the error I'm receiving.

Thanks for your help.

I do not use if statement, I use your code with case statement.
My "if" operator just describes the behavior of your program. This is pseudocode.
Take a look at the snippet of your program, which I compiled in the Arduino IDE, uploaded to the Arduino board and showed you its output.
The case statement works, it just does nothing except currentState = DONE;

#define PRESSURE_SENSOR 20
#define DONE 21
int currentState = 0;

void setup() {
  Serial.begin(9600);
  while (!Serial) {}
  currentState = PRESSURE_SENSOR;
}

void loop() {
  
  Serial.print("currentState:");
  Serial.println(currentState);

  switch (currentState) {
    case PRESSURE_SENSOR:
      Serial.println("PRESSURE_SENSOR");
      // no sense, fromAddress is not used anywhere
      //UINT8 fromAddress = i2CScanner.findFirst();
      currentState = DONE;
      break;

    case DONE:
      Serial.println("Yes!!!");
      break;
    // unreacable code
    default:
      Serial.println("Default");
      break;
  }
  delay(500);
}

The files "I2cScanner.cpp" and "I2cScanner.h" are not for a Arduino Mega board, they are for the ESP8266 and those files are full of terrible bugs. Please delete those files and don't use them anymore.

It seems to have its origin at instructables with almost no bugs: https://www.instructables.com/ESP8266-I2C-PORT-and-Address-Scanner/.
But that is for a ESP8266, you can't use it for a Arduino Mega.

Your code works because you commented out the initialization of fromAddress, which is what causes the problem. Leave that in and the compiler will give you warnings, and never execute any of the subsequent cases.

#define PRESSURE_SENSOR 20
#define DONE 21
int currentState = 0;

void setup() {
  Serial.begin(9600);
  while (!Serial) {}
  currentState = PRESSURE_SENSOR;
}

void loop() {

  Serial.print("currentState:");
  Serial.println(currentState);

  switch (currentState) {
    case PRESSURE_SENSOR:
      Serial.println("PRESSURE_SENSOR");
      // no sense, fromAddress is not used anywhere
      byte fromAddress = 0; //dummy initialization since the function does not exist
      currentState = DONE;
      break;

    case DONE:
      Serial.println("Yes!!!");
      break;
    // unreacable code
    default:
      Serial.println("Default");
      break;
  }
  delay(500);
}

c++ does not allow a jump into the scope of a variable while bypassing the initialization of that variable. If you declare the variable without initialization, then subsequently assign a value to it, there will not be a problem, or simply enclose the statements within that case in curly brackets to limit the scope of the variable.

/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino: In function 'void loop()':
/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino:2:14: warning: jump to case label [-fpermissive]
 #define DONE 21
              ^
/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino:24:10: note: in expansion of macro 'DONE'
     case DONE:
          ^~~~
/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino:20:12: note:   crosses initialization of 'byte fromAddress'
       byte fromAddress = 0; //dummy initialization since the function does not exist
            ^~~~~~~~~~~
/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino:28:5: warning: jump to case label [-fpermissive]
     default:
     ^~~~~~~
/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino:20:12: note:   crosses initialization of 'byte fromAddress'
       byte fromAddress = 0; //dummy initialization since the function does not exist
            ^~~~~~~~~~~
/home/user/Arduino/sketch_jun10a/sketch_jun10a.ino:20:12: warning: unused variable 'fromAddress' [-Wunused-variable]

Google "c++ switch cross initialization" and you will see that this has been found numerous times.

I don't understand why call a function at all if its result is not used anywhere?

The OP believed that the function call was the cause of the problem, so included it in a minimal sketch to exhibit the effect. The fact that the result is never used makes no difference in this case, but eliminating that one line also eliminates the problem. < edit > Also note that removing the function call, while still declaring the variable without initialization, also eliminates the problem, making the function call suspect.