State machine problem

Question

I am trying to record the pulses from a water flow sensor using a state machine. My goal is to count the pulses until the flow stops (i.e. the rotor inside comes to rest). This will constitute a single reading. if there is no new pulse/interrupt 5 seconds after the rotor is stopped, the total pulse count is printed and pulse count is reset to zero for the subsequent readings.

However, in practice, I get a pulse count even when the sensor rotor is moving. The pulse count gets resets after 5 sec but the pulses between the new reading and until the rotor is stopped are somehow missed.

Could you please help me figure what am I doing wrong?

#include <Arduino.h>
#include <TelnetStream.h>
const byte pulsePin = 22;
byte volatile triggerCounter = 0;
byte newState = 0;
unsigned long newTime = 0;
unsigned long oldTime = 0;
unsigned long pulseCountNew = 0;
portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED;
void IRAM_ATTR handleInterrupt()
{
 portENTER_CRITICAL_ISR(&mux);
 triggerCounter++;
 portEXIT_CRITICAL_ISR(&mux);
}
void stateMachine()
{
 switch (newState)
 {
 case 0 /* Reset */:
 newState = 1;
 break;
 case 1 /* Start */:
 if (triggerCounter > 0)
 {
 newState = 2;
 }
 break;
 case 2 /* Pulse count start */:
 pulseCountNew++;
 triggerCounter--;
 newTime = millis();
 newState = 3;
 break;
 case 3 /* Pulse count stop */:
 if (triggerCounter > 0)
 {
 newState = 2;
 }
 if (newTime - oldTime >= 5000)
 {
 oldTime = newTime;
 newState = 4;
 }
 break;
 case 4 /* Cycle complete */:
 newState = 0;
 TelnetStream.println("State 4");
 break;
 }
}
void setup()
{
 pinMode(pulsePin, INPUT_PULLUP);
 attachInterrupt(digitalPinToInterrupt(pulsePin), handleInterrupt, FALLING);
 TelnetStream.begin();
}
void loop()
{
 if (newState == 4)
 {
 TelnetStream.println(pulseCountNew);
 pulseCountNew = 0;
 }
 stateMachine();
}

• jsotola: The pulse count gets resets after 5 sec ... don't reset the counter ... refer to how millis() is used
• Juraj: do you have less than 255 readings in one batch? you use byte
• Sim Son: The code will have a mich better readability if you use makros or an enum for the state, so you can have something like case RESET:...
• Sim Son: You should have all code that is relevant for the state machine in the switch block. The TelnetStream.println() might be ok in thw loop(), but pulseCountNew=0 should be in case 4.
• Sim Son: triggerCounter is declared as byte, so it can't become negative. The statements triggerCounter-- and if(triggerCounter>0) are error prone and probably don't what you expect.

Answer 1

I do this usually without an extra state variable.

unsigned long readSensor() {
 const unsigned long READING_TIMEOUT_MILLIS = 5000;
 static unsigned long lastReadingMillis;
 static unsigned long lastCounterValue;
 noInterrupts();
 unsigned long counter = triggerCounter;
 interrupts();
 if (counter != lastCounterValue) { // if new reading
 lastReadingMillis = millis(); // reset timout timer
 lastCounterValue = counter; 
 } else if (counter > 0 && (millis() - lastReadingMillis) > READING_TIMEOUT_MILLIS) { // if something was counted and is timeout
 noInterrupts();
 triggerCounter = 0;
 interrupts();
 lastCounterValue = 0;
 lastReadingMillis = 0;
 return counter;
 }
 return 0;
}
void loop() {
 unsigned long value = readSensor();
 if (value) {
 Serial.println(value);
 }
}

sorry, the code is not tested.

• Zaffresky: I thought using a simple state machine would be simpler than using multiple conditional statements. However, your example is much more compact.