Integrating 'simple pulse counter' example into a while loop for a project

Hi, first post on here, new to programming with Arduino and am after some guidance with a particular feature of my first project.

I'm using the 'simple pulse counter example' that I found here: Simple Pulse Counter - Programming Questions - Arduino Forum, It works fine when I use it in a sketch on it's own, but once I integrate this feature into my sketch I am experiencing issues, and am having trouble visualising what the sketch is doing to cause the issue.

This is my sketch:

const unsigned long SECOND = 1000UL;
const unsigned long MINUTE = (SECOND*60UL);
const unsigned long HOUR = (MINUTE*60UL);                     // for the 'clock'                                  

unsigned long waterTimeA = (10*SECOND);                        //Run time 45sec for solenoid A
unsigned long delayTimeA = (20*SECOND);                       //Delay time 3Hrs for solenoid A                          [TIMING]

//-----------------------------------------------------------

const int pumpPin = 2;
const int SolenoidAPin = 3;
const int Hallinput = 12;                                     //Define the physical pins on the board                   [BOARD]
const int LockoutLED = 5;
const int resetSwitch = 6;

unsigned long previousMillis = 0;                             // will store last time Solenoid was updated
int SolenoidStateA = HIGH;                                    // State used to set the solenoid,                        
                                                              // HIGH (pump on) on startup
                                                            
//---------------------------------------------------------------------------------------------------------------------------------------------------------                                                              

                             
                                                               
void setup() 
 {delay (5000);                                           //gives me a few seconds to open the serial monitor before program starts
  Serial.begin(9600);
 
  pinMode(SolenoidAPin, OUTPUT);
  pinMode(pumpPin, OUTPUT); 
  pinMode(Hallinput, INPUT_PULLUP);                         //set-up pins                                               [BOARD][SETUP]
  pinMode(LockoutLED, OUTPUT);
  pinMode(resetSwitch, INPUT_PULLUP);
                             
pinsOn();                                                 // pump etc starts on startup until water time has elapsed.
}

 
void loop()
{
  unsigned long currentMillis = millis();                                                       //Current 'time'
  
 
  if((SolenoidStateA == HIGH) && (currentMillis - previousMillis >= waterTimeA))
  {
    SolenoidStateA = LOW;                                                                       // Turn it off
    previousMillis = currentMillis;                                                             // Remember the time                                       //to do: **************** for '5' times then delay for 9 hrs*****************
    pinsOff();
  }
  else if ((SolenoidStateA == LOW) && (currentMillis - previousMillis >= delayTimeA))
  {
    SolenoidStateA = HIGH;                                                                      // Turn it on
    previousMillis = currentMillis;                                                             // Remember the time
    pinsOn();
  }                                                                                                                     //[TIMING][MAIN LOOP]
}



 //FUNCTIONS-------------//FUNCTIONS-----------//

 void pinsOn(){
    digitalWrite(SolenoidAPin, SolenoidStateA);                                                
    runPumpProtect();
    Serial.println(" pinsOn running") ;                               // Runs @ watering time
}


//-----------------------------------------------------------

 void pinsOff(){
    digitalWrite(SolenoidAPin, SolenoidStateA);                                                
    digitalWrite(pumpPin, SolenoidStateA);                                                   
    digitalWrite(Hallinput, SolenoidStateA);
    Serial.println(" pinsOff running - Solenoid OFF Hall OFF pump OFF ") ;                     // Runs to turn pump etc. off   
}


//-----------------------------------------------------------

 void runPumpProtect(){
  
  digitalWrite(pumpPin, HIGH);
  Serial.println("PumpProtect running");
  delay (10);                                                             // delay for pump priming
  hallSensor();                                                           // runs Hall sensor to check flow is OK
 }


//-----------------------------------------------------------

void hallSensor(){                                                                   
  int pulse = 0;                                                        // Variable for saving pulses count.
  int var = 0;
  unsigned long currentTime = millis();
  
while (currentTime + 5000 > millis()){
  if(digitalRead(Hallinput) > var)
  {
    var = 1;
    pulse++;
   
    Serial.print(pulse);
    Serial.print(" pulse recieved ");
  }
  
  if(digitalRead(Hallinput) == 0) {var = 0;}
  
  delay(50);                                                          // debouncing.
}                                                                     //pulses counted over a period of 5seconds (5000 millis)

if (pulse >= 5){
  Serial.print(" 5+ pulses recieved- flow is OK");                    //Continues rest of program until waterTimeA =! TRUE
}
else if (pulse <5){
  Serial.print("LOCKOUT");                                           //lockout requires all pins = OFF
  LOCKOUT();                                                         //runs lockout if <5 pulses recieved in time frame
}
else{
  Serial.print("not sure whats happening");                          // this should never run
  LOCKOUT();
}

}


//-----------------------------------------------------------

void LOCKOUT(){                                                           //Runs if Hall sensor recieves <5 pulses in 5 sec. **Dry run protection for the pump.**
   digitalWrite(SolenoidAPin, LOW);                                                
   digitalWrite(pumpPin, LOW);                                                   
   digitalWrite(Hallinput, LOW);
   digitalWrite(LockoutLED, HIGH);
   digitalWrite(resetSwitch, HIGH);
    Serial.println("EMERGENCY LOCKOUT") ;                    
  int L = 1;

  while (L == 1) {
    if (digitalRead (resetSwitch)== LOW){
      Serial.println("reset pressed");
      digitalWrite(LockoutLED, LOW);
      break;} 
      };
 
    }

In a nutshell, this will water my greenhouse, the plan is for it to water the tomatoes for 45 seconds every 3 hours, but I have changed the timing in this example to water for 10seconds every 20 seconds and to print what it's doing to the serial monitor, just so i can visualise what is happening.

The problem I am facing is in the hallSensor() function. I'm planning on using a hall effect water flow sensor to count the water flow over 5 seconds once the pump is started, if the water flow sensor detects pulses (ie the water is flowing) then the pump will continue to run for the full 45 second watering time, if the flow sensor detects no water flow this would mean the water butt is empty and the program would run an emergency lockout code to provide protection to the pump to stop it running dry. (reset switch would need to be pressed to resume normal operation.)

Like I said before, the 'simple pulse counter example' (Link above) works well on its own, but once i use it in my sketch, it works ok in the first run through the loop- it detects no flow and then goes into 'Lockout' (runs the lockout() function). Once I hit my reset switch (pin 6) it runs the code again then the pulse counter is counting loads of pulses in the 5 second time window meaning the 'pump' is continuing to run when it shouldn't.

If anyone can give me some help visualising what is going on to make the code behave like this I would be grateful, i suspect the fault lies with the while loop in the HallSensor() function, but can't fathom out why, as it works when running as it's own example.

If anyone would like to make other suggestions to simplify the code or tell me what other noob errors I've made then feel free. If you need any more info or clarification please let me know. Using an Arduino Leonardo.

Thank you for any guidance.

This will not work all the time.

(currentTime + 5000 > millis())

Stick with unsigned subtraction.

It is also inside of a while loop that blocks everything else for... usually 5 seconds but much longer when the cracked time expression above does fail.

You would do better at reading the Hall sensor in void loop() and have that function change variables the other code uses.

It is best not to use WHILE. Instead, use IF and allow loop() to do the repetition.

Something like

if(millis() - currentTime < 5000) {
   if(digitalRead(Hallinput) > var)
   // etc

Have a look at how the code is organized in Several Things at a Time

Note how each function runs very briefly and returns to loop() so the next one can be called. None of the functions tries to complete a task in one call. And there may be dozens of calls to a function before it is actually time for it to do anything.

...R

Thanks for the quick replies, I think i'm once again pointing in the right direction, I will start messing around with the code again later.

No doubt i'll have more questions when it comes to connecting all the components up and putting this system in to practice.

Thanks.

Just for completeness:

I have decided to keep the while loop, reason being I want all processes to stop and for the sketch to evaluate how much flow is through the pipe for 5 seconds before making a decision on whether to continue or lockout.

While loop now looks like this:

void hallSensor(){                                                                   
  int pulse = 0;                                                        // Variable for saving pulses count.
  bool var = false;
  


unsigned long startTime = millis();
unsigned long endTime = startTime;


while ((endTime - startTime) <= 5000) 
{
  if((digitalRead(Hallinput) == LOW) && (var == false))
  {
    pulse++;
    var = true;
  
    Serial.print(pulse);
    Serial.print(" pulse recieved ");
  }
  else if ((digitalRead(Hallinput) == HIGH) && (var == true)){var = false;}
  
  
  
  delay(50); 

endTime = millis();                                          //pulses counted over a period of 5seconds (5000 millis)
}

BUT! most importantly I have found the culprit that kept causing the pulse counter to count when it shouldn't.....

in the pinsOff() function

digitalWrite(Hallinput, SolenoidStateA);

I was writing a value to the input pin when I shouldn't have been. That's why it was working first time around and not subsequent times.

Again, thank you both for the help

iainjdavies:
I have decided to keep the while loop, reason being I want all processes to stop and for the sketch to evaluate how much flow is through the pipe for 5 seconds before making a decision on whether to continue or lockout.

For information, it is quite possible to achieve that effect without resorting to WHILE, even if, in this case the use of WHILE does not cause other problems.

...R

You could get more accuracy using micros() timing instead of millis(). Millis can be off +/- 1.

unsigned long startTime = millis();
unsigned long endTime = startTime;


while ((endTime - startTime) <= 5000)
{

I'm pretty sure that that <= 5000 millis should be <= 4999 or simply < 5000 since the count starts at 0.

With micros, add 3 zeros.

iainjdavies:
Just for completeness:

I have decided to keep the while loop, reason being I want all processes to stop and for the sketch to evaluate how much flow is through the pipe for 5 seconds before making a decision on whether to continue or lockout.

Yep, you can always use a hammer to drive in a screw. Still doesn’t make it the correct tool for the job.

You could get more accuracy using micros() timing instead of millis(). Millis can be off +/- 1.

That's helpful, thanks, I've looked at the micros function micros() - Arduino Reference and can see it says:

"This number will overflow (go back to zero), after approximately 70 minutes"

So this will offer best accuracy for timing over a few seconds/minutes, not sure if my current sketch will need to be that accurate timing wise, as long as it takes a few seconds to assess what the flow is like in the pipe, thats fine for me.
I am thinking of trying to write a sketch for a 'Binary wall clock' for my next project, so I will remember micros() for when that time comes.

Yep, you can always use a hammer to drive in a screw. Still doesn't make it the correct tool for the job.

OK, that made me laugh! Yes, I am aware that the while loop will stop everything else from happening while its stuck in the loop. However, the sensor I've got is a water flow sensor, pic below:

As that blade is spinning it will be going true/false/true/false quite often, what I don't want is for the sketch to be executing some line of code further up and miss the readings from the sensor, for example if i used an IF statement to only read the sensor once everytime through the loop(), it may be false everytime a reading is taken........probability says not, but the possibility is still there.

I have thought of using interrupts in order to count all the pulses but this seems like over engineering when the while loop would do the job to a satisfactory (for this project) standard anyway.

Tell me if my thought process here makes sense, or am i missing something

iainjdavies:
As that blade is spinning it will be going true/false/true/false quite often, what I don't want is for the sketch to be executing some line of code further up and miss the readings from the sensor, for example if i used an IF statement to only read the sensor once everytime through the loop(), it may be false everytime a reading is taken........probability says not, but the possibility is still there.

I have thought of using interrupts in order to count all the pulses but this seems like over engineering when the while loop would do the job to a satisfactory (for this project) standard anyway.

A lot depends on the frequency of the pulses. If loop() repeats fast enough so that it does not miss a pulse then polling is fine. if the pulses happen too fast for that then an interrupt would be appropriate.

Using a WHILE loop is a way to poll frequently during a period when the rest of the program is paused. If that works for a particular project then there is nothing wrong with it. However this can cause problem if at a later stage the program is extended to include other features that don't like having to wait while the pulses are counted.

...R

iainjdavies:

You could get more accuracy using micros() timing instead of millis(). Millis can be off +/- 1.

That's helpful, thanks, I've looked at the micros function micros() - Arduino Reference and can see it says:

"This number will overflow (go back to zero), after approximately 70 minutes"

So this will offer best accuracy for timing over a few seconds/minutes, not sure if my current sketch will need to be that accurate timing wise, as long as it takes a few seconds to assess what the flow is like in the pipe, thats fine for me.
I am thinking of trying to write a sketch for a 'Binary wall clock' for my next project, so I will remember micros() for when that time comes.

And there's the whole millis skips values meaning that the unsigned subtraction of end - start can be +/- 1 milli.
When single millis cound, use micros.

Yep, you can always use a hammer to drive in a screw. Still doesn't make it the correct tool for the job.

OK, that made me laugh! Yes, I am aware that the while loop will stop everything else from happening while its stuck in the loop. However, the sensor I've got is a water flow sensor, pic below:

As that blade is spinning it will be going true/false/true/false quite often, what I don't want is for the sketch to be executing some line of code further up and miss the readings from the sensor, for example if i used an IF statement to only read the sensor once everytime through the loop(), it may be false everytime a reading is taken........probability says not, but the possibility is still there.

I have thought of using interrupts in order to count all the pulses but this seems like over engineering when the while loop would do the job to a satisfactory (for this project) standard anyway.

Tell me if my thought process here makes sense, or am i missing something

Reality check.

Just how fast is that wheel going to turn? You don't need to check the sense pin more than 50,000 times a second, do you? That should read HIGH a few times in a row. You can do that with polling, one peek every 20 usecs or less.

Just how slow is that wheel going to turn? The resolution is dismal below about 300 RPM. If you can see through it's possible to use IR beam-break to trigger on every spoke.

Just a thought. If I put a magnet on a piece of ferrous material, it becomes magnetized. Molded into a plastic rotor, all the vanes have field.

With 1 spot to sense per rev, time between updates can get long even as turn rate changes.

But if you can mount a small bar magnet across the wheel hub, the field will turn with the wheel.
Don't just detect it, a linear Hall sensor gives you field strength along the sensor axis. You can make 8000 analog reads a second and still have cycles to do something with them, just avoid using float/double when using smaller working units gives integers "extra places" when converting to larger display units where decimal places get printed