Yes lol I did notice this. But I assumed you were just typing it up quick as an example
*lol it was from my code
The way I ran this in the prototype, was to have a function call in loop which updates the global sensor values, and then those values get checked in the state machine if needed. This was entirely non-blocking and worked great.
seems that your just arguing for an unconcentional single generic state to handle states that need to delay rather than have explicit states for each type of delayed processing
but which approach is easier to debug? sounds like you need to do a lot more things
There's a lot of stuff replied here lol, I did mention that it would be better renamed to be an individual delay state for the Formula mixing cycle. In fact that is becoming obvious as the next coding needs are considered. I am not sure if that is entirely what you mean. Thank you for your help though. I need to digest a lot of what has been said here. I appreciate everyones replies.
I see what you are doing, that is clever. It is often necessary to stay in one state for some period, and you have solved it.
But it is a hack. Your state machine will be in DELAY most of the time, the "real" state is lost, and you need NEXT_STATE to fully express the true state of the machine.
There are other ways to handle timing a state. One is to have multiple DELAY-like states with meaningful names like FILLING_TANK might be held for an amount of time during a state where the tank is fillling. EMPTYING_TANK same thing.
Or.
Any state that needs to be held for a time can set a timer variable, and the entire state machine can be kept from running at all until the single timer expires. A timer of zero might mean to do the next state immediately (or immindiately).
So the state remains meaningful and useful perhaps in other code that is running alongside. And you have only one millis()/elapsedTime code section, right at the top of the FSM deciding whether it is at all time to run the machine and advance the process.
I'll find an example, I use the pattern alla time.
a7
So you are right, current/previous state would be lost to Delay. But in application, it will be writing what it is currently doing to the LCD.
I've been playing with this for awhile, I abstracted the mixing out into an independent state machine and added some other non-blocking functions. Added a ton of comments and fixed up the syntax in places. Hopefully the intent of what is going on is clearer in this code.
class CA { // character array - "safe" featureless string replacement for comparison and printing
public:
char characters[20];
bool operator==(const CA& other) const {
for (int i = 0; i < 20; ++i) {
if (characters[i] != other.characters[i]) {
return false;
}
}
return true;
}
};
struct Valve { // valves control the output line for the main pump
CA name;
int pin;
};
Valve Valves[4];
Valve VALVE_INPUT = {CA{"INPUT VALVE"}, 11};
Valve VALVE_MIX = {CA{"MIXING VALVE"}, 11};
Valve VALVE_RES_1 = {CA{"RES 1 VALVE"}, 11};
Valve VALVE_RES_2 = {CA{"RES 2 VALVE"}, 11};
struct Sensor {
CA name;
int pin;
bool full;
};
Sensor Sensors[9];
Sensor LEVEL_MIX_LOW = {CA{"MIX LOW"}, 11, false};
Sensor LEVEL_MIX_MID = {CA{"MIX MID"}, 11, false};
Sensor LEVEL_MIX_HIGH = {CA{"MIX HIGH"}, 11, false};
Sensor LEVEL_RES_1_LOW = {CA{"RES_1 LOW"}, 11, false};
Sensor LEVEL_RES_1_MID = {CA{"RES_1 MID"}, 11, false};
Sensor LEVEL_RES_1_HIGH = {CA{"RES_1 HIGH"}, 11, false};
Sensor LEVEL_RES_2_LOW = {CA{"RES_2 LOW"}, 11, false};
Sensor LEVEL_RES_2_MID = {CA{"RES_2 MID"}, 11, false};
Sensor LEVEL_RES_2_HIGH = {CA{"RES_2 HIGH"}, 11, false};
struct Pump {
CA name;
int pin;
};
Pump Pumps[8]; // DOSING PUMPS
Pump WaterPumps[3]; // WATER PUMPS
Pump PUMP_MAIN = {CA{"PUMP MAIN"}, 10};
Pump PUMP_DRAIN_1 = {CA{"PUMP DRAIN 1"}, 10};
Pump PUMP_DRAIN_2 = {CA{"PUMP DRAIN 2"}, 10};
Pump PUMP_SILICA = {CA{"SILICA"}, 11};
Pump PUMP_THRIVE = {CA{"SUPERTHRIVE"}, 12};
Pump PUMP_MICRO = {CA{"MICRO"}, 13};
Pump PUMP_BLOOM = {CA{"BLOOM"}, 14};
Pump PUMP_GROW = {CA{"GROW"}, 15};
Pump PUMP_NECTAR = {CA{"NECTAR"}, 16};
Pump PUMP_WET = {CA{"NATURAL WET"}, 17};
Pump PUMP_PEROXIDE = {CA{"PEROXIDE"}, 18};
struct Formula { // formula refers to a configuration of pump run, mix and rest times
CA name;
int doses;
Pump pumps[8];
int runTimes[8];
int mixTimes[8];
int restTimes[8];
};
Formula Formulas[8];
Formula currentFormula;
Formula SEEDLING = {
CA{"SEEDLING"},
8,
{PUMP_SILICA, PUMP_THRIVE, PUMP_MICRO, PUMP_BLOOM, PUMP_GROW, PUMP_NECTAR, PUMP_WET, PUMP_PEROXIDE},
/*RUN*/ {11, 12, 13, 14, 15, 16, 17, 18},
/*MIX*/ {21, 22, 23, 24, 25, 26, 27, 28},
/*REST*/ {31, 32, 33, 34, 35, 36, 37, 38}
};
Formula GROW_MIN = {
CA{"GROW MIN"},
5,
{PUMP_SILICA, PUMP_THRIVE, PUMP_MICRO, PUMP_BLOOM, PUMP_GROW},
/*RUN*/ {11, 12, 13, 14, 15},
/*MIX*/ {21, 22, 23, 24, 25},
/*REST*/ {31, 32, 33, 34, 35}
};
Formula GROW_MAX = {
CA{"GROW MAX"},
5,
{PUMP_SILICA, PUMP_THRIVE, PUMP_MICRO, PUMP_BLOOM, PUMP_GROW},
/*RUN*/ {11, 12, 13, 14, 15},
/*MIX*/ {21, 22, 23, 24, 25},
/*REST*/ {31, 32, 33, 34, 35}
};
enum mixingStates {
MIX, // mix a formula
DOSE_START,
DOSE_END,
DELAY,
REST,
FINISH,
IDLE
} mixingState;
unsigned long delayStart;
unsigned long delayFor;
mixingStates nextState;
void MixingDelay(unsigned long dur) {
mixingState = DELAY;
delayStart = millis();
delayFor = dur;
}
int currentMixingDelay;
int currentMixingRest;
Pump currentMixingPump;
int mixingIndex = 0;
bool mixingCycleIsRunning = false;
void mixingCycle() {
switch (mixingState) {
case DELAY: // use Delay(duration) and set nextState - delays the mixing cycle for a time
if (millis() - delayStart > delayFor) { // all unsigned long, subtracted, safe millis()
mixingState = nextState; // set the next state
}
break;
case MIX: // start and handle the mixing of a Formula
mixingCycleIsRunning = true;
currentMixingPump = currentFormula.pumps[mixingIndex]; // extract data from Formula structs
currentMixingDelay = currentFormula.mixTimes[mixingIndex];
currentMixingRest = currentFormula.restTimes[mixingIndex];
mixingIndex++; // increment the mixing index after object extraction
if (mixingIndex > currentFormula.doses) {
mixingState = FINISH; // no more pumps to process
} else {
mixingState = DOSE_START; // begin dosing
}
break;
case DOSE_START: // begin dosing a nutrient
digitalWrite(PUMP_MAIN.pin, HIGH); // enable the main pump
digitalWrite(VALVE_MIX.pin, HIGH); // turn the mixer on
digitalWrite(currentMixingPump.pin, HIGH); // turn the dosing pump on
nextState = DOSE_END; // after delay, end dosing for this pump
MixingDelay(currentMixingDelay); // wait with dosing pump on for a time
break;
case DOSE_END: // end the dosing of a nutrient
digitalWrite(currentMixingPump.pin, LOW); // turn the dosing pump off
mixingState = REST; // rest the pump now
break;
case REST: // rest to allow nutrients to settle and to not overheat the main pump
digitalWrite(PUMP_MAIN.pin, LOW); // turn off the pump to rest it
nextState = MIX; // return back to the MIX state which has incremented previously
MixingDelay(currentMixingRest); // wait with main pump off for a time
break;
case FINISH: // perform final tasks for the cycle, use this state to cancel
mixingIndex = 0; // reset the mixing increment
mixingState = IDLE; // after final tasks, idle the mixing until user/timer intervention
digitalWrite(VALVE_MIX.pin, LOW); // turn off the mixing valve
mixingCycleIsRunning = false;
break; // not necessary as idle is next
case IDLE: // idle as in, paused by user during cycle or just not running
break;
}
}
static unsigned long FILLING_CYCLE_DELAY = 600000; // don't try to fill more often than every 10 minutes, probably a day in practic
// static unsigned long FILLING_CYCLE_DELAY = 86400000; // don't try to fill more often than once a day
unsigned long lastFillingCycleRun = millis() + FILLING_CYCLE_DELAY; // initial delayed value
void fillingCycle() { // fill the resvs with nutrient solution from the mixing bucket
if (millis() - lastFillingCycleRun > FILLING_CYCLE_DELAY) {
if (mixingCycleIsRunning) { // don't try to pump anything out while the cycle is active
return; // all for now
}
if (!LEVEL_MIX_LOW.full) { // when out of water entirely (the mixing bucket low sensor is inactive)
// this checks only on low because filling will not happen if mixing was requested which ends at full
digitalWrite(VALVE_RES_1.pin, LOW); // turn off the output valves if they were on
digitalWrite(VALVE_RES_2.pin, LOW); // turn off the output valves if they were on
mixingState = MIX; // formulateify up some nutrificents, sets mixingCycleIsRunning to true
return;
}
// now that there is water, pump it where needed
digitalWrite(PUMP_MAIN.pin, HIGH); // enable the main pump
if (!LEVEL_RES_1_HIGH.full) { // if res 1 is not full, move nutrients to it
digitalWrite(VALVE_RES_1.pin, HIGH); // assume this can be issued repeatedly
return; // return if we are currently doing res 1
} else {
digitalWrite(VALVE_RES_1.pin, LOW); // stop moving nutrients when it is full
}
if (!LEVEL_RES_2_HIGH.full) { // if res 2 is not full, move nutrients to it
digitalWrite(VALVE_RES_2.pin, HIGH);
return;
} else {
digitalWrite(VALVE_RES_2.pin, LOW); // stop moving nutrients when it is full
}
digitalWrite(PUMP_MAIN.pin, LOW); // turn off the main pump
lastFillingCycleRun = millis();
}
}
int LEVEL_TOLERANCE = 400;
void readWaterSensors() {
Sensor currentSensor;
for (int i = 0; i < 9; i++) {
currentSensor = Sensors[i];
int value = analogRead(currentSensor.pin);
if (value < LEVEL_TOLERANCE) { // low value is under water
currentSensor.full = true;
} else {
currentSensor.full = false;
}
}
}
void checkMaximumWaterLevels() { // extra safety shutoff for full things which is safe from my dumb coding errors
// this runs non-blocking and constantly will be trying to shut off the outputs if full.
if (LEVEL_MIX_HIGH.full) { digitalWrite(VALVE_INPUT.pin, LOW); }
if (LEVEL_RES_1_HIGH.full) { digitalWrite(VALVE_RES_1.pin, LOW); }
if (LEVEL_RES_2_HIGH.full) { digitalWrite(VALVE_RES_2.pin, LOW); }
}
void loop() {
mixingCycle();
readWaterSensors();
checkMaximumWaterLevels();
fillingCycle();
}
void setup() {
Serial.begin(9600);
Serial.println("");
Formulas[0] = SEEDLING ;
Formulas[1] = GROW_MIN ;
Formulas[2] = GROW_MAX ;
Valves[0] = VALVE_INPUT ;
Valves[1] = VALVE_MIX ;
Valves[2] = VALVE_RES_1 ;
Valves[3] = VALVE_RES_2 ;
WaterPumps[0] = PUMP_MAIN ;
WaterPumps[1] = PUMP_DRAIN_1 ;
WaterPumps[2] = PUMP_DRAIN_2 ;
Pumps[0] = PUMP_SILICA ;
Pumps[1] = PUMP_THRIVE ;
Pumps[2] = PUMP_MICRO ;
Pumps[3] = PUMP_BLOOM ;
Pumps[4] = PUMP_GROW ;
Pumps[5] = PUMP_NECTAR ;
Pumps[6] = PUMP_WET ;
Pumps[7] = PUMP_PEROXIDE ;
Sensors[0] = LEVEL_MIX_LOW ;
Sensors[1] = LEVEL_MIX_MID ;
Sensors[2] = LEVEL_MIX_HIGH ;
Sensors[3] = LEVEL_RES_1_LOW ;
Sensors[4] = LEVEL_RES_1_MID ;
Sensors[5] = LEVEL_RES_1_HIGH ;
Sensors[6] = LEVEL_RES_2_LOW ;
Sensors[7] = LEVEL_RES_2_MID ;
Sensors[8] = LEVEL_RES_2_HIGH ;
Serial.println("CONFIGURED FORMULAS;");
for (int i = 0; i < 9; i++ ) {
if (Formulas[i].doses) {
Serial.print("FORMULA: "); Serial.println(Formulas[i].name.characters);
for (int j = 0; j < 7; j++) {
if (Formulas[i].pumps[j].pin) {
Serial.print("PUMP: "); Serial.print(Formulas[i].pumps[j].name.characters);
Serial.print(" ON PIN: "); Serial.print(Formulas[i].pumps[j].pin);
Serial.print(" DUR: "); Serial.print(Formulas[i].runTimes[j]);
Serial.print(" MIX: "); Serial.print(Formulas[i].mixTimes[j]);
Serial.print(" REST: "); Serial.println(Formulas[i].restTimes[j]);
}
}
}
}
}
There was other advice that seemed to be leaning toward a minimum number of states. But I believe I agree, and I think anywhere a state is useful one should be used, at least in my little world.
As such, adding a case to deal with waiting for a water level;
case WAIT_FOR_WATER:
if (!LEVEL_MIX_MID.full) {
digitalWrite(VALVE_INPUT.pin, HIGH)
return;
} else {
mixingState = DOSE_START;
return;
}
Where exactly is that going to return to ?
I disagree. The current state is DELAY, it's not lost, you know where yo are. What you lost is where you came from, but it's quite often the case in simple state machines (we don't store usually the previous stage). Here the next state after the delay is "calculated" by checking into the variable NEXT_STATE. it's an OK mean of transition. (it's not worse than storing the value of millis before the transition so that your wait state knows when it started)
So you don't know where you are coming from but you know where you are going to. That's OK.
alternative would be to have dedicated wait states which adds a bit of clarity when reading the code and you don't have to remember to set the NEXT_STATE before jumping there, but you get lots of repeated code if all the states require some delay upon the transition.
The other "problem" that your DELAY introduces, which offends the purists, is allowing a function outside the FSM to tinker with the state variable.
This can be useful, but can usually be avoided. On principal.
This is your mixingCycle() function with the one delay mechanism installed. It uses a variable dwell when it needs to hang in a state for a period.
I tried not to damage the actual functionality, on the same hand I did not test this, but it is the pattern of which I speak. I used "//..." to call out the modifications.
Any state that wants to hang just set dwell to the hang time, and nextState to the destination after hanging.
It's still a bit hacky - with a bit of work we can be in meaningful states at all times and jettison any need for a nextState. Not today.
void mixingCycle() {
//...
static unsigned long dwell; // time to postpone running the FSM, 0 = no delay
static unsigned long lastFSMTime;
if (dwell && (millis() - lastFMSTime < dwell)) return; // DELAY in state
if (dwell) mixingState = nextState;
lastFSMTime = millis();
dwell = 0; // unless we set it to stay in a state. 0 = immindiately
switch (mixingState) {
case MIX: // start and handle the mixing of a Formula
mixingCycleIsRunning = true;
currentMixingPump = currentFormula.pumps[mixingIndex]; // extract data from Formula structs
currentMixingDelay = currentFormula.mixTimes[mixingIndex];
currentMixingRest = currentFormula.restTimes[mixingIndex];
mixingIndex++; // increment the mixing index after object extraction
if (mixingIndex > currentFormula.doses) {
mixingState = FINISH; // no more pumps to process
} else {
mixingState = DOSE_START; // begin dosing
}
break;
case DOSE_START: // begin dosing a nutrient
digitalWrite(PUMP_MAIN.pin, HIGH); // enable the main pump
digitalWrite(VALVE_MIX.pin, HIGH); // turn the mixer on
digitalWrite(currentMixingPump.pin, HIGH); // turn the dosing pump on
nextState = DOSE_END; // after delay, end dosing for this pump
//... not MixingDelay(currentMixingDelay); // wait with dosing pump on for a time
dwell = currentMixingDelay; // hang in this state with dosing pump on for a time
break;
case DOSE_END: // end the dosing of a nutrient
digitalWrite(currentMixingPump.pin, LOW); // turn the dosing pump off
mixingState = REST; // rest the pump now
break;
case REST: // rest to allow nutrients to settle and to not overheat the main pump
digitalWrite(PUMP_MAIN.pin, LOW); // turn off the pump to rest it
nextState = MIX; // return back to the MIX state which has incremented previously
//... not MixingDelay(currentMixingRest); // wait with main pump off for a time
dwell = currentMixingRest; // hang with main pump off for a time
break;
case FINISH: // perform final tasks for the cycle, use this state to cancel
mixingIndex = 0; // reset the mixing increment
mixingState = IDLE; // after final tasks, idle the mixing until user/timer intervention
digitalWrite(VALVE_MIX.pin, LOW); // turn off the mixing valve
mixingCycleIsRunning = false;
break; // not necessary as idle is next
case IDLE: // idle as in, paused by user during cycle or just not running
break;
}
}
HTH
a7
2 Likes
What you lose is what the mechanism doing. I agree it is unimportant or at least not critical, especially in a tiny state machine; I just like to have the state be reflective of what the machine is up to at any moment.
So hanging in "FILLING_TANK" for 7000 milliseconds, not DELAY, that someone looking at the state variable on LEDs or debugging or whatever would have better information.
a7
I see - makes sense too
by the way, the idea (not the code you use for comparison) of the helper class CA is fine, but when I see 13 instances with structures containing an int ( two or more bytes!) pin initialized with value 11 ... I can't help myself, that doesn't look very efficient.
HI @alto777,
with other words
- DOSE_START and DOSE_END are actions/functions performed on a transition
- DOSING (not realized yet) would be the state after DOSE_START and before DOSE_END
The transitions do not necessarily have to be designed as "transition states":
case DOSING:
if (startDosing){
// initialize DOSING
// Do whatever is required here once
startDosing = false;
}
...
boolean endDosing = functionToEndDosing();
if(endDosing) {
// Stop DOSING
// Do whatever is required here once
startDosing = true; // for the next entry
state = WHATEVERCOMESNOW;
}
break;
Do you agree?
I find those state machine where you embed the initialisation within the state quite annoying because they make the case more complicated.
you still need to initialise startDosing to true in the previous state.
it makes sense to include the end of dosing as it's a transition and you prepare for the next stage
I prefer
case XYZ:
•••
if (itsDosingTime()) {
stopXYZ();
startDosing();
state = DOSING;
}
break;
case DOSING:
•••
if(itsEndDosingTime()) {
stopDosing();
startWhateverComesNow();
state = WHATEVERCOMESNOW;
}
break;
==> I find this approach easier to understand et very symmetrical.
When you transition you close what you were doing and prepare for the next stage you know is coming. When you arrive in that state, it's already initialized and you are truly in that state.
3 Likes
Yes.
In fact you raise another area where I have struggled with how to best code a matter, that is states that have some initial things to do, then some real work, then a transition.
Say a state is supposed to fire five missiles, you'd set a count in the one-off, and the termination condition would be having fired the fifth.
So the actual state sometimes is a major "official" state combined with the missile counter or whatever is marking progress in the state. Time itself, I suppose, in this case of DELAY by dwell.
I usually code an additional state, INIT_MISSILES, which either drops through or certainly does not dwell on its way to FIRING_MISSILES.
Transitions to a next state can house some initialisation for that state. This I do, but sorta don't like as it means a state has stuff not strictly related to it. Of course if there is more than one path into the missile firing process, the first stop for all those paths would be the single initialiser. State or a flagged, as you illustrate, first part of a state.
Sry to use missiles as my example. I'm mostly reading today's paper here under the umbrella.
a7
1 Like
Yes. I use that pattern frequently, even if I mildly offend myself. 
a7
Just wanted to show this as a valid possibility.
I have done both in different statemachines depending on the complexity of the application and the readability.
E.g. if you can change to a certain state from several other states a separate "transition state" or an integrated init routine may be a nice solution.
But it looks like personal preferences (which is completely ok). The most important point seems to me not to mix styles ... 
I just haven't put the real pins in yet, they are placeholders for that.
It should be break. But it is essentially the same (might even be the same to the compiler) as return since it will just fall past all other states. It returns back to the non-blocking main loop to process other tasks.