Trying to make it beautiful

Projects Programming
1

Hello, I adapted this code to my needs it is working perfect but I am looking to make it better, maybe shorter and easier to read. What you developers would do better.
It is a simple command relay to turn ON OFF two different devices in predetermined hour, Thanks in advance.

Blockquote
#include <DS3231.h>
'
int RelayIN1 = 4;
int RelayIN2 = 5;
'
DS3231 rtc(SDA, SCL);
Time t;
'
const int OnHour1 = 22;
const int OnMin1 = 10;
const int OnSec1 = 00;
const int OffHour1 = 22;
const int OffMin1 = 10;
const int OffSec1 = 30;
'
const int OnHour2 = 22;
const int OnMin2 = 11;
const int OnSec2 = 00;
const int OffHour2 = 22;
const int OffMin2 = 11;
const int OffSec2 = 35;
'
const int OnHour3 = 22;
const int OnMin3 = 12;
const int OnSec3 = 00;
const int OffHour3 = 22;
const int OffMin3 = 12;
const int OffSec3 = 32;
'
const int OnHour4 = 22;
const int OnMin4 = 11;
const int OnSec4 = 05;
const int OffHour4 = 22;
const int OffMin4 = 12;
const int OffSec4 = 35;
'
void setup() {
Serial.begin(115200);
rtc.begin();
'
pinMode(RelayIN1, OUTPUT);
digitalWrite(RelayIN1, LOW);
'
pinMode(RelayIN2, OUTPUT);
digitalWrite(RelayIN2, LOW);
}
void loop() {
t = rtc.getTime();
Serial.print(t.hour);
Serial.print(" hour(s), ");
Serial.print(t.min);
Serial.print(" minute(s)");
Serial.print(t.sec);
Serial.print(" seconds(s)");
Serial.println(" ");
delay (1000);
'
if(t.hour == OnHour1 && t.min == OnMin1 && t.sec == OnSec1){
digitalWrite(RelayIN1,HIGH);
Serial.println("Morning Spray ON");
}
else if(t.hour == OffHour1 && t.min == OffMin1 && t.sec == OffSec1){
digitalWrite(RelayIN1,LOW);
Serial.println("Morning Spray OFF");
}
'
if(t.hour == OnHour2 && t.min == OnMin2 && t.sec == OnSec2){
digitalWrite(RelayIN1,HIGH);
Serial.println("Afternoon Spray ON");
}
else if(t.hour == OffHour2 && t.min == OffMin2 && t.sec == OffSec2){
digitalWrite(RelayIN1,LOW);
Serial.println("Afternoon Spray OFF");
}
'
if(t.hour == OnHour3 && t.min == OnMin3 && t.sec == OnSec3){
digitalWrite(RelayIN1,HIGH);
Serial.println("Evening Spray ON");
}
else if(t.hour == OffHour3 && t.min == OffMin3 && t.sec == OffSec3){
digitalWrite(RelayIN1,LOW);
Serial.println("Evening Spray OFF");
}
'
if(t.hour == OnHour4 && t.min == OnMin4 && t.sec == OnSec4){
digitalWrite(RelayIN2,HIGH);
Serial.println("LED Day Lights ON");
}
else if(t.hour == OffHour4 && t.min == OffMin4 && t.sec == OffSec4){
digitalWrite(RelayIN2,LOW);
Serial.println("LED Night OFF");
}
}
Blockquote

2

I'd get rid of all those single quotes - they'll screw up your compilation.

The constants I'd put in an array of struct, and reduce the memory footprint at the same time by using more appropriate types.

1 Like
3

The single quotes were the only way to keep the Blockquote in all the code. Sorry

4

I wish I understood that.

Code tags make it easy to post code.

1 Like
5

Here is one way to clean things up. Using a 'struct' to put all of the data for a time interval into one spot. Then you can add any number of time intervals for any number of relays.

Note: Looking for an exact match on Hour, Minute, and Second is risky, especially if you have a 'delay(1000)' in your code. Much better to convert times to seconds and change the output if the relay should be on but isn't or should be off but is on. This also avoids displaying four messages every time through loop().

Note: In C++, a 'struct' is almost identical to a 'class'. It can have member functions. The main difference is that a 'struct' starts with an implicit "public:" label and a 'class' starts with an implicit "private:" label.

#include <DS3231.h>

int RelayIN1 = 4;
int RelayIN2 = 5;

DS3231 rtc(SDA, SCL);
Time t;

struct HMS
{
  byte Hour;
  byte Minute;
  byte Second;

  unsigned long AsSeconds()
  {
    return (((Hour * 60ul) + Minute) * 60) + Second;
  }
};

struct Interval
{
  int relayPin;
  HMS onTime;
  HMS offTime;
  const char *message;
  boolean isOn;

  boolean shouldBeOn(unsigned long seconds)
  {
    return seconds >= onTime.AsSeconds() && seconds < offTime.AsSeconds();
  }

  void update(byte hour, byte minute, byte second)
  {
    HMS time = {hour, minute, second};
    boolean nowOn = shouldBeOn(time.AsSeconds());
    if (nowOn == isOn)
      return; // no change

    digitalWrite(relayPin, nowOn);
    isOn = nowOn;

    Serial.print(message);
    if (nowOn)
      Serial.println(" ON");
    else
      Serial.println(" OFF");
  }
};

Interval IntervalList[] =
{
  {RelayIN1, {22, 10, 00}, {22, 10, 30}, "Morning Spray", false},
  {RelayIN1, {22, 11, 00}, {22, 11, 35}, "Morning Spray", false},
  {RelayIN1, {22, 12, 00}, {22, 12, 32}, "Morning Spray", false},
  {RelayIN2, {22, 11, 05}, {22, 12, 35}, "LED Lighting", false},
};
const size_t IntervalCount = sizeof IntervalList / sizeof IntervalList[0];

void setup()
{
  Serial.begin(115200);
  rtc.begin();

  for (size_t i = 0; i < IntervalCount ; i++)
  {
    IntervalList[i].isOn = false;
    digitalWrite(IntervalList[i].relayPin, LOW);
    pinMode(IntervalList[i].relayPin, OUTPUT);
  }
}

void loop()
{
  t = rtc.getTime();
  Serial.print(t.hour);
  Serial.print(" hour(s), ");
  Serial.print(t.min);
  Serial.print(" minute(s)");
  Serial.print(t.sec);
  Serial.print(" seconds(s)");
  Serial.println(" ");
  delay (1000);

  for (size_t i = 0; i < IntervalCount ; i++)
  {
    IntervalList[i].update(t.hour, t.min, t.sec);
  }
}
2 Likes
6

May I suggest that when you check if it is time to actuate on the relay

  1. Convert both times to seconds to make the comparison instruction cleaner and easier to read. You can even use a function such as
unsigned long seconds(int h, int m, int s) {
	return s + m * 60ul + 3600 * 3600ul;
}

// and then

timeOn1 = seconds(OnHour1, OnMin1, OnSec1);
timeOff1 = seconds(OffHour1, OffMin1, OffSec1)
  1. Use a construction such as
  currentTime = seconds(t.hour, t.min, t.sec);
  timeOn1  = seconds(OnHour1,  OnMin1,  OnSec1);
  timeOff1 = seconds(OffHour1, OffMin1, OffSec1);
  if (timeOff1 < timeOn1) {
  	// overnight watering
  	timeOff1 += 24ul*60*60;
  }
  if (currentTime >= timeOn1 && currentTime < timeOff1) {
    digitalWrite(RelayIN1, HIGH);
    Serial.println("Morning Spray ON"); // ...morning?
  } else {
    digitalWrite(RelayIN1, LOW);
    Serial.println("Morning Spray OFF");
  }

This way, if for ANY reason the exact second is missed, the Spray will still turn on or off when due. For example, if you turn on or reset your Arduino any time during the period of time when the relay should be on, this code will turn it on.

Note the adjustment on the time for say, start at 11pm and stop at 1am

1 Like
7

Hi John, thank you for your time.
This is really much better, as soon as I arrive at home I will try it, this is a project for a microgreens growth, the idea is to spray water for 10~15 seconds 3x a day for about two weeks, after day 4 or four loops it will start LED lights for 16 hours a day for about ten days. that is why I need the loops.
I am struggling in make the code understand that I need to start Lights after 4 days as I am using only hours and minutes.

8

Hi mancera, Thanks for your time as well, that is a good idea, I explained to John above what is the project, easier to plan if you have an idea fot the proposal, I am using seconds because the pump has to be on 3x a day for only few seconds, otherwise will flood the seeds, this seconds will depend on the power of the pump used tests must be made before the system is in place.

9

The RTC library should be able to give you an "epoch time". That's the number of seconds since a particular date and time. If you record the start date and time you can calculate the number of weeks, days, hours, minutes, and seconds since the start. Then you could use 'if':

    unsigned int elapsedDays = currentDays - startDays;
    if (elapsedDays > 4)
    {
      // Do the LED part
    }
1 Like
10

you are the best thanks exact what I need.

11

where should I add this code of yours to the main one? I am still studying your code and don't understand where it will get elapsed days and current days to make this calculation. :frowning:

12

If your epoch time is in seconds, divide the current epoch time by the number of seconds in a day to get currentDay. Subtract startDay from currentDay to get elapsedDays.

const unsigned long MINUTE = 60ul;
const unsigned long HOUR = MINUTE * 60;
const unsigned long DAY = HOUR * 24;

unsigned long CurrentDay = epochTime / DAY;
13

Are you using Millis? I can't find it :frowning:

14

What DS3231 library are you using?

15

from here:
http://www.rinkydinkelectronics.com/library.php?id=73

16

I am struggling in make the code understand that I need to start Lights after 4 days as I am using only hours and minutes.

You'd like to start Lights exactly 4 days after watering started or on the 4th day at 6am for example?

The t struct which you update from the RTC in every iteration has a t.date member also..

17

yes

18

long epochTime = rtc.getUnixTime(rtc.getTime());

See: Unix time - Wikipedia

19

Okay, that did not answer my question. But never mind, I was thinking in terms of your original code and I realized that suggested code has a different approach.

20

Hello, I am looking to count this loop 4 times (4 days), and start another LED. The code is from this forum made by johnwasser.

#include <DS3231.h>

int RelayIN1 = 4;
int RelayIN2 = 5;
int LED_Level = 8;

//set the float sensor for pin 3
#define Float_Switch 3

DS3231 rtc(SDA, SCL);
Time t;

struct HMS
{
  byte Hour;
  byte Minute;
  byte Second;

  unsigned long AsSeconds()
  {
    return (((Hour * 60ul) + Minute) * 60) + Second;
  }
};

struct Interval
{
  int relayPin;
  HMS onTime;
  HMS offTime;
  const char *message;
  boolean isOn;

  boolean shouldBeOn(unsigned long seconds)
  {
    return seconds >= onTime.AsSeconds() && seconds < offTime.AsSeconds();
  }

  void update(byte hour, byte minute, byte second)
  {
    HMS time = {hour, minute, second};
    boolean nowOn = shouldBeOn(time.AsSeconds());
    if (nowOn == isOn)
      return; // no change

    digitalWrite(relayPin, nowOn);
    isOn = nowOn;

    Serial.print(message);
    if (nowOn)
      Serial.println(" ON");
    else
      Serial.println(" OFF");
  }
};

Interval IntervalList[] =
{
  {RelayIN1, {10, 38, 00}, {10, 38, 15}, "Morning Spray", false},
  {RelayIN1, {10, 38, 20}, {10, 38, 30}, "Afternoon Spray", false},
  {RelayIN1, {10, 38, 40}, {10, 38, 50}, "Evening Spray", false},
  {RelayIN2, {10, 38, 05}, {10, 38, 35}, "LED Lighting", false},
};
const size_t IntervalCount = sizeof IntervalList / sizeof IntervalList[0];

void setup()
{
  Serial.begin(115200);
  rtc.begin();

  for (size_t i = 0; i < IntervalCount ; i++)
  {
    IntervalList[i].isOn = false;
    digitalWrite(IntervalList[i].relayPin, LOW);
    pinMode(IntervalList[i].relayPin, OUTPUT);
  }

// Water Float Level Sswitch 
     pinMode(LED_Level, OUTPUT);
     pinMode(Float_Switch, INPUT_PULLUP);
}

void loop()
{
  t = rtc.getTime();
  Serial.print(t.hour);
  Serial.print(" hour(s), ");
  Serial.print(t.min);
  Serial.print(" minute(s)");
  Serial.print(t.sec);
  Serial.print(" seconds(s)");
  Serial.println(" ");
  delay (1000);

  for (size_t i = 0; i < IntervalCount ; i++)
  {
    IntervalList[i].update(t.hour, t.min, t.sec);
  }

if(digitalRead(Float_Switch) == HIGH)
  {
    digitalWrite(LED_Level, HIGH); //Turn LED on
  }

 else
  {
    digitalWrite(LED_Level, LOW); //Turn LED off
  }

  
}