Something is terribly wrong with my code

The while loop can freeze the code if the flow meter pulses stop before the count is reached. Have the ISR toggle the built-in LED each time a pulse arrives. That should show you if you are getting pulse when there is no flow or no pulses when there is flow.

Second thought :

  • if (SolenoidPumpPin==HIGH) : SolenoidPumpPin is the pin number, not the pin state.
  • buttonPin1 has not been explicitly declared as Input. Not mandatory but good practice.

hence this new code :

// YFS201 Flow Sensor Pulse Count Example

#include <LiquidCrystal_I2C.h> 
LiquidCrystal_I2C lcd(0x27, 16, 2); 

const int sensorPin = 2;    // digital input pin connected to the YFS201 sensor
unsigned int pulseCount = 0;  // variable to hold the pulse count

const int buttonPin1 = 12;
int buttonState1 = 0;

const int SolenoidPumpPin=11;
bool PumpRunning = false;

void setup() {
  Serial.begin(9600);       // initialize serial communication at 9600 baud rate
  pinMode(buttonPin1, INPUT); // set the buttonPin1 pin as input
  pinMode(sensorPin, INPUT);  // set the sensor pin as input
  pinMode(SolenoidPumpPin, OUTPUT);
  digitalWrite(SolenoidPumpPin, LOW);

  lcd.init(); 
  lcd.clear();
  lcd.backlight();
}


void loop() 
{
  if(pulseCount<=10)
  {
    buttonState1 = digitalRead(buttonPin1);
    Serial.print("buttonState1 = "); Serial.println(buttonState1);
    if (buttonState1 == HIGH)
    {
      PumpRunning = true;
      digitalWrite(SolenoidPumpPin, HIGH);
    }

    if (PumpRunning)
    {
      if (digitalRead(sensorPin) == HIGH) 
      {  // check if the sensor output is high
          pulseCount++;            // increment the pulse count
          Serial.print("Pulse Count: ");
          Serial.println(pulseCount);  // print the pulse count to the serial monitor
          lcd.setCursor(0, 0); 
          lcd.print(pulseCount);
      }
      delay(500);    // delay to allow for accurate pulse counting
    }  
  }
  else
  {
    digitalWrite(SolenoidPumpPin, LOW);
    PumpRunning = false;
    pulseCount = 0;
  }
}

and its associated Wokwi : BottleFill V1 - Wokwi ESP32, STM32, Arduino Simulator

Pulse count does not actually count pulses but 10th of seconds sensorPin HIGH. (half seconds in my more relaxed code)

Nice. I can't from my tablet here where I am - if the lcd code is still mostly there and functioning, could you throw an 16x2 I2C LCD display in there? Four wires…

TIA or it was just a thought, n'mind.

a7

@TomGeorge I tested it, took the sensor wires as far as possible (pretty far, in inches) from the 12V set up. It didn't change anything. I tried all the three full codes, and the new sketches too which I and @Etienne_74 have been trying, same issue everywhere (and they don't even have a while loop). I have also notcied that the longer the delay() function the less likely the hanging of the system. It has to be a memory issue in my belief. I think we should try getting rid of the while loop. I tried myself, but it kept filling and wouldn't stop. Suggestions and ideas please.

The only thing that a 12V is close to is possibly the arduino itself, which is lying right next to the pump -ve wire (the -ve 12V is basically held close to the Arduino due to some taping here and there, some knotting here and there, some soldering here and there). Could proximity of Arduino to -ve 12V have an effect? And if it can, why does @johnwasser code behave the worst? why don't all codes behave equally bad? Why do codes with delay function behave better?

Modified code with if statement:

buttonState1 = digitalRead(buttonPin1);
        if (buttonState1 == HIGH)
        {            
          lcd.clear();//new
            
            lcd.setCursor(0,0);
            lcd.clear();//new
    
            lcd.setCursor(0, 0); 
            lcd.print("Volume        ml");
            lcd.setCursor(0, 1); 
            lcd.print("Filled        ml");
            lcd.setCursor(10, 0);
            
            lcd.print(volume);
            finalValue=fillVolume;
            //delay(100);//needed with 12v adaptor
            //finalValue=fillVolume;

          
          //pulse_freq = 0;
          //flow = 1.4 * pulse_freq;
          //VolumeSet = finalValue;
        }

        
          
          if(totalMilliLitres < fillVolume)
          {
            digitalWrite(SolenoidPumpPin, HIGH);
            if ((millis() - oldTime) > 1000) 
            
              {
              detachInterrupt(sensorInterrupt);
              flowRate = ((1000.0 / (millis() - oldTime)) * pulseCount) / calibrationFactor;
              oldTime = millis();
              flowMilliLitres = (flowRate / 60) * 1000;
              totalMilliLitres += flowMilliLitres;
              unsigned int frac;
              lcd.setCursor(10, 1);
              lcd.print(totalMilliLitres);
              
              }
              pulseCount = 0;
              attachInterrupt(sensorInterrupt, pulseCounter, FALLING);
          }
          
          else 
          {
              digitalWrite(SolenoidPumpPin, LOW);
              fillVolume = 0;
          }
          
    

Basically I thought now after the test we should try getting rid of the while loop, but since the codes tried by me and @Etienne_74 on the lines of @er_name_not_found suggestions don't have a while loop and still have trouble, I don't see how removing while loop would help. But if it isn't a coding issue, why different problem behaviour in codes with longer or shorter delay function?? Do I need to start moving my arduino away too?

Can you temporary replace the pump and solenoid with proxy LEDs in their stead, and still exercise the logic?

That woukd reduce if not eliminate questions about problems caused from switching the motor/coil loads.

a7

1 Like

Done.

volatile byte pulseCount = 0;
.
.
.

void pulseCounter() {
  pulseCount++;
}

mountainrock : pulseCount is declared as byte (= 8 bit value). Are you sure there is no overflow (>255) ?

1 Like

Dumb it down a little for me please, I don't speak bits and bytes very well :upside_down_face:. What is overflow? How would I know it?

If you only have room for 2 digits, you can't print numbers over 99 : 100 won't fit and the leftmost digit will be lost. You end-up with 00 instead.

A bit is a binary digit (or base 2, it can only be 0 or 1) and a byte is a block of 8 bits so you can't count over 0b11111111.
(0b is the prefix indicating a binary number). 0b11111111 is 255 in decimal and if you add one, it will overflow to 0b00000000 (leftmost digit is lost) and your program will consider the bottle as empty and will thus continue pumping (endlessly).

If you declare pulseCount as a 16 bit integer, it could solve the problem but you will "never know" it was. So you can alter the ISR to set an Arduino pin HIGH in case of an overflow :

void pulseCounter() {
  if(pulseCount=255) digitalWrite(LedPin, HIGH);
  else pulseCount++;
}

or you can display pulseCount instead of, say, finalValue inside the while() loop

To be continued...

1 Like

Fixed == instead of =.

If you do go to a multiple byte integer in the ISR, not only does it need to be volatile, but access to it outside the ISR must be protected.

Most ppl do this below to grab a safe copy:

    noInterrupts();
    int myCopy = pulseCoint;
    interrupts();

Then use myCopy where otherwise it would have been pulseCoint.

HTH

a7

1 Like

But I had a look at the original program and apart from a few things, it looks OK:

  • volatile byte pulseCount = 0; should be declared as "large_enough" : Each pulse is approximately 2.25 milliliters => 1.5 liter is then 667 pulses : an int (16 bit integer) should do. By the way, "Will a byte overflow ?" : YES, it will !
  • solenoidValve is mysterious...
  • unsigned int frac; is unused (compiler warning).

I didn't check the messages management nor the math...

=> I'd say, go for the original program, it should just need these few tweaks...

1 Like

Happens to me all the time, Pascal ':=' was so nice...

2 Likes

@mountainrock I recast your schematic for my own purposes and to alleviate some dizziness studying it was causing.

I moved the gate resistor with respect to the pull down resistor on the MOSFET. Probably not your problem, bet best practice.

I guessed at what might be attached at pin 12. Is my guess correct?

What values are the resistors that are unmarked?

If it looks like I drew it with my finger I can tell you why. :expressionless:

a7

@Etienne_74 thank you so much. You are amazing, you finally caught it. I used an unsigned long int and it finally worked. No issues so far. I don't think there is any reason why there would be in the future, but if there is I'll hop into your message box directly to pick your brains. This had been bothering me for so long. And I learned some very good stuff today too. Thank you.

@alto777 I think the resistances would be standard 1K and 10K, nothing special about this circuit, and the problem isn't with the hardware as we just found out. And yeah, at pin 12 is the button that starts the dance we were stumbling at.

just for educational purpose, could you tell me are there any fallouts of an overkill? like if we used a 32bit declaration?

A 16 bit unsigned integer can count from 0 up to 65535 (half of that for a signed integer : from -32768 up to 32767).

2.25 milliliters times 65535 is 147 liter.

32 bit uint is up to 2^32 (minus 1) = 4 294 967 295 => 9 663 676 liter

Yours to choose...

1 Like

Cool. Can you post the sketch that works now please?

a7

ok, so I made a mistake? I should have declared "signed long int" for 16 bit ? or just int? I was actually reading here. I am still learning all this. And for a 32 bit its uint?

Thanks anyways, I'll study these variable types in detail now that I have been alerted of their significance.

Budd there is no new sketch, its the same old sketch pasted above several times from the beginning, with only the variable type for the pulsecounter changed basis @Etienne_74 's solution. The problem wasn't the sketch, but the overflow which we were not cautious for.

Still if it helps:

//screen and keyboard code starts
#include <Keypad.h>
#include <LiquidCrystal_I2C.h> 
#include <Wire.h>

LiquidCrystal_I2C lcd(0x27, 16, 2); 

#include<stdio.h>
const int ROW_NUM = 4; 
const int COLUMN_NUM = 3; 
char keys[ROW_NUM][COLUMN_NUM] = {
  {'1', '2', '3'},
  {'4', '5', '6'},
  {'7', '8', '9'},
  {'*', '0', '#'}
};
byte pin_rows[ROW_NUM] = {9,8,7,6}; 
byte pin_column[COLUMN_NUM] = {5,4,3}; 
Keypad keypad = Keypad( makeKeymap(keys), pin_rows, pin_column, ROW_NUM, COLUMN_NUM );

char volume[4];
byte volumeCount = 0;
bool edit = 0;
int finalValue = 0;
//screen and keyboard code ends

//flow sensor and other machinery code starts

int flowsensor = 2; // Sensor Input
int sensorInterrupt = 0;
unsigned int SetPoint = 400;
String code = "";
float calibrationFactor = 8;
unsigned long int pulseCount = 0;
float flowRate = 0.0;
unsigned int flowMilliLitres = 0;
unsigned long totalMilliLitres = 0, VolumeSet = 0;
unsigned long oldTime;

int SolenoidPumpPin=11; //was 11 with original design

// constants won't change. They're used here to set pin numbers:
const int buttonPin1 = 12;     // the number of the pushbutton pin for start flow

// variables will change:
int buttonState1 = 0;         // variable for reading the pushbutton status

void pulseCounter() {
 
  pulseCount++;
}

//flow sensor and other machinery code ends

void setup() 
{
  // screen code starts
lcd.init(); 
  lcd.clear();
  lcd.backlight();
  lcd.setCursor(0, 0); 
  lcd.print("Volume        ml");
   lcd.setCursor(0, 1); 
   lcd.print("Filled        ml");
   // screen code ends


  //sensor code starts
  pinMode(SolenoidPumpPin, OUTPUT);
  digitalWrite(SolenoidPumpPin, LOW); //Switch Solenoid ON

totalMilliLitres = 0;
  pinMode(flowsensor, INPUT);
   digitalWrite(flowsensor, HIGH); 
attachInterrupt(sensorInterrupt, pulseCounter, FALLING); // Setup Interrupt

   //sensor code ends

}


void loop() 
{

  buttonState1 = digitalRead(buttonPin1);
  
  char key = keypad.getKey();
  
  if(key) // check if any key was pressed
  {
    if(key == '*') // if * was pressed switch to edit mode
    {
        lcd.setCursor(0,0); // set your cursor at columnt 0, row 0
        lcd.clear();//new
        lcd.print("Enter volume: ");
        edit = true;
        lcd.setCursor(0,1); // set your cursor to second row 
        volumeCount = 0;
        volume[0] = '0';
        volume[1] = '0';
        volume[2] = '0';
        volume[3] = '0';
    }
    
    if(edit && volumeCount < 4 && key != '*' && key != '#') // enter edit mode
    {
        volume[volumeCount] = key; // save key to a char array
        lcd.setCursor(volumeCount,1);  // set your cursor to the next position
        lcd.print(volume[volumeCount]); // print the pressed button to lcd
        volumeCount++; // increment the array index (cursor position)
    }
    
    if(volumeCount == 4 || key == '#') // array.length == 3 OR you pressed #
    {
      if (key == '#' && volumeCount != 4)
      {
        volume[volumeCount] = '\0';
        edit = false; // disable edit mode

        volumeCount = 0; // reset your counter
        //lcd.setCursor(0,0);
        lcd.clear();//new

        lcd.setCursor(0, 0); 
        lcd.print("Volume        ml");
        lcd.setCursor(0, 1); 
        lcd.print("Filled        ml");
        lcd.setCursor(10, 0);
        
        lcd.print(volume); //new
        finalValue = atoi(volume); // save your entered value for further usage
        //volume[0] = '0';
        //volume[1] = '0';
        //volume[2] = '0';
      }
      else
      {
        edit = false; // disable edit mode

        
        volumeCount = 0; // reset your counter
        //lcd.setCursor(0,0);
        lcd.clear();//new
        
        lcd.setCursor(0,0);
        lcd.clear();//new

        lcd.setCursor(0, 0); 
        lcd.print("Volume        ml");
        lcd.setCursor(0, 1); 
        lcd.print("Filled        ml");
        lcd.setCursor(10, 0);
        
        lcd.print(volume); //new
        finalValue = atoi(volume); // save your entered value for further usage
        //volume[0] = '0';
        //volume[1] = '0';
        //volume[2] = '0';

                  
        }
      }
        
    }

    //sensor and other machine code starts
    buttonState1 = digitalRead(buttonPin1);
        if (buttonState1 == HIGH)
        {            
          lcd.clear();//new
            
            lcd.setCursor(0,0);
            lcd.clear();//new
    
            lcd.setCursor(0, 0); 
            lcd.print("Volume        ml");
            lcd.setCursor(0, 1); 
            lcd.print("Filled        ml");
            lcd.setCursor(10, 0);
            
            lcd.print(volume);
            //delay(100);//needed with 12v adaptor

          
          //pulse_freq = 0;
          //flow = 1.4 * pulse_freq;
          //VolumeSet = finalValue;
          
          while(totalMilliLitres < finalValue)
          {
            digitalWrite(SolenoidPumpPin, HIGH);
            if ((millis() - oldTime) > 1000) 
            
              {
              detachInterrupt(sensorInterrupt);
              flowRate = ((1000.0 / (millis() - oldTime)) * pulseCount) / calibrationFactor;
              oldTime = millis();
              flowMilliLitres = (flowRate / 60) * 1000;
              totalMilliLitres += flowMilliLitres;
              //unsigned int frac;
              lcd.setCursor(10, 1);
              lcd.print(totalMilliLitres);
              pulseCount = 0;
              }
              attachInterrupt(sensorInterrupt, pulseCounter, FALLING);
          }
          digitalWrite(SolenoidPumpPin, LOW);
          //VolumeSet = 0;
          totalMilliLitres = 0;
          /*else 
          {
              digitalWrite(SolenoidPumpPin, HIGH);
              VolumeSet = 0;
          }*/
        
        }

          
          
          /*while(flow<=(finalValue))//+1 to match the figures and be on the safe side
          {
            digitalWrite(SolenoidPumpPin, LOW);  
            lcd.setCursor(10, 1);
            int finalflow = flow; //just so we can print it in int format without decimal
            lcd.print(finalflow);
            flow = 1.4 * pulse_freq;
          }
          digitalWrite(SolenoidPumpPin, HIGH);
          lcd.setCursor(10, 1);
          //int finalflow = flow; //required only if we are printing finalflow in next line
          lcd.print(volume); //for hack sake and visuals sake we will print volume here instead of final flow
        }*/
  //sensor and other machinery code ends
  
  
  
}

Suit yourself budd. Just heed @alto777's advice from #56, even if it works now w/o doing.

a7

2 Likes