With the following code, both ''second-layer''-else statements will get ignored. If xval1 < 500 is TRUE, then regardless of the xval3 value it will always print ''status: ping''. If xval1 >= 500 is TRUE, it will always print ''status: tilt alert2''. I suspect it has to do something with the break statements, but changing it up doesn't seem to help at all.
case DEFINE:
if (xval1 < 500) {
if (xval3 < 50 && xval3 > -50) {
strncpy(eventStatus, "Ping", sizeof(eventStatus)-1);
Serial.println("status: ping");
state = PUBLISH;
break;
}
else {
strncpy(eventStatus, "Tilt alert", sizeof(eventStatus)-1);
Serial.println("status: tilt alert1");
state = PUBLISH;
break;
}
}
else {
if (xval3 < 50 && xval3 > -50) {
strncpy(eventStatus, "Tilt alert", sizeof(eventStatus)-1);
Serial.println("status: tilt alert2");
state = PUBLISH;
break;
}
else {
strncpy(eventStatus, "Collection", sizeof(eventStatus)-1);
Serial.println("status: collection");
state = PUBLISH;
break;
}
}
it’s part of a switch(state). Basically I’m gathering some data from a sensor, and then based on these values I want to attach a certain status to it for a client. Code works without issues, but it assigns the wrong status repeatedly.
Tested without any break statements at all. Still doesnt work.
In C/C++, the switch case statement has “fall-through” behavior–that is after one case is done, the next case in line will be executed unless you end each case section with a break statement.
I would have written this code slightly differently since every path out of the case needs a break, I would just put a break statement after the if/then/else in each case.
@Vitesze using abs(), the absolute value returned will never be negative, by definition. As such, your if (xval3 < 50 && xval3 > -50) may not be doing what you might want it to do.
Have you actually printed out xval3? You would have seen that it never went negative. Have you verified that the values of xval1 and xval2 are valid?
I print out all values and they always seem accurate. Yeah I suppose I didn’t think clearly when I did the -50 part, as I added in abs() just for the sake of not having to deal with negative values. Shouldn’t cause any issues though
When printing just now i got:
xval1 = 38
xval2 = 975
xval3 = 937
Result is status: ping
While it should be tilt alert1. I’m not sure what’s wrong; xval1,2,3 are all defined as integers and printing values goes fine
First take a measurement with the accelerometer, which will be xval2. Then calculate the difference between xval1 and 2, and call it xval3 (which is always positive).
After that, depending on xval1 and 3 values, it should receive one of four statusses.
case ANGLE:
if (xval2 == 0) {
xl.readXYZTData(XValue, YValue, ZValue, Temperature);
xval2 = (XValue);
Serial.print("xval2=");
Serial.println(xval2);
break;
}
else {
int xval3 = std::abs(xval1-xval2);
Serial.print("xval3=");
Serial.println(xval3);
state = DEFINE;
break;
}
case DEFINE:
if (xval1 < 500) {
if (xval3 < 50) {
strncpy(eventStatus, "Ping", sizeof(eventStatus)-1);
Serial.println("status: ping");
state = PUBLISH;
break;
}
else {
strncpy(eventStatus, "Tilt alert", sizeof(eventStatus)-1);
Serial.println("status: tilt alert1");
state = PUBLISH;
break;
}
}
else {
if (xval3 < 50) {
strncpy(eventStatus, "Tilt alert", sizeof(eventStatus)-1);
Serial.println("status: tilt alert2");
state = PUBLISH;
break;
}
else {
strncpy(eventStatus, "Collection", sizeof(eventStatus)-1);
Serial.println("status: collection");
state = PUBLISH;
break;
}
}
break;
Well that looks weird. You are defining xval3 inside the else clause curly braces, so that is not available to the other clauses. Is there another declaration for xval3 somewhere?
The value of xval3 you set up in the ELSE of case ANGLE is not available to the IF statements in case DEFINE.
Maybe you didn't mean to redefine it and should remove the int part?
Really? I didn’t know about that.There is no declaration of xval3 anywhere else, but I figured that defining it in an ELSE clause should still make it available to the other cases? How would I be declaring xval2 and 3 here then? Without any if-else statements?
Just declare it above where xval1 and 2 are declared and make sure it has the value you want (via assignment like xval3 = std::abs(xval1-xval2); or whatever) before you use it.
Just on an unrelated topic, is there any harm in adding a lot of cases in a switch(state)? Should they be made as compact as possible or is it totally fine to have as many as you want (provided the code is ok)?
There is no harm in added cases to a switch statement. It gets hard to read and maintain after about 7 or 8 for me. If you are making state machines (which is what I use switch/case for) then you can think about having a function for each case instead of inline code and have those functions return the next state you want to go to and do what ever other work needed to be done:
switch (currentState) {
case RESET:
nextState = ResetFunction();
break;
case FIRSTTHING:
nextState = FirstThingFunction();
break;
...
}
currentState = nextState;
xval3 wasn’t preserved because when you hit your DEFINE case, you were in a new loop, and it didn’t get defined.
When you are in the ANGLE case, once you hit a break statement, you completely exit the switch{}. I think your mistake might have been thinking that since you just reset the state to DEFINE, that it would just hit the case DEFINE immediately after and execute from there. But that’s not how switch/case/break works.