Enum loop states running simultaneously

Hello, I am trying to make my code run, such that when a start button is pushed, there will be an initial delay, and then subsequently, as long as the stop button is not pushed, the motor will run at the same time everyday. Everytime the motor runs, the counter 'CountRounds' will increase by one. However, currently the counter is increasing at the same rate as it counts milliseconds. After checking using the serial monitor, I realised that my states 'ButtonChecks' and 'SwitchOnMotor' are running simultaneously. Could you advise me on the problem?

void(motordelay) {
t = rtc.getTime();
  unsigned long currentMillis = millis();
  static unsigned long previousMillis = 0;  // timestamp when the current State was started
 
  // this is the state-machine based on the switch-case-statement
  // depending on the value of variable "currentState" only the commands
  // below one "case" gets executed
  
        
  switch (currentState) {
    case States::WaitForButtonPress:
      // a state that waits for an external input to occur
      if(digitalRead(stopbuttonPin) == HIGH)     {
      if (digitalRead(startbuttonPin) == HIGH)  {
      digitalWrite(LED, HIGH);
        previousMillis = currentMillis;
        codestart = true;
      }
      }
      else {
    digitalWrite(LED, LOW);
    digitalWrite(motor, LOW);
      }
      
      if(codestart){
      if (currentMillis - previousMillis >= firstmotorOffPeriod) {
        codestart = false;
        currentState = States::SwitchOnMotor;
        
      }
      }
      break;

    case States::ButtonChecks:
      if(digitalRead(stopbuttonPin) == HIGH) {
        currentState = States::SwitchOnMotor;
      }
      else{
        CountRounds = 0;
        currentState = States::WaitForButtonPress;
      }
      break;
      
    case States::SwitchOnMotor:
    Serial.println("reached");
    if(t.hour == OnHour && t.min == OnMin) {
    Serial.println("Turning");
    digitalWrite(motor, HIGH);
  }
  else if (t.hour == OffHour && t.min == OffMin) {
    digitalWrite(motor, LOW);
  }
      currentState = States::ButtonChecks;
      CountRounds++;
      Serial.println(CountRounds);
      break;

    
  }
}

In the Arduino IDE, use Ctrl T or CMD T to format your code then copy the complete sketch.

Use the </> icon from the ‘reply menu’ to attach the copied sketch.

Here is my entire code.

#include "DHT.h" //including DHT11/21 library
#include<Wire.h>
#include<LiquidCrystal_I2C.h> //Including library for LCD that is connected to an I2C device
#include <DS3231.h> //Including library for clock that is connected to I2C
LiquidCrystal_I2C lcd1 (0x27, 16, 2); //Defining the LCD's I2C address, and size
DS3231 rtc(SDA, SCL);
Time t;

const byte startbuttonPin = 8;        //Button connected to GND
const byte stopbuttonPin = 9;
const byte LCDbuttonPin = 7;
const byte testbuttonPin = 6;
const byte LED = 12;    // Pin connected to LED
const byte turningLED = 5;
const byte motor = 10; //Pin connected to motor through a relay
const byte DHTPIN = 11;   // Pin that DHT11/21 is connected to

const int OnHour = 9;   //Using DS3231 clock to tell time to on motor
const int OnMin = 57;
const int OffHour = 9;  //Using DS3231 clock to tell time to off motor
const int OffMin = 58;

#define DHTTYPE DHT11   // DHT 11 
//#define DHTTYPE DHT22   // DHT 22  (AM2302)
//#define DHTTYPE DHT21   // DHT 21 (AM2301)
DHT dht(DHTPIN, DHTTYPE); //Initializing DHT sensor for arduino

// an enumeration class for the different states of the state machine
enum class States { // Assigns names to a set of numbers starting at 0.
  WaitForButtonPress,
  ButtonChecks,
  SwitchOnMotor,
}
currentState; // these constants are used for this variable named "currentState"

enum class LCDs {
  original,
  pressed,
  changed,
}
currentstate;

enum class counts {
  Shows,
  Waits,
}
currentvalues;

enum class Days {
  ButtonPress,
  Check,
  Interval,
  ShowDays,
}
currentarea;

bool codestart = false; //used to only start motor code when button has been released
bool state = false;

// variables for timePeriods
const unsigned long firstmotorOffPeriod = 8000;  //the initial 4 days that the motor is off (in milliseconds), make cost if variable never changes
const unsigned long motorOnPeriod  = 1000;  // how long should the motor be on (in milliseconds), make const if variable never changes
const unsigned long motorOffPeriod = 3000; // how long should the motor be Off (in milliseconds), make const if variable never changes
const unsigned long dhtDelay = 4000;       //the intervals where the LCD updates the humidity/temperature values
const unsigned long DayDelay = 10000;       //the delay to update the day value
const unsigned long DayCount = 10000;       //the time for a day to pass

int CountRounds = 0;                       // used to count loops, to stop code after certain loops
int CountDays = 0;                        //used to count days passed, for the LCD


void setup() {
  Serial.begin(9600); //rate in milliseconds that arduino sends info to serial monitor

  rtc.begin();  //starting up Clock
  lcd1.init();  //starting up LCD display
  lcd1.backlight();
  dht.begin(); //starting up DHT sensor

  pinMode(startbuttonPin, INPUT_PULLUP);  //Button pin is an input device, and will atomatically pull up its state
  pinMode(stopbuttonPin, INPUT_PULLUP);
  pinMode(LCDbuttonPin, INPUT_PULLUP);
  pinMode(testbuttonPin, INPUT_PULLUP);
  pinMode(LED, OUTPUT); // LED pin is an output
  pinMode(turningLED, OUTPUT);
  pinMode(motor, OUTPUT); // Motor pin is an output
  currentState = States::WaitForButtonPress; //Setting the current state for the enum function
  digitalWrite(LED, LOW); //LED's state is initially always LOW
  digitalWrite(motor, LOW); //Motor's state is initially always LOW
}

// parts of the program that belong to one functional unit are coded in their
// own function where each function has a self-explaining name
// This makes it much easier to test code and keep an overview
// with the growing of the code


void loop() {
  NumberofDays();
  TempHumidLCD(); //the state machine called inside the loop to run the humidity/temp snsor to display values on the LCD
  motordelay();   // the state machine called inside the loop to run the relay/motor
  // DayLCD();  //the state machine called inside the loop  to print the number of days and number of loops
}



void motordelay() {
  t = rtc.getTime();
  unsigned long currentMillis = millis();
  static unsigned long previousMillis = 0;  // timestamp when the current State was started

  // this is the state-machine based on the switch-case-statement
  // depending on the value of variable "currentState" only the commands
  // below one "case" gets executed


  switch (currentState) {
    case States::WaitForButtonPress:
      // a state that waits for an external input to occur
      if (digitalRead(stopbuttonPin) == HIGH)     {
        if (digitalRead(startbuttonPin) == HIGH)  {
          digitalWrite(LED, HIGH);
          previousMillis = currentMillis;
          codestart = true;
        }
      }
      else {
        digitalWrite(LED, LOW);
        digitalWrite(motor, LOW);
      }

      if (codestart) {
        if (currentMillis - previousMillis >= firstmotorOffPeriod) {
          codestart = false;
          currentState = States::SwitchOnMotor;

        }
      }
      break;

    case States::ButtonChecks:
      if (digitalRead(stopbuttonPin) == HIGH) {
        currentState = States::SwitchOnMotor;
      }
      else {
        CountRounds = 0;
        currentState = States::WaitForButtonPress;
      }
      break;

    case States::SwitchOnMotor:
      Serial.println("reached");
      if (t.hour == OnHour && t.min == OnMin) {
        Serial.println("Turning");
        digitalWrite(motor, HIGH);
      }
      else if (t.hour == OffHour && t.min == OffMin) {
        digitalWrite(motor, LOW);
      }
      currentState = States::ButtonChecks;
      CountRounds++;
      Serial.println(CountRounds);
      break;


  }
}

void TempHumidLCD() {
  float h = dht.readHumidity();
  // Read temperature as Celsius
  float t = dht.readTemperature();
  // Read temperature as Fahrenheit
  float f = dht.readTemperature(true);
  unsigned long currentMillis = millis();
  static unsigned long previous = 0;

  switch (currentstate) {
    case LCDs ::original:
      //Serial.print("Humidity: ");
      //Serial.print(h);
      //Serial.print(" %\t");
      //Serial.print("Temperature: ");
      //Serial.print(t);
      //Serial.println(" *C ");
      lcd1.setCursor(0, 0);
      lcd1.print("No. of turns:");
      lcd1.print("    "); //  two blank spaces assuming CountRounds is a max of two figures,
      lcd1.setCursor(0, 0);
      lcd1.print("No. of turns:");
      lcd1.print(CountRounds);
      lcd1.setCursor(0, 1);
      lcd1.print("No. of days:");
      lcd1.print("    ");
      lcd1.setCursor(0, 1);
      lcd1.print("No. of days:");
      lcd1.print(CountDays);
      previous = currentMillis;
      currentstate = LCDs::pressed;
      break;

    case LCDs ::pressed:
      if (digitalRead(LCDbuttonPin) == HIGH) {


        lcd1.setCursor(0, 0);
        lcd1.print("Temp: ");
        lcd1.print("           ");
        lcd1.setCursor(0, 0);
        lcd1.print("Temp: ");
        lcd1.print(t);
        lcd1.print((char)223);
        lcd1.print("C");
        lcd1.setCursor(0, 1);
        lcd1.print("Humidity: ");
        lcd1.print(h);
        lcd1.print("%");
        currentstate = LCDs::changed;
      }

      if (digitalRead(LCDbuttonPin) == LOW) {
        currentstate = LCDs::original;
      }

      break;

    case LCDs ::changed:
      if (currentMillis - previous >= dhtDelay) {
        // Serial.println("waiting");
        currentstate = LCDs::original;
      }
  }

}

void NumberofDays() {

  unsigned long currentMillis = millis();
  static unsigned long previousday = 0;


  switch (currentarea) {

    case Days:: ButtonPress:
      CountDays = 0;
      if (digitalRead(stopbuttonPin) == HIGH) {
        if (digitalRead(startbuttonPin) == HIGH) {
          currentarea = Days::ShowDays;

        }
      }
      else {
        CountDays = 0;
      }
      break;


    case Days::ShowDays:
      if (digitalRead(stopbuttonPin) == HIGH) {
        //Serial.println("Counting");
        CountDays++;
        previousday = currentMillis;
        currentarea = Days::Interval;
      }
      else {
        currentarea = Days::ButtonPress;

      }
      break;

    case Days::Interval:
      if (digitalRead(stopbuttonPin) == HIGH) {
        if (currentMillis - previousday >= DayCount) {
          //Serial.println("Waiting for day to pass");
          currentarea = Days::ShowDays;
        }
      }
      else {
        currentarea = Days::ButtonPress;

      }
      break;
  }
}





In button checks you check whether stopbuttonPin is high. Since it is using a mode of inputPullup, it probably is high so state flips to SwitchOnMotor.

SwitchOnMotor does its thing and then unconditionally goes back to ButtonChecks.

Hello, sorry I forgot to mention. For my stop button, I am using a normally closed button. This means that when the state is high, then Arduino recognises that the button is not pushed and vice versa.

When you say that SwitchOnMotor does its thing and then unconditionally goes back to ButtonChecks, why does that happen even when the time is not reached, meaning that my motor does not turn on?

I don't understand the logic yet, but this is not impacted by the time:

currentState = States::ButtonChecks;

It runs whenever your SwitchOnMotor state is reached.

Hi ignoring the time, shouldn't by enum only go to
the ButtonChecks state only if the code under the SwitchOnMotor has finished? meaning that it will not change state unless the task has finished? currently both of my states are forming a sort of infinite loop.

Do correct me if my understanding of the enum loop is wrong, I just recently learned how to use it.

In SwitchOnMotor state, depending on the time, the motor may get turned on or off. Whether it does or not, the code after those ifs means that control will return to ButtonChecks, as you observe.

What did you want to happen?

I was expecting it to work systematically, meaning that after SwitchOnMotor state, then it goes to ButtonChecks. If it checks for that period that the stop button is not pressed, then it will stay at the SwitchOnMotor until the next time the motor turns on and off. So basically I thought that it would cycle through, instead of skipping to each state every millisecond.

ButtonChecks should not be a 'state' but rather something you do while inside SwitchOnMotor state and, depending on the values of the buttons, stop the motor or do nothing.

If this were my code, I would use the StateChangeDetection example (File->examples->02.digital->StateChangeDetection) to read all the buttons at the start of loop() and determine if they had changed or not. I would use this information within the various state machines to determine whether or not to transition to a new state or not.

They seem to be designed to ping-pong back and forth between those two states. That probably means that they shouldn't be separate states. Each state should check for the button presses that would change the state. Something like this:

void(motordelay)
{
  t = rtc.getTime();
  unsigned long currentMillis = millis();
  static unsigned long previousMillis = 0;  // timestamp when the current State was started

  // this is the state-machine based on the switch-case-statement
  // depending on the value of variable "currentState" only the commands
  // below one "case" gets executed

  // All states check for Stop button
  if (digitalRead(stopbuttonPin) == LOW)
  {
    digitalWrite(LED, LOW);
    digitalWrite(motor, LOW);
    currentState = States::WaitForStartButtonPress;
  }

  switch (currentState)
  {
    case States::WaitForStartButtonPress:
      CountRounds = 0;
      if (digitalRead(startbuttonPin) == LOW)
      {
        digitalWrite(LED, HIGH);
        previousMillis = currentMillis;
        currentState = States::InitialDelay;
      }
      break;

    case States::InitialDelay:
      if (currentMillis - previousMillis >= firstmotorOffPeriod)
      {
        currentState = States::WaitForOnTime;
      }
      break;


    case States::WaitForOnTime:
      if (t.hour == OnHour && t.min == OnMin)
      {
        Serial.println("Turning On");
        CountRounds++;
        Serial.println(CountRounds);
        digitalWrite(motor, HIGH);
        currentState = States::WaitForOffTime;
      }
      break;

    case States::WaitForOffTime:
      if (t.hour == OffHour && t.min == OffMin)
      {
        Serial.println("Turning Off");
        digitalWrite(motor, LOW);
        currentState = States::WaitForOnTime;
      }
      break;
  }
}

As pointed out earlier, the test in ::ButtonChecks can cause the system to toggle between ::SwitchOnMotor and ::ButtonChecks.
I'm not convinced that the button is wired and reading as you think. Place a Serial print in this statement to see if you are inadvertantly getting into the state. Indeed, more serial prints everywhere can help sort out the logic and the program flow.

if (digitalRead(stopbuttonPin) == HIGH) {
        Serial.println("stopbuttonPin reads HIGH");
        currentState = States::SwitchOnMotor;
      }

Hello, I tried your code and it works perfectly. Can I understand what is the main difference between your code and mine? Meaning why does yours not ping pong between your WaitForOnTime and WaitForOffTime states, and why did mine?

i realize you may already be satisfied but i found your code confusing.

consider the following, that simply exercises your motor state machine. see if it's not easier for you to understand.


struct Time {
    int hour;
    int min;
};

struct DS3231 {
    int min = 0;

    DS3231 (int a, int b) {  };
    Time getTime (void)  {
        min++;
        Time t = { 9, min};
        return t;
    };
};

DS3231  rtc(SDA, SCL);
Time    t;

// -----------------------------------------------------------------------------
#undef MyHW
#ifdef MyHW
enum { ButOn = LOW,  ButOff = HIGH };   // normally-open pull-ups
enum { OutOn = LOW,  OutOff = HIGH };   // active low

const byte startbuttonPin   = A1;
const byte stopbuttonPin    = A2;

#else
enum { ButOn = HIGH, ButOff = LOW  };   // normally-closed pull-ups
enum { OutOn = HIGH, OutOff = LOW  };   // active high

const byte startbuttonPin   = 8;        //Button connected to GND
const byte stopbuttonPin    = 9;
#endif

const byte LED              = 12;    // Pin connected to LED
const byte motor            = 10; //Pin connected to motor through a relay

enum States { // Assigns names to a set of numbers starting at 0.
    MotorOff,
    MotorDelay,
    MotorOn,
}
currentState; // these constants are used for this variable named "currentState"

bool codestart = false; //used to only start motor code when button has been released

const int OnHour  = 9;   //Using DS3231 clock to tell time to on motor
const int OffHour = 9;   //Using DS3231 clock to tell time to off motor
const int OnMin   = 10;
const int OffMin  = 25;

const unsigned long firstmotorOffPeriod = 8000;  //the initial 4 days that the motor is off (in milliseconds), make cost if variable never changes

int CountRounds = 0;                       // used to count loops, to stop code after certain loops

// -----------------------------------------------------------------------------
void setup () {
    Serial.begin (9600); //rate in milliseconds that arduino sends info to serial monitor

    pinMode (startbuttonPin, INPUT_PULLUP);
    pinMode (stopbuttonPin,  INPUT_PULLUP);

    pinMode      (LED,   OUTPUT);
    pinMode      (motor, OUTPUT);
    digitalWrite (LED,   OutOff);
    digitalWrite (motor, OutOff);
}

// -----------------------------------------------------------------------------
char s [80];

void loop () {
    motordelay ();

    sprintf (s, "%s: state %d, hr %2d, min %2d",
        __func__, (int) currentState, t.hour, t.min);
    Serial.println (s);

    delay (1000);
}

// -----------------------------------------------------------------------------
bool
checkOn (void)
{
    return (digitalRead (startbuttonPin) == ButOn)
            || (t.hour == OnHour && t.min == OnMin);
}

bool
checkOff (void)
{
    return (digitalRead (stopbuttonPin) == ButOn)
            || (t.hour == OffHour && t.min == OffMin);
}

// -----------------------------------------------------------------------------
void motordelay () {
           unsigned long currentMillis = millis ();
    static unsigned long previousMillis = 0;  // timestamp when the current State was started

    t = rtc.getTime ();

    // this is the state-machine based on the switch-case-statement
    // depending on the value of variable "currentState" only the commands
    // below one "case" gets executed
    switch (currentState) {
    case MotorOff:
        // a state that waits for an external input to occur
        if (checkOn ())  {
            digitalWrite (LED, OutOn);
            previousMillis = currentMillis;
            codestart = true;
            currentState = MotorDelay;
            Serial.println (" MotorOn: start");
        }
        break;

    case MotorDelay:
        if (digitalRead (stopbuttonPin) == ButOn) {
            digitalWrite (LED,   OutOff);
            currentState = MotorOff;
            Serial.println (" MotorDelay: stop");
        }

        else if (currentMillis - previousMillis >= firstmotorOffPeriod) {
            codestart = false;
            currentState = MotorOn;
            CountRounds++;
            Serial.println (" MotorDelay: timeout");
        }
        break;
    case MotorOn:
        digitalWrite (motor, OutOn);

        if (checkOff ())  {
            digitalWrite (LED,   OutOff);
            digitalWrite (motor, OutOff);
            currentState = MotorOff;
            Serial.println (" MotorOn: stop");
        }
        break;
    }
}

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