Switch case block completely refuses to execute

I try this code as follows:
I launch it, it works, the state is 0.
I input a variable, the state passes to 1, and executes the code - when I apply the right acceleration, the state passes to 2 (FLY) and the delay is executed. Once the delay finishes, the state passes to 3... and NOTHING happens! Even just printing "hello" and not even doing the commented parts.

I know states.states is equal to 3, as I printed it at the beginning of the loop to check, and it prints 3.
When I made the states.states go to 0 - it works, when I go to 1, it works as expected, and same for 2.
When I make it go to to 4 however (i put a "hello" print in it too) it doesn't do anything either.

The states.states assignation must work, given it changes?
The transition seems to work, if it can go to 0/1/2 but not 3/4.

So what gives? I've gone over this with a friend and myself a bunch and I can't figure anything out!

P.S. The extra function calls seem to all work, which is why I didn't add the code for them: they're all very basic and taken/edited from libraries. I also don't think they're the problem given blocks 0/1/2 work perfectly and the only functions calls in 3/4 are called in other blocks. That, and the fact even if the only code in block 3 is "Serial.println("hello");". it doesn't work.

Also please excuse me for some of the syntax (case 0: in one place, case READY in the other) I've just spent the last 2h30 modifying this to try to find SOME solution.

Thank you so much for any help!

//Includes
#include <Wire.h>
#include <SPI.h>
#include <Adafruit_BMP280.h>
#include <Adafruit_MPU6050.h>
#include <Adafruit_Sensor.h>
#include "Adafruit_EEPROM_I2C.h"

//Defines
#define MinimumHeight 400

//Structures
struct startData {
  float groundAltitude;
  float groundPressure;
  float groundTemperature;
} ;

struct flightData {
  uint32_t time;
  uint32_t heightRate;
  uint32_t height;
} ;

struct stateMachine{
  uint8_t states;
  uint8_t minimumHeight;
} ;

/*
*Function Initializations
*/

//Barometer
void initializeBarometerSleep();
void initializeBarometerNormal();
void printBarometerValues();
float barometerAltitude ();
float barometerHeight ();
void initGroundData();

//Accelerometer
void initializeAccelerometer();
void printAccelerometerValues ();

//Second Event
void setSecondEventCurrentImpulse();
void resetSecondEventCurrentImpulse();
void secondEvent();

//EEPROM
void initializeEEPROM();
void eepromWriteState();
void eepromReadState();
void eepromWritePrimer();
void eepromReadPrimer();
void eepromWriteGroundData();
void eepromReadGroundData();

//Device Initializations
Adafruit_MPU6050 mpu;
Adafruit_BMP280 bmp;
Adafruit_EEPROM_I2C i2ceeprom;
#define EEPROM_ADDR 0x50


//Struct Initalizations
startData groundData;
stateMachine states;
#define FLIGHT_DATA_BUFFER_SIZE 5
flightData flightDataArray[FLIGHT_DATA_BUFFER_SIZE]; 


// States and Counters
enum {SLEEP, AWAKE, FLY, READY, TRIGGERED,};
#define SLEEP 0
#define AWAKE 1
#define FLY 2
#define READY 3
#define TRIGGERED 4

int sleepCounter = 0;
int awakeCounter = 0;
int flyCounter = 0;
int readyCounter = 0;
int triggeredCounter = 0;

//Mach Delay
int machDelayA;
int machDelayB;
int machDelayTotal;

#define machDelay 5000

int Status;

byte ReceivedMessage;

int counter;

void setup() {
  Serial.begin(9600);
  while ( !Serial ) delay(100);
  Wire.begin(0x08);
  Wire.onReceive(AVTransmission);

  states.states = 0;

  initializeBarometerSleep();
  initializeAccelerometer();
  initializeEEPROM();
  delay(100);
  eepromReadState();
  eepromReadPrimer();
  if (states.states != 0) {
    bmp.setSampling(Adafruit_BMP280::MODE_NORMAL,     /* Operating Mode. */
                  Adafruit_BMP280::SAMPLING_X2,     /* Temp. oversampling */
                  Adafruit_BMP280::SAMPLING_X16,    /* Pressure oversampling */
                  Adafruit_BMP280::FILTER_X16,      /* Filtering. */
                  Adafruit_BMP280::STANDBY_MS_500);
  }
  if (states.states == 0) {
      states.states = SLEEP;
  }

  states.states = 3;
}


void loop() {
  // Serial.print("State = ");
  // Serial.println(states.states);
  // delay(1000);
  switch (states.states) {
    case 0:
    //Transition from Sleep to Awake is handled by the AVTransmission function.
    if (Serial.available() > 0) {
      states.states = AWAKE;
      Status = Serial.read();
    }
    delay(10);
      break;
    case AWAKE:
      
      if (awakeCounter == 0) {
        eepromWriteState();
        initializeBarometerNormal();
        mpu.enableSleep(false);
        initGroundData();
        awakeCounter++;

        eepromReadState();
        Serial.print("Pressure = ");
        Serial.println(groundData.groundPressure);
        Serial.print("Altitude = ");
        Serial.println(groundData.groundAltitude);
        Serial.print("Temperature = ");
        Serial.println(groundData.groundTemperature);
        Serial.print("State = ");
        Serial.println(states.states);


      }

      sensors_event_t a;
      mpu.getAccelerometerSensor()->getEvent(&a);
      if (awakeCounter == 1) {
        if (a.acceleration.z >= 0) {
          states.states = FLY;
        }
      }
      Serial.println(a.acceleration.z);

    if (Serial.available() > 0) {
      states.states = FLY;
    }
      break;
    case FLY:
      if (flyCounter == 0) {
        eepromWriteState();
        machDelayA=millis();
        flyCounter++;
        machDelayTotal = 0;
      }
      int height = barometerHeight();
      if (height > MinimumHeight) {
        states.minimumHeight = 1;
        eepromWritePrimer();
      }
      // Separation Mechanism Signal is handle by the AVTransmission function.
      //Mach Delay Implementation in milliseconds - Mach Delay Incompatible With EEPROM in current iteration
      machDelayB = millis();
      machDelayTotal = machDelayB - machDelayA;
      Serial.println(machDelayTotal);
      if (machDelayTotal >= machDelay) {
        states.states = 3;
        Serial.println("blublub");
        Serial.println(states.states);
      }
      break;
    case 3:
      Serial.println("hello");
      // if (readyCounter == 0) {
      //   eepromWriteState();
      //   readyCounter++;
      // }

      // Serial.print("Height Reached = ");
      // Serial.println(states.minimumHeight);
      // Serial.print("Height = ");
      // Serial.println(barometerHeight());

      // secondEvent();
    break;
    case TRIGGERED:
      Serial.println("Hello");
      if (triggeredCounter == 0) {
        eepromWriteState();
        triggeredCounter++;
      }
      break;
  }
  
}
void AVTransmission (int numBytes) {
  if (Wire.available()) {
    ReceivedMessage = Wire.read();
  }
  if (ReceivedMessage == 0x00) {
    states.states = AWAKE;
  }
  if (ReceivedMessage == 0x01) {
    states.states = READY;
  }
}

The only thing sticking out too me is the local var declared in case FLY.
Not suppose to declare vars inside of cases, make it a global or declare outside of switch..

good luck.. ~q

1 Like

In addition to @qubits-us message...

@0xpedro
Declaring the variable inside the switch case is definitely a bad programming style and cause an undefined behaviour.
If you are sure that you need it, enclose the entire case in curly braces. making it a code block:

case FLY:
    {
      if (flyCounter == 0) {
           eepromWriteState();
           machDelayA=millis();
           flyCounter++;
           machDelayTotal = 0;
      }
      int height = barometerHeight();
      if (height > MinimumHeight) {
        states.minimumHeight = 1;
        eepromWritePrimer();
      }
   }

PS you also has a local variable inside the AWARE vase

Pretty sure you don't need/want the final comma.

Completely legal for reasons, unnecessary but harmless.

a7

if you want to show off and use the savant terminology, you can call that a compound statement (but block is accepted too)

:innocent:

understanding the role of statements is important.

It works!!

Thank you so much

1 Like

If you have an enum:

why did you call some cases by name and others by number?

At first I used an enum, and then switched to numbers to store into the EEPROM - with the #defines each enum is also a number.

Technically it's just leftovers of past code tbh, that I took on/off while testing the error

Declaring a local variable inside of a case is not a problem.
Declaring a local variable with an initializer inside a case is.

The scope of the variable is from where it is declared to the end of the switch/case. For every subsequent case after the declaration, the compiler sees a variable that should have an initial value, but the code that sets that initial value has been skipped.

Also, if you switch on compiler warnings in preferences , such problems become visible.

That is no reason to use numbers. As you say, each enum value represents a number so it is valid to do something like

EEPROM.put(address, FLY);

this is correct

The spec states to be more precise that each enumerator is associated with a value of the underlying type so the number of EEPROM bytes used depends on the type of FLY when you cal lput() .

The spec says

the underlying type is an implementation-defined integral type that can represent all enumerator values; this type is not larger than int unless the value of an enumerator cannot fit in an int or unsigned int so here it will be the size of an int, so 2 or 4 bytes.

it would be more optimal to force the underlying type:

enum State : byte {SLEEP, AWAKE, FLY, READY, TRIGGERED};

to fix the underlying type is known and occupies only 1 byte.

You can see that with

enum  {SLEEP_UNTYPED, AWAKE_UNTYPED};
enum : byte {SLEEP_TYPED, AWAKE}_TYPED;

void setup() {
  Serial.begin(115200);
  Serial.println(sizeof SLEEP_UNTYPED);
  Serial.println(sizeof SLEEP_TYPED);
}
void loop() {}

that will print 2 and 1 on a UNO or 4 and 1 on an ESP32

Challenge might happen when reading back because an enum is a formal type and assigning a random value (read from EEPROM as a byte) might fall outside the defined set of values in the type.

The spec states

Values of integer, floating-point, and enumeration types can be converted to any enumeration type by using static_cast. Note that the value after such conversion may not necessarily equal any of the named enumerators defined for the enumeration

on ESP32 if you try to compile

enum  {SLEEP_UNTYPED, AWAKE_UNTYPED} untypedState;
enum : byte {SLEEP_TYPED, AWAKE_TYPED} typedState;

void setup() {
  Serial.begin(115200);
  untypedState = 1;
  typedState = 1;
  Serial.println(untypedState);
  Serial.println(typedState);
}

void loop() {}

you will get a compile error

sketch.ino:8:18: error: invalid conversion from 'int' to '<unnamed enum>' [-fpermissive]
   untypedState = 1;
                  ^
sketch.ino:9:16: error: invalid conversion from 'int' to '<unnamed enum>' [-fpermissive]
   typedState = 1;
                ^

even though 1 is a valid value.

You need to actually static cast the value

enum UNTYPED_t {SLEEP_UNTYPED, AWAKE_UNTYPED} untypedState;
enum TYPED_t : byte {SLEEP_TYPED, AWAKE_TYPED} typedState;

void setup() {
  Serial.begin(115200);
  untypedState = static_cast<UNTYPED_t>(1);
  typedState = static_cast<TYPED_t>(1);
  Serial.println(untypedState);
  Serial.println(typedState);
}

void loop() {}

to remove the error but there is no validation that the value is fit for purpose. The compiler will gladly accept this

enum UNTYPED_t {SLEEP_UNTYPED, AWAKE_UNTYPED} untypedState;
enum TYPED_t : byte {SLEEP_TYPED, AWAKE_TYPED} typedState;

void setup() {
  Serial.begin(115200);
  untypedState = static_cast<UNTYPED_t>(42); // <=== 42 is not a valid value 
  typedState = static_cast<TYPED_t>(42);     // <=== 42 is not a valid value 
  Serial.println(untypedState);
  Serial.println(typedState);
}

void loop() {}

➜ care should be taken when reading back from EEPROM if using an enum. Ideally you would double check if the value you are trying to statically cast into the state is within the correct range.

I just defined them as numbers instead of doing an enum, it worked just as well for me!

Thank you for the information though :slight_smile:

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.