What is wrong with my state machines?

Question

My goal is to make an automatic gate and garage door, and it has a servo as the lock mechanism on the gate, and alarm systems that notify when the gate or garage door is open too long, more than the interval (in my case 10 seconds) through a buzzer.

It also uses:

Bluetooth to send a command (which is readChar) to open/close the gate and open/close the garage.
4 limit switches (2 switched located at both end of gate and garage to stop its movement).

I want that the gate and garage doors can be opened/closed at the same time, so there are 2 state machines. And the alarm system (if it is activated) will stop when the gate reaches limit switch 1, and the garage reaches limit switch 3.

However with my current code (compiled), I get:

The first state machine (stateGate) can open and close the gate, but the buzzer does not buzz when it exceeds the limit.
The second state machine (stateGarage) did not do anything/did not operate.

Can someone point out/correct what is the mistake? Because I can't seem to find what is wrong. Thank you in advance!

Here is my current full code:

#include <Servo.h>

Servo Servokey;

// Motor for Gate (MotorA)
const byte IN1 = 9;
const byte IN2 = 8;
const byte EnableMotorA = 6;

// Motor for Garage Door (MotorB)
const byte IN3 = 4;
const byte IN4 = 3;
const byte EnableMotorB = 5;

const byte LimitSwitch1 = A1;
const byte LimitSwitch2 = A2;
const byte LimitSwitch3 = A3;
const byte LimitSwitch4 = A4;

const int piezoPin = 10; // piezo for the alarm system

int StateGate;
int StateGarage;
boolean gateClosed = false;
boolean garageClosed = false;

unsigned long GateMillis; //will hold the timer for Gate
unsigned long GarageMillis; // will hold the timer for Garage
unsigned long interval = 10000;

void setup() {
  pinMode(IN1, OUTPUT);
  pinMode(IN2, OUTPUT);
  pinMode(EnableMotorA, OUTPUT);

  pinMode(IN3, OUTPUT);
  pinMode(IN4, OUTPUT);
  pinMode(EnableMotorB, OUTPUT);

  pinMode(LimitSwitch1, INPUT);
  pinMode(LimitSwitch2, INPUT);
  pinMode(LimitSwitch3, INPUT);
  pinMode(LimitSwitch4, INPUT);

  Servokey.attach(11); // lock mechanism for Gate
  Servokey.write(70);

  unsigned long GateMillis = 0; //will hold the timer for Gate
  unsigned long GarageMillis = 0;

  Serial.begin(9600);
}

void loop() {
  byte readChar = 0;
  if (Serial.available() > 0)
    readChar = Serial.read();
  stateGate(readChar);
  stateGarage(readChar);
}

void stateGate(byte readChar) {
  static enum { StateGate_Idle, StateGate_Opening, StateGate_Open, StateGate_Closing, StateGate_Close, StateGate_Alarm} StateGate;
  switch (StateGate) {
    case StateGate_Idle:
      { // If we received a valid command, start the operation
        if (readChar > 0) {
          switch (readChar) {
            case 'A':
              StateGate = StateGate_Opening;
              break;
            case 'B':
              StateGate = StateGate_Closing;
              break;
            default:
              // Here you can notify that there was a bad command
              break;
          }
        }
      }
      break;
    case StateGate_Opening:
      Servokey.write(140);
      delay(50);
      digitalWrite(IN1, LOW);
      digitalWrite(IN2, HIGH);
      analogWrite(EnableMotorA, 250);
      StateGate = StateGate_Open;
      break;
    case StateGate_Open:
      if (digitalRead(LimitSwitch2)) {
        digitalWrite(IN1, LOW);
        digitalWrite(IN2, LOW);
        analogWrite(EnableMotorA, 0);
        GateMillis = millis();
        gateClosed = false;
        StateGate = StateGate_Idle;
      }
      break;
    case StateGate_Closing:
      digitalWrite(IN1, HIGH);
      digitalWrite(IN2, LOW);
      analogWrite(EnableMotorA, 250);
      StateGate = StateGate_Close;
      break;
    case StateGate_Close:
      if (digitalRead(LimitSwitch1)) {
        digitalWrite(IN1, LOW);
        digitalWrite(IN2, LOW);
        analogWrite(EnableMotorA, 0);
        Servokey.write(70);
        delay(50);
        gateClosed = true;
        StateGate = StateGate_Idle;
      }
      break;
    case StateGate_Alarm:
      if ((millis() - GateMillis) > interval && gateClosed == false) {
        tone(piezoPin, 5000);
        StateGate = StateGate_Idle;
      } else if (gateClosed == true) {
        StateGate = StateGate_Idle;
      }
      break;
    default:
      StateGate = StateGate_Idle;
      break;
  }
}

void stateGarage(byte readChar) {
  static enum { StateGarage_Idle, StateGarage_Opening, StateGarage_Open, StateGarage_Closing, StateGarage_Close, StateGarage_Alarm} StateGarage;
  switch (StateGarage) {
    case StateGarage_Idle:
      { // If we received a valid command, start the operation
        if (readChar > 0) {
          switch (readChar) {
            case 'C':
              StateGarage = StateGarage_Opening;
              break;
            case 'D':
              StateGarage = StateGarage_Closing;
              break;
            default:
              // Here you can notify that there was a bad command
              break;
          }
        }
      }
      break;
    case StateGarage_Opening:
      digitalWrite(IN3, LOW);
      digitalWrite(IN4, HIGH);
      analogWrite(EnableMotorA, 250);
      StateGarage = StateGarage_Open;
      break;
    case StateGarage_Open:
      if (digitalRead(LimitSwitch4)) {
        digitalWrite(IN3, LOW);
        digitalWrite(IN4, LOW);
        analogWrite(EnableMotorA, 0);
        GarageMillis = millis();
        garageClosed = false;
        StateGarage = StateGarage_Idle;
      }
      break;
    case StateGarage_Closing:
      digitalWrite(IN3, HIGH);
      digitalWrite(IN4, LOW);
      analogWrite(EnableMotorA, 250);
      StateGarage = StateGarage_Close;
      break;
    case StateGarage_Close:
      if (digitalRead(LimitSwitch3)) {
        digitalWrite(IN3, LOW);
        digitalWrite(IN4, LOW);
        analogWrite(EnableMotorB, 0);
        garageClosed = true;
        StateGarage = StateGarage_Idle;
      }
      break;
    case StateGarage_Alarm:
      if ((millis() - GarageMillis) > interval && garageClosed == false) {
        tone(piezoPin, 5500);
        StateGarage = StateGarage_Idle;
      } else if (garageClosed == true)
        StateGarage = StateGarage_Idle;
      break;
    default:
      StateGarage = StateGarage_Idle;
      break;
  }
}

I made another code like what @CodeGorilla had suggested, however it turned out that both state machines did not operate.... I think I am still missed out something

Here's the other code:

#include <Servo.h>

Servo Servokey;

enum StateGate { StateGate_Idle = 0, StateGate_Opening, StateGate_Open, StateGate_Closing, StateGate_Closed, StateGate_Alarm, StateGate_Last };
enum StateGarage { StateGarage_Idle = 0, StateGarage_Opening, StateGarage_Open, StateGarage_Closing, StateGarage_Closed, StateGarage_Alarm, StateGarage_Last };

struct Data1
{
  byte IN1;
  byte IN2;
  bool Enable;
  int NextState;

  Data1(byte in1, byte in2, bool enable, int nextstate)
       :IN1(in1), IN2(in2), Enable(enable), NextState(nextstate)
  {}
};

struct Data2
{
  byte IN3;
  byte IN4;
  bool Enable;
  int NextState;

  Data2(byte in3, byte in4, bool enable, int nextstate)
       :IN3(in3), IN4(in4), Enable(enable), NextState(nextstate)
  {}
};

struct MotorData1
{
  byte IN1;
  byte IN2;
  byte Enable;
  MotorData1(byte i1, byte i2, byte enable)
       : IN1(i1), IN2(i2), Enable(enable)
  {}
  void pinMode() const
  {
    :: pinMode(IN1, OUTPUT);
    :: pinMode (IN2, OUTPUT);
    :: pinMode (Enable, OUTPUT);
  }
  int Set(const Data1& data1) const
  {// Set the motors to the given state
    digitalWrite(IN1, data1.IN1);
    digitalWrite(IN2, data1.IN2);
    analogWrite(Enable, data1.Enable?250:0);
    return data1.NextState;
  }
};

struct MotorData2
{
  byte IN3;
  byte IN4;
  byte Enable;
  MotorData2(byte i3, byte i4, byte enable)
       : IN3(i3), IN4(i4), Enable(enable)
  {}
  void pinMode() const
  {
    :: pinMode(IN3, OUTPUT);
    :: pinMode (IN4, OUTPUT);
    :: pinMode (Enable, OUTPUT);
  }
  int Set(const Data2& data2) const
  {// Set the motors to the given state
    digitalWrite(IN3, data2.IN3);
    digitalWrite(IN4, data2.IN4);
    analogWrite(Enable, data2.Enable?250:0);
    return data2.NextState;
  }
};
// Define the motors
const MotorData1 MotorsA(9, 8, 6);
const MotorData2 MotorsB(4, 3, 5);

const byte LimitSwitch1 = A1;
const byte LimitSwitch2 = A2;
const byte LimitSwitch3 = A3;
const byte LimitSwitch4 = A4;

const int piezoPin = 10; // piezo for the alarm system

StateGate CurrentGateState = StateGate_Idle;
StateGarage CurrentGarageState = StateGarage_Idle;
boolean gateClosed = false;
boolean garageClosed = false;

unsigned long GateMillis(0); //will hold the timer for Gate
unsigned long GarageMillis(0); // will hold the timer for Garage
unsigned long interval = 10000;

void setup() {
    MotorsA.pinMode();
    MotorsB.pinMode();

    pinMode(LimitSwitch1, INPUT);
    pinMode(LimitSwitch2, INPUT);
    pinMode(LimitSwitch3, INPUT);
    pinMode(LimitSwitch4, INPUT);

    Servokey.attach(11); // lock mechanism for Gate
    Servokey.write(70);

    Serial.begin(9600);
}

void loop()
{
    byte readChar = 0;
    if (Serial.available() > 0)
        readChar = Serial.read();
    stateGate(readChar);
    stateGarage(readChar);
}

void stateGate(byte readChar) 
{// Define the state machine
    static const Data1 stateData1[StateGate_Last] =
    {
          Data1(LOW, LOW, false, StateGate_Idle)  // StateGate_Idle
        , Data1(LOW, HIGH, true, StateGate_Open)  // StateGate_Opening
        , Data1(LOW, LOW, false, StateGate_Idle)  // StateGate_Open
        , Data1(HIGH, LOW, false, StateGate_Closed)  // StateGate_Closing
        , Data1(LOW, LOW, false, StateGate_Idle)  // StateGate_Closed
        , Data1(LOW, LOW, false, StateGate_Idle)  // StateGate_Alarm
    };
switch (CurrentGateState)
    {
        case StateGate_Idle:
        { // If we received a valid command, start the operation
            if (readChar > 0) {
                switch (readChar) {
                case 'A':
                    if (CurrentGateState != StateGate_Open)
                    {  // Prevent opening the gate if its already open
                        CurrentGateState = StateGate_Opening;
                    }
                    break;
                case 'B':
                    if (CurrentGateState != StateGate_Closed)
                    {// Prevent closing the gate if its already closed.
                        CurrentGateState = StateGate_Closing;
                    }
                    break;
                default:
                    // Here you can notify that there was a bad command
                    break;
                }
            }
        }
        break;
    case StateGate_Opening:
        Servokey.write(140);
        delay(50);
        CurrentGateState = MotorsA.Set(stateData1[StateGate_Opening]);
        break;
    case StateGate_Open:
        if (digitalRead(LimitSwitch2)) 
        {
            CurrentGateState = MotorsA.Set(stateData1[StateGate_Open]);
            GateMillis = millis();
            gateClosed = false;
        }
        break;
    case StateGate_Closing:
        CurrentGateState = MotorsA.Set(stateData1[StateGate_Closing]);
        break;
    case StateGate_Closed:
        if (digitalRead(LimitSwitch1))
        {
            CurrentGateState = MotorsA.Set(stateData1[StateGate_Closed]);
            Servokey.write(70);
            delay(50);
            gateClosed = true;
        }
        break;
    case StateGate_Alarm:
        if ((millis() - GateMillis) > interval && gateClosed == false) {
            tone(piezoPin, 5000);
            CurrentGateState = StateGate_Idle;
        }
        else if (gateClosed == true) {
            CurrentGateState = StateGate_Idle;
        }
        break;
    default:
        CurrentGateState = StateGate_Idle;
        break;
    }
}

void stateGarage(byte readChar) 
{// Define the state machine
    static const Data2 stateData2[StateGarage_Last] =
    {
          Data2(LOW, LOW, false, StateGarage_Idle)  // StateGate_Idle
        , Data2(LOW, HIGH, true, StateGarage_Open)  // StateGate_Opening
        , Data2(LOW, LOW, false, StateGarage_Idle)  // StateGate_Open
        , Data2(HIGH, LOW, false, StateGarage_Closed)  // StateGate_Closing
        , Data2(LOW, LOW, false, StateGarage_Idle)  // StateGate_Closed
        , Data2(LOW, LOW, false, StateGarage_Idle)  // StateGate_Alarm
    };
switch (CurrentGarageState)
    {
        case StateGarage_Idle:
        { // If we received a valid command, start the operation
            if (readChar > 0) {
                switch (readChar) {
                case 'A':
                    if (CurrentGarageState != StateGarage_Open)
                    {  // Prevent opening the gate if its already open
                        CurrentGarageState = StateGarage_Opening;
                    }
                    break;
                case 'B':
                    if (CurrentGarageState != StateGarage_Closed)
                    {// Prevent closing the gate if its already closed.
                        CurrentGarageState = StateGarage_Closing;
                    }
                    break;
                default:
                    // Here you can notify that there was a bad command
                    break;
                }
            }
        }
        break;
    case StateGarage_Opening:
        CurrentGarageState = MotorsB.Set(stateData2[StateGarage_Opening]);
        break;
    case StateGarage_Open:
        if (digitalRead(LimitSwitch2)) 
        {
            CurrentGarageState = MotorsB.Set(stateData2[StateGarage_Open]);
            GarageMillis = millis();
            garageClosed = false;
        }
        break;
    case StateGarage_Closing:
        CurrentGarageState = MotorsB.Set(stateData2[StateGarage_Closing]);
        break;
    case StateGarage_Closed:
        if (digitalRead(LimitSwitch1))
        {
            CurrentGarageState = MotorsB.Set(stateData2[StateGarage_Closed]);
            Servokey.write(70);
            delay(50);
            garageClosed = true;
        }
        break;
    case StateGarage_Alarm:
        if ((millis() - GarageMillis) > interval && garageClosed == false) {
            tone(piezoPin, 5000);
            CurrentGarageState = StateGarage_Idle;
        }
        else if (garageClosed == true) {
            CurrentGarageState = StateGarage_Idle;
        }
        break;
    default:
        CurrentGarageState = StateGarage_Idle;
        break;
    }
}

you check for state StateGarage_Alarm, but you have no command that actually causes the state machine to enter that state. .... your state machine has some questionable functionality ... the StateGate_Opening state code sets the state to StateGate_Open without checking if the gate actually opened. ... similar thing is being repeated in other parts of the code — jsotola, Jan 5 '18 at 8:36
for state 'StateGarage_Alarm', i've been doubting where should connect it to, and thought that with 'if' function it can be worked, so is it better to connect it to 'StateGate_Open' or 'StateGate_Closing' ? — Husna Amiliansyah, Jan 5 '18 at 8:55
what is the problem with 'StateGate_Open'? do i need to create 'boolean openGate'? — Husna Amiliansyah, Jan 5 '18 at 8:58
You could refactor this code and make it much easier to debug. Each case could be reduced to a 5 element structure {IN1State, IN2State, EnableA, EnableB, StateGArage}. garageClosed could be replaced with two extra StateGarage_ entries Opened and Closed. — Code Gorilla, Jan 5 '18 at 15:05

Code Gorilla · Accepted Answer · 2018-01-07 08:31:14Z

There a couple of places where you define variables which hide other variables. eg Line 27 is hidden by 48 for a little bit. More importantly line 22 is hidden by 63 and this is probably why your state machine doesn't work.

As I said in my comment you could make life easier for yourself by refactoring it. Most of the states have the same lines of code in, so by using a structure you can reduce the number of lines and hopefully the number of bugs. I had a play a roughed out this to show what I mean. (I have not compiled it, let alone run it, but it might be a starting point.

Servo Servokey;
// Global to prevent accidental variable declaration.
static enum StateGate { StateGate_Idle = 0, StateGate_Opening, StateGate_Open, StateGate_Closing, StateGate_Closed, StateGate_Alarm, StateGate_Last };
// Defines the state machines    
struct Data
{
    byte IN1;
    byte IN2;
    bool Enable;
    int NextState;

    Data(byte in1, byte in2, bool enable, int nextState)
        : IN1(in1), IN2(in2), Enable(enable), NextState(nextState))
    {}
};
// Group the motor data to make things tidier
struct MotorData
{
    byte IN1;
    byte IN2;
    byte Enable;
    MotorData(byte i1, byte i2, byte enable)
        :IN1(i1), IN2(i2), Enable(enable)
    {}
    void pinMode() const  // Set the pin mode
    {
        ::pinMode(IN1, OUTPUT);  // Use the global pinMode not this function
        ::pinMode(IN2, OUTPUT);
        ::pinMode(Enable, OUTPUT);
    }
    int Set(const Data& data) const
    { // Set the motors to the given state
        digitalWrite(IN1, data.IN1);
        digitalWrite(IN2, data.IN2);
        analogWrite(Enable, data.Enable?250:0);
        return data.NextState
    }
};
// Define the motors
const MotorData MotorsA(9, 8, 6);
const MotorData MotorsB(4, 3, 5);

const byte LimitSwitch1 = A1;
const byte LimitSwitch2 = A2;
const byte LimitSwitch3 = A3;
const byte LimitSwitch4 = A4;

const int piezoPin = 10; // piezo for the alarm system

StateGate CurrentGateState = StateGate_Idle;  // It is best to initialise the variables.
int StateGarage;
boolean gateClosed = false;
boolean garageClosed = false;

unsigned long GateMillis(0); //will hold the timer for Gate
unsigned long GarageMillis(0); // will hold the timer for Garage
unsigned long interval = 10000;

void setup() {
    MotorsA.pinMode();
    MotorsB.pinMode();

    pinMode(LimitSwitch1, INPUT);
    pinMode(LimitSwitch2, INPUT);
    pinMode(LimitSwitch3, INPUT);
    pinMode(LimitSwitch4, INPUT);

    Servokey.attach(11); // lock mechanism for Gate
    Servokey.write(70);

    Serial.begin(9600);
}

void loop()
{
    byte readChar = 0;
    if (Serial.available() > 0)
        readChar = Serial.read();
    stateGate(readChar);
    stateGarage(readChar);
}

void stateGate(byte readChar) 
{// Define the state machine
    static const Data stateData[StateGate_Last] =
    {
          Data(LOW, LOW, false, StateGate_Idle)  // StateGate_Idle
        , Data(LOW, HIGH, true, StateGate_Open)  // StateGate_Opening
        , Data(LOW, LOW, false, StateGate_Idle)  // StateGate_Open
        , Data(HIGH, LOW, false, StateGate_Closed)  // StateGate_Closing
        , Data(LOW, LOW, false, StateGate_Idle)  // StateGate_Closed
        , Data(LOW, LOW, false, StateGate_Idle)  // StateGate_Alarm
    };

    switch (CurrentGateState)
    {
        case StateGate_Idle:
        { // If we received a valid command, start the operation
            if (readChar > 0) {
                switch (readChar) {
                case 'A':
                    if (CurrentGateState != StateGate_Open)
                    {  // Prevent opening the gate if its already open
                        CurrentGateState = StateGate_Opening;
                    }
                    break;
                case 'B':
                    if (CurrentGateState != StateGate_Closed)
                    {// Prevent closing the gate if its already closed.
                        CurrentGateState = StateGate_Closing;
                    }
                    break;
                default:
                    // Here you can notify that there was a bad command
                    break;
                }
            }
        }
        break;
    case StateGate_Opening:
        Servokey.write(140);
        delay(50);
        CurrentGateState = MotorsA.Set(stateData[StateGate_Opening]);
        break;
    case StateGate_Open:
        if (digitalRead(LimitSwitch2)) 
        {
            CurrentGateState = MotorsA.Set(stateData[StateGate_Open]);
            GateMillis = millis();
            gateClosed = false;
        }
        break;
    case StateGate_Closing:
        CurrentGateState = MotorsA.Set(stateData[StateGate_Closing]);
        break;
    case StateGate_Closed:
        if (digitalRead(LimitSwitch1))
        {
            CurrentGateState = MotorsA.Set(stateData[StateGate_Closed]);
            Servokey.write(70);
            delay(50);
            gateClosed = true;
        }
        break;
    case StateGate_Alarm:
        if ((millis() - GateMillis) > interval && gateClosed == false) {
            tone(piezoPin, 5000);
            CurrentGateState = StateGate_Idle;
        }
        else if (gateClosed == true) {
            CurrentGateState = StateGate_Idle;
        }
        break;
    default:
        CurrentGateState = StateGate_Idle;
        break;
    }
}

void stateGarage(byte readChar) {
    static enum { StateGarage_Idle, StateGarage_Opening, StateGarage_Open, StateGarage_Closing, StateGarage_Close, StateGarage_Alarm } StateGarage;
    switch (StateGarage) {
    case StateGarage_Idle:
    { // If we received a valid command, start the operation
        if (readChar > 0) {
            switch (readChar) {
            case 'C':
                StateGarage = StateGarage_Opening;
                break;
            case 'D':
                StateGarage = StateGarage_Closing;
                break;
            default:
                // Here you can notify that there was a bad command
                break;
            }
        }
    }
    break;
    case StateGarage_Opening:
        digitalWrite(IN3, LOW);
        digitalWrite(IN4, HIGH);
        analogWrite(EnableMotorA, 250);
        StateGarage = StateGarage_Open;
        break;
    case StateGarage_Open:
        if (digitalRead(LimitSwitch4)) {
            digitalWrite(IN3, LOW);
            digitalWrite(IN4, LOW);
            analogWrite(EnableMotorA, 0);
            GarageMillis = millis();
            garageClosed = false;
            StateGarage = StateGarage_Idle;
        }
        break;
    case StateGarage_Closing:
        digitalWrite(IN3, HIGH);
        digitalWrite(IN4, LOW);
        analogWrite(EnableMotorA, 250);
        StateGarage = StateGarage_Close;
        break;
    case StateGarage_Close:
        if (digitalRead(LimitSwitch3)) {
            digitalWrite(IN3, LOW);
            digitalWrite(IN4, LOW);
            analogWrite(EnableMotorB, 0);
            garageClosed = true;
            StateGarage = StateGarage_Idle;
        }
        break;
    case StateGarage_Alarm:
        if ((millis() - GarageMillis) > interval && garageClosed == false) {
            tone(piezoPin, 5500);
            StateGarage = StateGarage_Idle;
        }
        else if (garageClosed == true)
            StateGarage = StateGarage_Idle;
        break;
    default:
        StateGarage = StateGarage_Idle;
        break;
    }
}

EDIT ----------------

This code is a complete version of the code refactored to reduce the repetitive statements and try and minimise the number of potential bugs. I have also removed one of the state functions, since apart form the lock and acting on different motors and switches they seemed to be the same.

#include "Servo.h"

Servo Servokey;

enum State { State_Idle = 0, State_Opening, State_Open, State_Closing, State_Closed, State_Alarm, State_Last };

struct Data
{
    byte IN1;
    byte IN2;
    bool Enable;
    State NextState;

    Data(byte in1, byte in2, bool enable, State nextstate)
        :IN1(in1), IN2(in2), Enable(enable), NextState(nextstate)
    {}
};

struct MotorData
{
    byte IN1;
    byte IN2;
    byte Enable;
    MotorData(byte i1, byte i2, byte enable)
        : IN1(i1), IN2(i2), Enable(enable)
    {}
    void pinMode() const
    {
        ::pinMode(IN1, OUTPUT);
        ::pinMode(IN2, OUTPUT);
        ::pinMode(Enable, OUTPUT);
    }
    State Set(const Data& Data) const
    {// Set the motors to the given state
        digitalWrite(IN1, Data.IN1);
        digitalWrite(IN2, Data.IN2);
        analogWrite(Enable, Data.Enable ? 250 : 0);
        return Data.NextState;
    }
};

/// Defines all the data for the gate or garage.
struct Object
{
    State           CurrentState;               // The current state of the object
    char            OpenCommand;                // The command to open the gate/garage
    char            CloseCommand;
    MotorData&      Motors;                     // A reference to the motors object
    byte            LimitSwitchA;
    byte            LimitSwitchB;
    bool            LockObject;                 // True if the object should be locked/ unlocked
    unsigned long   OpenTime;                   // The time the object was last opened.
    bool            IsClosed;                   // True when the object is closed.
    Object(char o, char c, MotorData m, byte la, byte lb, bool lock)
        : CurrentState(State_Idle)
        , OpenCommand(o)
        , CloseCommand(c)
        , Motors(m)
        , LimitSwitchA(la)
        , LimitSwitchB(lb)
        , LockObject(lock)
        , OpenTime(0)
        , IsClosed(false)
    {}
};

// Define the motors
const MotorData MotorsA(9, 8, 6);
const MotorData MotorsB(4, 3, 5);

const byte LimitSwitch1 = A1;
const byte LimitSwitch2 = A2;
const byte LimitSwitch3 = A3;
const byte LimitSwitch4 = A4;

const int piezoPin = 10; // piezo for the alarm system

Object Gate('A', 'B', MotorsA, LimitSwitch1, LimitSwitch2, true);
Object Garrage('C', 'D', MotorsB, LimitSwitch3, LimitSwitch4, false);

unsigned long interval = 10000;

/// Lock or unlock the lock.
void LockGate(bool lock=true)
{
    if (lock)
        Servokey.write(70);
    else
        Servokey.write(140);
    delay(50);
}

void setup() {
    MotorsA.pinMode();
    MotorsB.pinMode();

    pinMode(LimitSwitch1, INPUT);
    pinMode(LimitSwitch2, INPUT);
    pinMode(LimitSwitch3, INPUT);
    pinMode(LimitSwitch4, INPUT);

    Servokey.attach(11); // lock mechanism for Gate
    Servokey.write(70);

    Serial.begin(9600);
}

void loop()
{
    byte readChar = 0;
    if (Serial.available() > 0)
        readChar = Serial.read();
    StateMachine(readChar, Gate);
    StateMachine(readChar, Garrage);
}

void StateMachine(const byte readChar, Object& objectData)
{// Define the state machine
    static const Data stateData[State_Last] =
    {
        Data(LOW, LOW, false, State_Idle)  // State_Idle
        , Data(LOW, HIGH, true, State_Open)  // State_Opening
        , Data(LOW, LOW, false, State_Idle)  // State_Open
        , Data(HIGH, LOW, true, State_Closed)  // State_Closing
        , Data(LOW, LOW, false, State_Idle)  // State_Closed
        , Data(LOW, LOW, false, State_Idle)  // State_Alarm
    };
    switch (objectData.CurrentState)
    {
    case State_Idle:                // If we received a valid command, start the operation
        if (readChar > 0) 
        {
            if (readChar == objectData.OpenCommand)
            {
                objectData.CurrentState = State_Opening;
            }
            else if (readChar == objectData.CloseCommand)
            {
                objectData.CurrentState = State_Closing;
            }
            else
            {
                // Here you can notify that there was a bad command
            }
        }
        break;
    case State_Opening:
        if (objectData.LockObject)
            LockGate(false);
        objectData.CurrentState = objectData.Motors.Set(stateData[State_Opening]);
        break;
    case State_Open:
        if (digitalRead(LimitSwitch2))
        {
            objectData.CurrentState = objectData.Motors.Set(stateData[State_Open]);
            objectData.OpenTime = millis();
            objectData.IsClosed = false;
        }
        break;
    case State_Closing:
        objectData.CurrentState = objectData.Motors.Set(stateData[State_Closing]);
        break;
    case State_Closed:
        if (digitalRead(LimitSwitch1))
        {
            objectData.CurrentState = objectData.Motors.Set(stateData[State_Closed]);
            if (objectData.LockObject)
                LockGate(true);
            objectData.IsClosed = true;
        }
        break;
    case State_Alarm:
        if ((millis() - objectData.OpenTime) > interval && objectData.IsClosed == false) {
            tone(piezoPin, 5000);
            objectData.CurrentState = State_Idle;
        }
        else if (objectData.IsClosed == true) {
            objectData.CurrentState = State_Idle;
        }
        break;
    default:
        objectData.CurrentState = State_Idle;
        break;
    }
}

As before I can't test it, without righting myself a test harness and "it doesn't work" doesn't really help me know what the fault is. One thing I did notice is the Alarm won't work. This is because nothing ever puts it into that state, maybe the check to see if the alarm has been triggered should be in the idle state and if it occurs the state machine should move into the alarm state.

What I have tried to show you is if you are doing the same things repeatedly on different data then define the data outside of the function and reduce the amount of code you write. You would never write these functions would you?

int Add2and2 ()
{return 2+2;}
int Add2and3 ()
{return 2+3;}
int Add2and4 ()
{return 2+4;}

But its often very difficult to spot that you are doing that in your own code.

If there is still something wrong, please describe what the fault is and I'll see what I can do.

I have made the code like your example, but I made the structures separately for both state machines. It has 269 lines in total (longer than the first one). And both of the state machines did not operate.... Could you please tell me what is wrong? I left the code on my question above — Husna Amiliansyah, Jan 5 '18 at 19:19
I don't understand why you have duplicated the data and motor structures, there is no need. You have also copied the state machine from the gate and used it for the garage, you have even left the letters the same. This answer was not about making your code shorter, it was about making it easier to debug an maintain. — Code Gorilla, Jan 7 '18 at 7:54

Stack Exchange Network

current community

your communities

more stack exchange communities

What is wrong with my state machines?

1 Answer 1

Your Answer

Not the answer you're looking for? Browse other questions tagged arduino-uno motor millis switch buzzer or ask your own question.

Hot Network Questions

What is wrong with my state machines?

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged arduino-uno motor millis switch buzzer or ask your own question.

Related

Hot Network Questions