Code check please - [SOLVED]

Hello lovely people!

Should start with the fact that I am very new to this but keen to learn...
I wrote a code for some project but can't get exactly what I want out of it...
First, I can't get it to stop as soon as level changes...
Second, due to the first problem not sure if it will cycle the pumps...
Would you be able to help me with this, or explain where are the mistakes and how to solve it?
Many thanks in advance.

Here is the code:

// C++ code

const int potentiometerPin = A5;
const int PUMP1_PIN = 9;
const int PUMP2_PIN = 8;
int PrimaryPump = 0; // Assuming 0 for Pump 1, 1 for Pump 2 (can be modified in loop)
const int HIGH_LED_PIN = 7;
const int LOW_LED_PIN = 6;
const int ALARM_PIN = 12;

int primaryPumpPin;
int secondaryPumpPin;

void setup() {
  pinMode(PUMP1_PIN, OUTPUT);    // Pump 1 Output;
  pinMode(PUMP2_PIN, OUTPUT);    // Pump 2 Output;
  pinMode(HIGH_LED_PIN, OUTPUT);  // Red LED to indicate high water level;
  pinMode(LOW_LED_PIN, OUTPUT);   // Green LED to indicate Low water level;
  pinMode(ALARM_PIN, OUTPUT);    // Alarm buzzer

  // Set pump pins based on PrimaryPump (can be adjusted in loop if needed)
  primaryPumpPin = PrimaryPump == 0 ? PUMP1_PIN : PUMP2_PIN;
  secondaryPumpPin = PrimaryPump == 0 ? PUMP2_PIN : PUMP1_PIN;
}

void loop() {
  int waterLevel = analogRead(potentiometerPin);

  // Handle Low Water Level
  if (waterLevel <= 200) {
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, HIGH); // Green LED ON
    digitalWrite(primaryPumpPin, LOW); // Stop Pump (assuming any pump was running)
    digitalWrite(secondaryPumpPin, LOW); 
  } else if (waterLevel > 200 && waterLevel < 800) {
    // Normal Level - Turn off LEDs and Alarm
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, LOW); // Green LED Off
    digitalWrite(HIGH_LED_PIN, LOW); // Red LED Off 
  } else if (waterLevel >= 800) {
    // High Level Detected - Red LED ON and activate Pump sequence
    digitalWrite(HIGH_LED_PIN, HIGH);
    delay(5000);

    digitalWrite(primaryPumpPin, HIGH);  // Start Pump 1
    delay(30000);

    if (waterLevel > 200) {
      // Low Level NOT reached after 30s with Pump 1 - Flash Red LED
      digitalWrite(HIGH_LED_PIN, HIGH);
      delay(1000);
      digitalWrite(HIGH_LED_PIN, LOW);
      delay(1000);

      digitalWrite(secondaryPumpPin, HIGH); // Start Pump 2
      delay(30000);
      digitalWrite(ALARM_PIN, HIGH);       // Alarm ON
    } else {
      // Level is not Low - Start Pump 2 (assuming level is normal)
      digitalWrite(secondaryPumpPin, HIGH);
      delay(30000);
    }
  }
}

Your motors should not be fed from a pin. You need mosfets to control the motors.
Or a relay or a transistor....

Your code uses delay(30000).
That is 30 seconds doing nothing.
You need to use timers and non blocking code (no delay() anymore).
Look at the blinking without delay example in the IDE.
You may also want to use a state machine. On this forum there are excellent tutorials on this subject.

By the way: compliments for a clear text, a clear diagram and code in code tags!

1 Like

I've made some comments about the structure of your if statements:

  if (waterLevel <= 200) {
    // Ok
  } else if (waterLevel > 200 && waterLevel < 800) {
    // At this point, we know the water level is over 200, there is no need to check that again
  } else if (waterLevel >= 800) {
    // At this point, we know water level must be 800 or over, no need to check that again
    if (waterLevel > 200) {
      // At this point, we know water level is 800 or over.  No point checking if it's over 200.
    } else {
      // At this point, we know that water level is over 800. It can't be under 200. Therefore THIS CODE WILL NEVER EXECUTE!
    }
  }
1 Like

Hello George

Welcome to the world's best Arduino forum ever.

Replace the if/ifelse/if salat by a range-based switch/case arrangement.
As already mentioned the call of the delay() function destroyes the expected realtime behaiviour of the sketch.
A design and coding of an event- and timer-controlled OOP-FSM will be helpfull.

Have a nice day and enjoy coding in C++.

1 Like

Hi @George ,

as @paulpaulson posted a Finite State Machine (FSM) is a good tool to solve applications like yours.

I implemented a state machine into your sketch; feel free to test it here:

Sketch
/*

  Forum: https://forum.arduino.cc/t/code-check-please/1240893/2
  Wokwi: https://wokwi.com/projects/393602101966308353

  Sketch from TO at 2024/03/28
  Modified by ec2021

*/
// C++ code


enum States {UNKNOWN, LEVELLOW, LEVELNORMAL, LEVELHIGH};
States state     = LEVELNORMAL;
States lastState = UNKNOWN;

const unsigned long waitBeforePumpTwo = 30000; 
const int potentiometerPin = A5;
const int PUMP1_PIN = 9;
const int PUMP2_PIN = 8;
int PrimaryPump = 0; // Assuming 0 for Pump 1, 1 for Pump 2 (can be modified in loop)
const int HIGH_LED_PIN = 7;
const int LOW_LED_PIN = 6;
const int ALARM_PIN = 12;

int primaryPumpPin;
int secondaryPumpPin;
boolean flashRed = false;
boolean secondPumpOn = false;
unsigned long levelHighStartTime;

void setup() {
  Serial.begin(115200);
  pinMode(PUMP1_PIN, OUTPUT);    // Pump 1 Output;
  pinMode(PUMP2_PIN, OUTPUT);    // Pump 2 Output;
  pinMode(HIGH_LED_PIN, OUTPUT);  // Red LED to indicate high water level;
  pinMode(LOW_LED_PIN, OUTPUT);   // Green LED to indicate Low water level;
  pinMode(ALARM_PIN, OUTPUT);    // Alarm buzzer

  // Set pump pins based on PrimaryPump (can be adjusted in loop if needed)
  primaryPumpPin = PrimaryPump == 0 ? PUMP1_PIN : PUMP2_PIN;
  secondaryPumpPin = PrimaryPump == 0 ? PUMP2_PIN : PUMP1_PIN;
}

void loop() {
  checkLevel();
  stateMachine();
  flashRedLed();
}

void checkLevel() {
  int waterLevel = analogRead(potentiometerPin);
  // Handle Low Water Level
  switch (waterLevel) {
    case 0 ... 200 :
      state = LEVELLOW;
      break;
    case 201 ... 799 :
      state = LEVELNORMAL;
      break;
    case 800 ... 1023:
      state = LEVELHIGH;
      break;
    default:
      Serial.println("Error") ;
  }
}

void stateMachine() {
  switch (state) {
    case LEVELLOW:
      lowLevel();
      break;
    case LEVELNORMAL:
      normalLevel();
      break;
    case LEVELHIGH:
      highLevel();
      break;
  }
}

void lowLevel() {
  if (lastState != LEVELLOW) {
    Serial.println("LOW");
    flashRed = false;
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, HIGH); // Green LED ON
    digitalWrite(HIGH_LED_PIN, LOW); // Red LED Off
    digitalWrite(primaryPumpPin, LOW); // Stop Pump (assuming any pump was running)
    digitalWrite(secondaryPumpPin, LOW);
    secondPumpOn = false;
    lastState = state;
  }
}

void normalLevel() {
  // Normal Level - Turn off LEDs and Alarm
  if (lastState != LEVELNORMAL) {
    Serial.println("NORMAL");
    flashRed = false;
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, LOW); // Green LED Off
    digitalWrite(HIGH_LED_PIN, LOW); // Red LED Off
    lastState = state;
  }
}

void highLevel() {
  // High Level Detected - Red LED ON and activate Pump sequence
  if (lastState != LEVELHIGH) {
    Serial.println("HIGH");
    flashRed = false;
    digitalWrite(ALARM_PIN, HIGH);  // Alarm On
    digitalWrite(LOW_LED_PIN, LOW); // Green LED Off
    digitalWrite(HIGH_LED_PIN, HIGH); // Red LED On
    digitalWrite(primaryPumpPin, HIGH);  // Start Pump 1
    levelHighStartTime = millis();  // Store the start time of LEVELHIGH here
    lastState = state;
  }
  // Check if waitBeforePumpTwo has expired after start of the first pump
  // If it has expired and pump 2 is off switch it on
  if (millis() - levelHighStartTime >= waitBeforePumpTwo && !secondPumpOn) {
    startSecondPump();
  }
// The following part is not required but nice for this demo ...
  static unsigned long lastPrint = 0;
  if (millis() - lastPrint >= 1000 && !secondPumpOn) {
    lastPrint = millis();
    Serial.print(".");
  }
}

void startSecondPump() {
  // and flash Red Led
  Serial.println("Start Second Pump");
  flashRed = true;
  secondPumpOn = true;
  digitalWrite(ALARM_PIN, HIGH);
  digitalWrite(secondaryPumpPin, HIGH); // Start Pump 2
}

void flashRedLed() {
  static unsigned long lastChange = 0;
  static byte ledState = HIGH;
  if (!flashRed) {
    return;
  }
  if (millis() - lastChange > 300) {
    lastChange = millis();
    digitalWrite(HIGH_LED_PIN, ledState);
    ledState = !ledState;
  }
}

If you are interested in some basics regarding state machines just google for "arduino state machine" or check this out

There are also tutorials in this forum:
https://forum.arduino.cc/t/yet-another-finite-state-machine-introduction/1198997

Good luck!
ec2021

3 Likes

This is amazing info all of you provided!
Thank you all very much!
I am already through the examples you linked in!
Could you guys point me in the right direction on how to cycle duty pump every cycle?
I googled abit about it and tried to write something but looking at what I wrote yesterday... even I can't understand it... (this is me on a 4th night shift trying to engage my brain at a "witching hour"...) Could I use FSM? not sure how but back of my brain shouting "YESSSS"...

Your code resembles a dual, alternating pump system, but is it a pump up (filling) or pump down (emptying) type system? What type of level sensor(s) will you use?

FSM is the way to go.
Not easy at the start, as it another way of thinking is needed.
But this different approach will make things way easier in the end...

2 Likes

Here is the revised code . There were few small errors. (My code could be wrong)

// C++ code

const int potentiometerPin = A5;
const int PUMP1_PIN = 9;
const int PUMP2_PIN = 8;
const int PRIMARY_PUMP = 0; // Pump 1 as primary pump
const int HIGH_LED_PIN = 7;
const int LOW_LED_PIN = 6;
const int ALARM_PIN = 12;

// Water level thresholds
const int LOW_WATER_THRESHOLD = 200;
const int HIGH_WATER_THRESHOLD = 800;

// Pump durations
const unsigned long PUMP_DURATION = 30000; // 30 seconds

// Function prototypes
void startPump(int pumpPin);
void stopPump(int pumpPin);
void flashRedLED();

void setup() {
  pinMode(PUMP1_PIN, OUTPUT);    // Pump 1 Output;
  pinMode(PUMP2_PIN, OUTPUT);    // Pump 2 Output;
  pinMode(HIGH_LED_PIN, OUTPUT);  // Red LED to indicate high water level;
  pinMode(LOW_LED_PIN, OUTPUT);   // Green LED to indicate Low water level;
  pinMode(ALARM_PIN, OUTPUT);    // Alarm buzzer
}

void loop() {
  int waterLevel = analogRead(potentiometerPin);

  if (waterLevel <= LOW_WATER_THRESHOLD) {
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, HIGH); // Green LED ON
    stopPump(PUMP1_PIN);
    stopPump(PUMP2_PIN);
  } else if (waterLevel >= HIGH_WATER_THRESHOLD) {
    digitalWrite(ALARM_PIN, HIGH);  // Alarm On
    digitalWrite(HIGH_LED_PIN, HIGH); // Red LED ON
    startPump(PUMP1_PIN);
    delay(PUMP_DURATION);
    // Check water level after primary pump
    waterLevel = analogRead(potentiometerPin);
    if (waterLevel >= HIGH_WATER_THRESHOLD) {
      flashRedLED();
      startPump(PUMP2_PIN);
    }
  } else {
    // Water level is normal
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(HIGH_LED_PIN, LOW); // Red LED Off 
    digitalWrite(LOW_LED_PIN, LOW); // Green LED Off
    stopPump(PUMP1_PIN);
    stopPump(PUMP2_PIN);
  }
}

void startPump(int pumpPin) {
  digitalWrite(pumpPin, HIGH);
}

void stopPump(int pumpPin) {
  digitalWrite(pumpPin, LOW);
}

void flashRedLED() {
  for (int i = 0; i < 3; i++) {
    digitalWrite(HIGH_LED_PIN, HIGH);
    delay(500);
    digitalWrite(HIGH_LED_PIN, LOW);
    delay(500);
  }
}

1 Like

Awesome guys, thanks a lot!

Just a hint:

The state machine approach is non-blocking. The advantage is that the water level is measured continuously and the pump/pumps stop immediately when low level is reached

With the use of delay this is not the case.

This may not happen in your application but prevents running pump/s while the level becomes too low...

1 Like

apologies for disappearing, had some health issues with visit to hospital...

It is pump down system.
So every cycle pump is required it should change the duty pumps meaning pump 1 starts then pump 2 joins if required first cycle, pump 2 starts then pump 2 joins if required second cycle, pump 1 starts then pump 2 joins if required and so on...

???

Here's thread gets around to offering a simulated tank emptying system with two pumps.

If one isn't doing the job, the other is added. If neither can do it, an alarm is raised.

First pump to act alternates between the two available to level the wear.

The code is two parts. One simulates the tank filling and being emptied, ignore or marvel at that.

The other is the solution logic that would be needed if you had a real tank and two pumps.

Four wires connect the two parts.

The solution logic reads tanks sensors on its inputs, wired to outputs provide by the tank simulator.

The solution logic controls two pumps on its outputs, wired to inputs on the simulator that tell it to run its simulated pumps.

The slide fader sets the fill rate. The slide switch turns on and off the filling. Fun!

a7

1 Like

float with two positions Low and High.

ok, thanks, I'll have a look! :+1:

When water is above the float, is the switch closed (ON) or open (OFF)?

So two limit switches - both NO.
LOW - closed when low
HIGH - closed when high

What is normal, water BELOW the switch, or ABOVE ?

I modified the Wokwi simulation to work with a switch that emulates the limit switches:

https://wokwi.com/projects/395791344414641153

Sketch:

/*

  Forum: https://forum.arduino.cc/t/code-check-please/1240893/2
  (first Version Wokwi: https://wokwi.com/projects/393602101966308353)
  Wokwi: https://wokwi.com/projects/395791344414641153

  Sketch from TO at 2024/03/28
  Modified by ec2021 2024/04/21

*/
// C++ code


enum States {UNKNOWN, LEVELLOW, LEVELNORMAL, LEVELHIGH};
States state     = LEVELNORMAL;
States lastState = UNKNOWN;

const unsigned long waitBeforePumpTwo =  4000; //30000;
const byte PUMP1_PIN = 9;
const byte PUMP2_PIN = 8;
const byte LOW_LEVEL_PIN = 5;
const byte HIGH_LEVEL_PIN = 4;
int PrimaryPump = 0; // Assuming 0 for Pump 1, 1 for Pump 2 (can be modified in loop)
const byte HIGH_LED_PIN = 7;
const byte LOW_LED_PIN = 6;
const byte ALARM_PIN = 12;

int primaryPumpPin;
int secondaryPumpPin;
boolean flashRed = false;
boolean secondPumpOn = false;
unsigned long levelHighStartTime;

void setup() {
  Serial.begin(115200);
  pinMode(PUMP1_PIN, OUTPUT);    // Pump 1 Output;
  pinMode(PUMP2_PIN, OUTPUT);    // Pump 2 Output;
  pinMode(HIGH_LED_PIN, OUTPUT);  // Red LED to indicate high water level;
  pinMode(LOW_LED_PIN, OUTPUT);   // Green LED to indicate Low water level;
  pinMode(ALARM_PIN, OUTPUT);    // Alarm buzzer
  pinMode(LOW_LEVEL_PIN, INPUT_PULLUP); // Low level switch pin goes here
  pinMode(HIGH_LEVEL_PIN, INPUT_PULLUP);  // High level switch pin goes here

  // Set pump pins based on PrimaryPump (can be adjusted in loop if needed)
  primaryPumpPin = PrimaryPump == 0 ? PUMP1_PIN : PUMP2_PIN;
  secondaryPumpPin = PrimaryPump == 0 ? PUMP2_PIN : PUMP1_PIN;
}

void loop() {
  checkLevel();
  stateMachine();
  flashRedLed();
}

void checkLevel() {
  boolean lowLevel  = !digitalRead(LOW_LEVEL_PIN);
  boolean highLevel = !digitalRead(HIGH_LEVEL_PIN);
  delay(30);// Simple debouncing ...
  if (lowLevel) {
    state = LEVELLOW;
  }
  if (highLevel) {
    state = LEVELHIGH;
  }
  // The next states are not simulated in Wokwi but
  // could be possible with a real limit switch
  if (!lowLevel && !highLevel) {
    state = LEVELNORMAL;
  }
  if (lowLevel && highLevel) {
    Serial.println("Sensor defect -> Error!!");
  }
}

void stateMachine() {
  switch (state) {
    case LEVELLOW:
      lowLevel();
      break;
    case LEVELNORMAL:
      normalLevel();
      break;
    case LEVELHIGH:
      highLevel();
      break;
  }
}

void lowLevel() {
  if (lastState != LEVELLOW) {
    Serial.println("LOW");
    flashRed = false;
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, HIGH); // Green LED ON
    digitalWrite(HIGH_LED_PIN, LOW); // Red LED Off
    digitalWrite(primaryPumpPin, LOW); // Stop Pump (assuming any pump was running)
    digitalWrite(secondaryPumpPin, LOW);
    // Now we exchange the pumpPins for the next cycle
    byte tempPin = primaryPumpPin;
    primaryPumpPin = secondaryPumpPin;
    secondaryPumpPin = tempPin;
    secondPumpOn = false;
    lastState = state;
  }
}

void normalLevel() {
  // Normal Level - Turn off LEDs and Alarm
  if (lastState != LEVELNORMAL) {
    Serial.println("NORMAL");
    flashRed = false;
    digitalWrite(ALARM_PIN, LOW);  // Alarm Off
    digitalWrite(LOW_LED_PIN, LOW); // Green LED Off
    digitalWrite(HIGH_LED_PIN, LOW); // Red LED Off
    lastState = state;
  }
}

void highLevel() {
  // High Level Detected - Red LED ON and activate Pump sequence
  if (lastState != LEVELHIGH) {
    Serial.println("HIGH");
    flashRed = false;
    digitalWrite(ALARM_PIN, HIGH);  // Alarm On
    digitalWrite(LOW_LED_PIN, LOW); // Green LED Off
    digitalWrite(HIGH_LED_PIN, HIGH); // Red LED On
    digitalWrite(primaryPumpPin, HIGH);  // Start Pump 1
    levelHighStartTime = millis();  // Store the start time of LEVELHIGH here
    lastState = state;
  }
  // Check if waitBeforePumpTwo has expired after start of the first pump
  // If it has expired and pump 2 is off switch it on
  if (millis() - levelHighStartTime >= waitBeforePumpTwo && !secondPumpOn) {
    startSecondPump();
  }
  // The following part is not required but nice for this demo ...
  static unsigned long lastPrint = 0;
  if (millis() - lastPrint >= 1000 && !secondPumpOn) {
    lastPrint = millis();
    Serial.print(".");
  }
}

void startSecondPump() {
  // and flash Red Led
  Serial.println("Start Second Pump");
  flashRed = true;
  secondPumpOn = true;
  digitalWrite(ALARM_PIN, HIGH);
  digitalWrite(secondaryPumpPin, HIGH); // Start Pump 2
}

void flashRedLed() {
  static unsigned long lastChange = 0;
  static byte ledState = HIGH;
  if (!flashRed) {
    return;
  }
  if (millis() - lastChange > 300) {
    lastChange = millis();
    digitalWrite(HIGH_LED_PIN, ledState);
    ledState = !ledState;
  }
}

Everytime it comes from HIGH level to LOW level the pump pins are changed.

The delay for the second pump is set to 4 secs just for testing. See "waitBeforePumpTwo"

If the limit switches work like the slide switch, it should do it as you explained ...

Feel free to give it a try ...

1 Like