Personally I don't like the way you handle the "state machine", and I'd implement it in what I perceive is a more robust way (I may be wrong, though).
Moreover if you have a button please add a debounce; I usually do this with the Bounce2 library (or alternatives I code which are modified versions of that library). So... Let's change the things in the way I prefer.
// Include the library
#include <Bounce2.h>
// declare a bounce object
Bounce buttonDebounce = Bounce();
//in the setup:
pinMode(IN_PIN, INPUT); // or INPUT_PULLUP?
buttonDebounce.attach(IN_PIN);
buttonDebounce.interval(5);
//in the loop:
buttonDebounce.update();
Then in the loop you can use the buttonDebounce.rose() function or buttonDebounce.fell() to detect the button presses.
Now, for the state machine. You have four meaningful states (let's call them DISPLAY_1 to 4). It's better to give them more meaningful names. And let's add an invalid state. I usually prefer to use const bytes to force the size rather than enums or defines, but.. whatever ;)
So outside loop or setup declare the values and the state variable:
const byte SM_ErrorState = 0;
const byte SM_Display_1 = 1;
const byte SM_Display_2 = 2;
const byte SM_Display_3 = 3;
const byte SM_Display_4 = 4;
const byte SM_Display_5 = 5;
byte SM_currentState;
Now, the function buttons can be implemented in the classical way I learned to implement state machines, which is first make a switch case to test whether you have to advance, then advance to the next state if necessary and perform the entry action.
void buttons()
{
buttonDebounce.update();
nextState = SM_currentState;
// Check if a new state is requested
switch (SM_currentState)
{
case SM_Display_1:
if (buttonDebounce.rose())
nextState = SM_Display_2;
break;
case SM_Display_2:
if (buttonDebounce.rose())
nextState = SM_Display_3;
break;
case SM_Display_3:
if (buttonDebounce.rose())
nextState = SM_Display_4;
break;
case SM_Display_4:
if (buttonDebounce.rose())
nextState = SM_Display_5;
break;
case SM_Display_5:
if (buttonDebounce.rose())
nextState = SM_Display_1;
break;
default:
nextState = SM_Display_1;
break;
}
// If an advancement is requested, advance and then perform the entry action
if (nextState != SM_currentState)
{
SM_currentState = nextState;
switch (SM_currentState)
{
case SM_Display_1:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Sensor 1 reads");
lcd.setCursor(0,1);
lcd.print(value1);
delay(20);
Serial. print("Case 0");
break;
case SM_Display_2:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Sensor 2 reads");
lcd.setCursor(0,1);
lcd.print(value2);
delay(10);
break;
case SM_Display_3:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Pump 1 ran");
lcd.setCursor(0,1);
lcd.setCursor(5,1);
lcd.print(runs1);
lcd.print(" times");
delay(10);
break;
case SM_Display_4:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Pump 2 ran");
lcd.setCursor(0,1);
lcd.print(runs2);
lcd.setCursor(5,1);;
lcd.print(" times");
delay(10);
break;
case SM_Display_5:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Next cycle in");
lcd.setCursor(0,1);
lcd.print(currentTime);
lcd.setCursor(5,1);
lcd.print(" Minutes");
delay(10);
break;
default:
break;
}
}
}
Now, the initialization. Remember that I told you to add an error state? This way you can force a state change on initialization by setting the variable to the SM_ErrorState value.
Then, the millis management in the loop. I'd avoid the particular case of the variable equal to 0 to trigger an immediate execution, since this will lead to errors later. Then I also prefer to sum the interval, to avoid having a drift. So I changed your way to another one. I also changed some code for better readability (for instance, letting the "timer" reset every minute rather than every 30 minutes, and changing the defines accordingly).
Here is my complete code; I left out the measure function since it does not impact the other code.
Let me know if this solves your problem. Oh, BTW, UNTESTED CODE, so some bugs may appear...
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <Bounce2.h>
LiquidCrystal_I2C lcd(0x3F, 16, 2);
int value1; // I think it is better if these
int value2; // two variables are initialized
int runs1 = 0; // Unsure if this is needed
int runs2 = 0; // Unsure if this is needed
int val = 0; // Unsure if this is needed
int inPin = 6;
unsigned long previousMillis = 0;
const unsigned int millisInOneMinute = 60000U;
byte elapsedMinutes;
const byte intervalMinutes = 30;
Bounce buttonDebounce = Bounce();
void setup() {
Serial.begin(9600);
lcd.begin();
pinMode(inPin, INPUT);
buttonDebounce.attach(inPin);
buttonDebounce.interval(5);
previousMillis = millis();
elapsedMinutes = intervalMinutes; // Force a new measure
}
void loop() {
if (((unsigned int)(millis() - previousMillis)) >= millisInOneMinute)
{
previousMillis += millisInOneMinute; // Avoid drifting
elapsedMinutes++;
}
if (elapsedMinutes >= intervalMinutes)
{
elapsedMinutes = 0;
measure();
}
buttons();
}
void measure() {
// do your stuff
}
void buttons()
{
buttonDebounce.update();
nextState = SM_currentState;
switch (SM_currentState)
{
case SM_Display_1:
if (buttonDebounce.rose())
nextState = SM_Display_2;
break;
case SM_Display_2:
if (buttonDebounce.rose())
nextState = SM_Display_3;
break;
case SM_Display_3:
if (buttonDebounce.rose())
nextState = SM_Display_4;
break;
case SM_Display_4:
if (buttonDebounce.rose())
nextState = SM_Display_5;
break;
case SM_Display_5:
if (buttonDebounce.rose())
nextState = SM_Display_1;
break;
default:
nextState = SM_Display_1;
break;
}
if (nextState != SM_currentState)
{
SM_currentState = nextState;
switch (SM_currentState)
{
case SM_Display_1:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Sensor 1 reads");
lcd.setCursor(0,1);
lcd.print(value1);
delay(20);
Serial. print("Case 0");
break;
case SM_Display_2:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Sensor 2 reads");
lcd.setCursor(0,1);
lcd.print(value2);
delay(10);
break;
case SM_Display_3:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Pump 1 ran");
lcd.setCursor(0,1);
lcd.setCursor(5,1);
lcd.print(runs1);
lcd.print(" times");
delay(10);
break;
case SM_Display_4:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Pump 2 ran");
lcd.setCursor(0,1);
lcd.print(runs2);
lcd.setCursor(5,1);;
lcd.print(" times");
delay(10);
break;
case SM_Display_5:
lcd.clear();
lcd.setCursor(0,0);
lcd.print("Next cycle in");
lcd.setCursor(0,1);
lcd.print(elapsedMinutes);
lcd.setCursor(5,1);
lcd.print(" Minutes");
delay(10);
break;
default:
break;
}
}
}
EDIT: Generic implementation of a state machine
Ok, here I post my generic scheme for a state machine. Usually I use the "state-based" state machine, which is actions are linked to states; sometimes I also saw "transition-based" state machines, where actions were linked to transitions, but... Ok, I'm not a real expert, so I don't want to write a lot of (wrong) things.
In this kind of machines, each state can have up to three kind of actions and a check:
- actions you perform when you ENTER the state
- actions you perform continuously when you are INSIDE the state
- actions you perform when you EXIT the state
- and of course, you have to CHECK whether you need to make a transition to another state
For instance, when you have in a microwave a state corresponding to the ON state, you have to:
- when you ENTER the state, turn on the heater
- when you are INSIDE the state, update the timer value
- when you are done and you are EXITing the state, turn off the heater
- when during the CHECK you see that the timer has expired or the stop button was pressed, you switch to another state
So there are four kind of actions: ENTER, INSIDE, EXIT and CHECK. For linearity I learned that it is much better to write each of them in a separate switch case. This is my usual implementation scheme:
void stateMachine()
{
byte nextState = currState; // currState of course is defined outside
switch (currState)
{
// Here you have to CHECK, for each state, the conditions
// needed to switch to another state; if you switch, put
// the new state in nextState
}
if (nextState != currState)
{
switch (currState)
{
// Perform the EXIT functions for each state
}
// Advance to the next state
currState = nextState;
switch (currState)
{
// Perform the ENTER functions for each state
}
}
else // See note below
{
switch (currState)
{
// Perform the actions to be performed continuously
// INSIDE each state
}
}
}
NOTE: the else can be omitted or not. If you leave it, in the cycle that you detect the change you do not perform the INSIDE action; if you omit it, the INSIDE action will be performed. This is useful in different cases.
Now, usually (and yours is one of these cases) you only need the CHECK and ENTER sections (if you look at the code, you will see that only these two sections were implemented). Once in a while you will need also the INSIDE section, and once in your life you will also need the EXIT ;) (at least in my experience)
Verifiable – Test the code you're about to provide to make sure it reproduces the problem– jose can u c Mar 13 '18 at 15:33