I've managed to control a water pump with a dedicated power supply with a relay using my Arduino Uno.
It works but I must have made tons of mistake and I would love to get your blunt feedback regarding what I can improve.
I've attached my Fritzing schematic as well as my code
Thanks for your feedback
//for ease of set-up and no hardcoding I define here the output used to control the relay
int relay1 = 10;
int lag = 1000;
void setup() {
//here I define my pin as an output
pinMode(relay1, OUTPUT);
//here I set the relay1 default state
digitalWrite(relay1, LOW);
//used for debugging
Serial.begin(9600);
}
void loop() {
digitalWrite(relay1, HIGH);
Serial.println("Relay is ON");
delay(lag);
digitalWrite(relay1, LOW);
Serial.println("OFF");
delay(lag);
}
instead of posting a cartoon from fritzrig, switch to schematic view and post a schematic.
most on here are not even aware that fritzrig does schematics because of these cartoons.
second, you do not have a reason to turn the pump on and off.
do you have a fountain for pleasure and you just want to turn in on and off every so often ?
usually one measures the level and turns pump on, tests the level and either leaves it run or turns off when some level is reached.
dave-in-nj:
instead of posting a cartoon from fritzrig, switch to schematic view and post a schematic.
most on here are not even aware that fritzrig does schematics because of these cartoons.
Thanks Dave, I need to step-up my game with Fritzing. I posted the cartoon view as it is less intimidating, and easier to understand for me at my newbie level. I appreciate schematics are better.
dave-in-nj:
second, you do not have a reason to turn the pump on and off.
do you have a fountain for pleasure and you just want to turn in on and off every so often ?
usually one measures the level and turns pump on, tests the level and either leaves it run or turns off when some level is reached.
I wanted to create something as simple as possible and I will quickly iterate with testing level from a reservoir and setting up alarms as I want to do yet another arduino watering system (it's the project I like the most to start learning and potentially make something helpful for me).
Thank you for taking the time to share your thoughts
Your lines are not indented, use Auto Format. You should use the 'const' keyword on values that don't change. You should use more descriptive names. You should differentiate global names from local variables, possibly with an initial capital letter. You have a strange relay module since most are Active Low (LOW turns the relay ON and HIGH turns the relay OFF). You should set the desired pin state before you set the mode to Output. I like to see Serial.begin(115200); at the TOP of setup(). No need to use a slow baud rate like 9600.
// Constants
const byte WaterPumpRelayPin = 10;
const int CycleDelay = 1000;
void setup()
{
//used for debugging
Serial.begin(115200);
//here I set the relay1 default state
digitalWrite(WaterPumpRelayPin, LOW);
//here I define my pin as an output
pinMode(WaterPumpRelayPin, OUTPUT);
}
void loop()
{
digitalWrite(WaterPumpRelayPin, HIGH);
Serial.println("Relay is ON");
delay(CycleDelay);
digitalWrite(WaterPumpRelayPin, LOW);
Serial.println("OFF");
delay(CycleDelay);
}
As part of your newbie-ness, keep your concept exactly as it is, but see if you can convert to millis() timing - then you will be on the way to more intuitive programming that can several things at the same time.
That change could also implement a single function to manage the pump
e,g
Your Fritzing diagram is perfectly adequate to show what you are doing, clear and well annotated. You are using Fritzing for the right reasons, and the disparaging remark in reply #1 is just pompous buillshit, but you might just learn line things up better, thereby perhaps avoiding some of the wrath of the purists. Annotation is the hard part in Fritzing, but you are already OK with that.
Since you are sending trivial data at one second intervals, there is no need to step up your serial rate but it isn't a bad idea and, if you do, remember to make sure your serial monitor is adjusted accordingly in the bottom-right corner.
It is a good idea to keep that excessive white space out of your code to make it easier to read. There is never a reason to have more than two consecutive blank lines in code - ever - and one usually suffices.
Thanks Slumpert, I'm essentially concerned I might do very stupid things. If nothing jumps out I will then proceed with my next iteration on this learning project
Thank you so much for your feedback, I've learnt a ton of new things, implemented your suggestions and pushed my project forward.
You'll find below the updated version of the code with proper indentation, naming and functions!
Now my project includes a potentiometer to be used as a threshold.
Next steps will be adding an IC2 led screen as well as a load sensor (to be used in order to see if the tank is close from being empty or not).
I still have so much to learn yet it feels like I've made a lot of progress thanks to you all.
#include <Time.h>
#include <TimeLib.h>
//key parameters
const byte waterPumpRelay1Pin = 10;
const int cycleDelay = 3000;
// select the input pin for the potentiometer
int sensorPin = A0;
int sensorValue = 0; // variable to store the value coming from the sensor
void setup() {
//used for debugging
Serial.begin(115200);
//here I set the relay1 default state
//HIGH turns the relay OFF
digitalWrite(waterPumpRelay1Pin, HIGH);
//here I define my pin as an output
pinMode(waterPumpRelay1Pin, OUTPUT);
}
void loop() {
// read the value from the sensor:
sensorValue = analogRead(sensorPin);
Serial.print ("Sensor value is: ");
Serial.println (sensorValue);
if (sensorValue > 800){
Serial.println ("Let's turn on this relay");
waterPump(waterPumpRelay1Pin,LOW);
}
waterPump(waterPumpRelay1Pin,HIGH);
}
//control water pump and log status
void waterPump(const byte relayPin, bool state){
digitalWrite(relayPin, state);
Serial.print("On: ");
Serial.println(showTime());
Serial.print("Water pump at pin ");
Serial.print(relayPin);
Serial.print(" is ");
Serial.println(state);
Serial.println(" ");
delay(cycleDelay);
}
int showTime(){
Serial.print(hour());
printDigits(minute());
printDigits(second());
Serial.print(" ");
Serial.print(day());
Serial.print(" ");
Serial.print(month());
Serial.print(" ");
Serial.print(year());
Serial.print(" ");
}
// utility function for digital clock display: prints preceding colon and leading 0
void printDigits(int digits) {
Serial.print(":");
if (digits < 10)
Serial.print('0');
Serial.print(digits);
}
For hd44780 controlled LCDs with the PCF8574 I2C backpacks the hd44780 library is superior to any of the LiquidCrystal libraries (my opinion) and will make getting the display to work easier and quicker.
