Code improvement

Newbie asking not to be roasted but asking for advice and tips on improving my code and newbie errors i have made as well as other ways to approach this project.

About the code, it is for Home automation. Here is a short list of the operations
*It runs 2 sets of LED light that is motion activated and timed off.
*Battery voltage measurement and charging
*Water tank(JOJO) level control (2 Solenoids)
*Garden irrigation & Fire Sprinkler (4 Solenoids)
*LCD display

Time is used in almost all of my functions. I have not attempted to use interrupts.

Any advice and tips would be appreciated!

Home_Automation.ino (16.9 KB)

Let's start at the very basic level

Why are the majority of your variables ints when their value will never exceed 255 so they could be bytes ? Don't use memory when you don't need to. One day it will matter

Thanks, will look at that to improve. Guess i just kept using it since started from "Blink without delay"....

(deleted)

(deleted)

spycatcher2k:
Sent in a PM :

Regarding Change these to : pinMode(Pin_Description,INPUT_PULLUP);
I might be dumb....but i just dont understand what changes it brings when changing it to PULLUP.

Can you please assist me and elaborate this as i clearly fail to understand that part.

Please don't PM these questions, post them in the forum, this will help others to learn. :wink:

The internal pullup is turned on, making the input HIGH by default. You then pull htis to ground through your switch/button and the input LOW. saves hooking up an external pull-up resistor, and stops the pin floating(giving indeterminate readings) when the swutch/button is not activated.

Thank you. Wish i knew this earlier! Regarding the 2 RTC library's, both is used as that i used functions from <DS1307RTC.h> that was not available in<DS3231.h> like "setSyncProvider(RTC.get); " When i got it working i just stuck to it. Will try to remove <DS1307RTC.h>, thanks for the advice!

You appear to only require single decimal digit accuracy in your battery measurements. Try to eliminate floating point math where it's not necessary, and replace it with fixed point scaled math. For example, BatteryLoop() has this:

float voltage;
float Ra = 55930.0;  //2 resistors
float Rb = 27190.0;
[...]
void BatteryLoop(){
 BatteryValue = analogRead(A1);
  voltage = BatteryValue * (5.0/1024)*((Ra + Rb)/Rb);
[...]
  if(voltage<11.9){
[...]
          Serial.print("Voltage =");
          Serial.println(voltage);

(Ra + Rb)/Rb = (55930+27190)/27190 = (5593+2719)/2719

rearrange so multiplication is first:

const int FIXED_SCALE = 10;
const long Ra = 55930/10L;
const long Rb = 27190/10L;
long voltage;
...
voltage = BatteryValue * 5L * (Ra + Rb) * FIXED_SCALE  / 1024 / Rb);
...
  if(voltage < (long)(11.9*FIXED_SCALE)) {
...
          Serial.print("Voltage =");
          Serial.println((float)voltage / FIXED_SCALE);

The bare approach may seem awkward, for example the casts and scaling, but you can encapsulate those in wrapper functions that you would make, like printScaled()...

For the comparisons you can also skip the formalities and compare to a scaled integer directly, and comment appropriately:

if(voltage < 119) { // 11.9V