State machine design?

yes.

why not simply have 2 explicit states making the state machine more maintainable? and give the states meaningful names

and get rid of the states that simply jump to another state?

a state should have some event that causes it to jump to another state

I am not sure what you mean by this?

So the machine must be non-blocking. There are other routines that will run on loop, I have not got to adding them yet but they include checking water level sensors and user button interaction.

Checking water level sensors HAS TO be its own thing that can control everything else and override everything else at all times. Otherwise I could flood the room! There are so many concerns like this that are not expressed in the code but which were considered.

I actually have 18 level sensors, 9 I don't have yet, but 9 of them are light-sensor type and 9 of them are no-drill I think electric type, haven't got one yet. They will be in pairs and checked as either sensor reporting full.

no problem at all. Your microcontroller can run several state machines "in near parallel".

Sure. And if all your code is written non-blocking it will well do this.

non-blocking means that all, each and every function works this way

quickly jump in - quickly jump out

    case DELAY: // use Delay(duration)
        if (millis() - DELAY_START < DELAY_FOR) {
            STATE = NEXT_STATE;
        }
        break;

this looks like some variable state where NEXT_STATE, DELAY_START and DELAY_END are variables that allow this state to be used for multiple purposes

since you code isn't that complicate why not have explicit states that wait on some timer to expire

looking at you code, it looks like the DOES_END and MIX states need to occur for some specified amount of time before transitioning to the next state. is this correct?

Yes. thank you. This is what my intent is. The prototype works this way with (essentially) 3 functions on loop, all non-blocking. I very much like this approach. Learning about state machines has helped in other arenas for me.
I am definitely reaching beyond my skill here, but this aint my first rodeo, so all your input is very helpful in validating the beginning of just one more of many rewrites, I am sure.
This version is WAY better than what I had, as a concept, at a minimum. Far more maintainable and editable at least.

Yes. And this machine ostensibly runs for years on end so millis() must be used very carefully. So with this approach, I am trying to make it easy to write in a delay and easy to respect millis.

So handling delays that happen normally in this routine, part of the overall machine, must be very clear and careful and easy to edit.

look this over

// -------------------------------------
const byte PinStart = A1;

unsigned long t0;
unsigned long tPeriod;

// -----------------------------------------------------------------------------
void loop() {
    switch (state) {
    case MIX:
        currentPump = CurrentFormula.pumps[mixingIndex];
        mixingIndex++;
        if (mixingIndex < 7) {
            state = IDLE;
        }
        else {
            state   = DOSE_START;
            t0      = millis();
            tPeriod = CurrentFormula.mix_times[mixingIndex];
            digitalWrite(currentPump.pin, HIGH);
        }
        break;

    case DOSE_START:
        if (millis () - t0 > tPeriod)
            digitalWrite(currentPump.pin, LOW);
            state   = REST;
            t0      = millis();
            tPeriod = CurrentFormula.rest_times[mixingIndex];
        break;

    case REST:
        if (millis () - t0 > tPeriod)  {
            state = MIX;
        }
        break;

    case IDLE: // waiting for a button to change the state
        if (LOW == digitalRead (PinStart))
            state = MIX;
        break;
    }
}

This careful use is to do two things

  1. consequently using unsigned longs for each and everything that has to do with the timing
  2. calculate timing with differencies

example
if (currentMillis - startTime) > delayTimeInMilliseconds)

unsigned integers don't have negative numbers

in usnigned integers
1 - 10 = +9 Plus nine (= unsigned no negative numbers)

And this is the reason why timing based on millis() even works if millis() has a rollover from
max to zero

should this be if (mixingIndex > y)

So it seems you have moved the features of DOSE_END in the prior, into DOSE_START, and moved the features of DOSE_START in the prior, into the top level of MIX. You have also removed the abstraction of the custom Delay()
I feel like this removes one state, and removes the need for NEXT_STATE, but initially seems more complex to me. I will spend some more time with it.

It also removes the ability to easily know that the routine is in a delay state, if one wanted to know that. Some states are definitely there for 'tracking purposes'.

I have written a WOKWI-Demo-code that is based on your concept code that you posted above
Just klick on the link then you will see the code

best regards Stefan

1 Like

do you just want to know that it's in some generic delay state or do you want to know that your in the mixing or dosing state

tracking purposes can be made by an array of c_strings where that c_string that belongs to its state is displayed on the serial monitor or on a LC-display

or simpler with switching on/off LEDs
or
using a 2 digit 7-segment display to show a number related to the sate

I think that the whole Delay() and naming is going to end up better named as MixingDelay() etc... this is really just the delay for the mixing cycle.

So here are some requirements to maybe help clarify - these are not all coded in but the "bigness" of framework is meant to include them.

Must be able to sense water levels and take action to disable relays etc... (non blocking routine on loop)
Must be able to adjust pre-programmed Formulas according to user input or an RTC routine (also non blocking on loop which would set STATE in various machines)
-routine for pre-filling
-routine for mixing (running a Formula), clock or user adjusted all of this...
-routine for pumping into resv's until full
-routine for draining resv's
-ability to list a long-duration routine based on RTC timing relative to an EEPROM stored starting date
-planned (since it does so much of this) - keep humidifier resv full
-routine for flushing after run (and various other non-feeding routines)
-planned - automatic PH of solution (which usually is fine anyway but why not)

That kinda came out as a bunch of notes. But it shows what this is intended to do, if one were interested.

And here's a current pic

3 Likes

there's no reason a state can't transition on multiple events. while in some state waiting for a timer to expire, it can also monitor some other input and jump to the appropriate state if that event occurs.

having unique states that wait on a timer allow those state to handle specific events and handle them uniquely that may not be as easy to do in a generic delay state

Here is the version that prints a state-change to the serial monitor only once but immidiately if the variable STATE changes

It uses this C-string array for doing this

// array of chars so called "c_string" the second index must be bigger than the longest string
const char myStateNames[][24] = {
  //234567901234567901234567890
  "wait for button",
  "start dosing",
  "wait for dosing to end",
  //2345679012345679012345
  "wait for resting to end"
};

best regards Stefan

This I had planned to handle by just issuing LCD.print when needed, I have a routine that will print my "CA" character array to the LCD. This seems to be working great as a simple drop in replacement for string and it doesn't seem to have a huge amount of memory impact. The problem wasn't status messages, it was the current activities that belong with those messages. (I mean to say, which pump, which part of the cycle, which valve, etc)

Thank you for sharing that and I will keep it in mind for the future.