New to the world, any guidance?

Hi All, new to the forum having got my arduino a few days ago....

So my project is a timed relay controller.

So far, I have got a DS1307, working, got the LCD, working, got the relays hitched up and able to control.

Ive done a little sketch that is just sequentially switching relays on/off on a millisecond count, and showing the day, hh:mm on an LCD.

I've tried my best to "figure it out" as I have done so far, and cobble bits together, but now I stuck..

Here is my code:
(code tags fixed by moderator)

// include the library code for DS1307
// include the library code for LCD
// Define the DS1307

#include <LiquidCrystal.h>
#include <Wire.h>
#define DS1307_I2C_ADDRESS 0x68

// initialize the library with the numbers of the interface pins for LCD

LiquidCrystal lcd(12, 11, 10, 5, 4, 3, 2);

const int relay0pin =  8; // Set the pin for relay0     
const int relay1pin =  9; // set the pin for relay1 

// Variables will change:

int relay0state = LOW; 
int relay1state = LOW;  

// ledState used to set the relays

long previousMillis0 = 1;        // will store last time relay0 was updated
long previousMillis1 = 1;        // will store last time relay1 was updated          


// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.

long interval0 = 5000;           // interval at which to switch (milliseconds)
long interval1 = 10000;           // interval at which to switch (milliseconds)

// Convert normal decimal numbers to binary coded decimal

 byte decToBcd(byte val)
 {
 return ( (val/10*16) + (val%10) );
 }

 // Convert binary coded decimal to normal decimal numbers

 byte bcdToDec(byte val)
 {
 return ( (val/16*10) + (val%16) );
 }

 // 1) Sets the date and time on the ds1307
 // 2) Starts the clock
 // 3) Sets hour mode to 24 hour clock
 // Assumes you're passing in valid numbers

 void setDateDs1307(byte second, // 0-59
 byte minute, // 0-59
 byte hour, // 1-23
 byte dayOfWeek, // 1-7
 byte dayOfMonth, // 1-28/29/30/31
 byte month, // 1-12
 byte year) // 0-99
 {
 Wire.beginTransmission(DS1307_I2C_ADDRESS);
 Wire.write(0);
 Wire.write(decToBcd(second)); // 0 to bit 7 starts the clock
 Wire.write(decToBcd(minute));
 Wire.write(decToBcd(hour));
 Wire.write(decToBcd(dayOfWeek));
 Wire.write(decToBcd(dayOfMonth));
 Wire.write(decToBcd(month));
 Wire.write(decToBcd(year));
 Wire.write(0x10); // writes 0x10 (hex) 00010000 (binary) to control register - turns on square wave
 Wire.endTransmission();
 }

 // Gets the date and time from the ds1307

 void getDateDs1307(byte *second,
 byte *minute,
 byte *hour,
 byte *dayOfWeek,
 byte *dayOfMonth,
 byte *month,
 byte *year)
 {

 // Reset the register pointer

 Wire.beginTransmission(DS1307_I2C_ADDRESS);
 Wire.write(0);
 Wire.endTransmission();
 Wire.requestFrom(DS1307_I2C_ADDRESS, 7);
 
 // A few of these need masks because certain bits are control bits
 
 *second = bcdToDec(Wire.read() & 0x7f);
 *minute = bcdToDec(Wire.read());
 *hour = bcdToDec(Wire.read() & 0x3f); // Need to change this if 12 hour am/pm
 *dayOfWeek = bcdToDec(Wire.read());
 *dayOfMonth = bcdToDec(Wire.read());
 *month = bcdToDec(Wire.read());
 *year = bcdToDec(Wire.read());
}

void setup() {
// set the digital pin as output:

pinMode(relay0pin, OUTPUT);   
pinMode(relay1pin, OUTPUT);     
 
byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
 Wire.begin();
 Serial.begin(9600);

// Change these values to what you want to set your clock to.
// You probably only want to set your clock once and then remove
// the setDateDs1307 call.

 second = 45;
 minute = 31;
 hour = 20;
 dayOfWeek = 6;
 dayOfMonth = 19;
 month = 4;
 year = 13;
 
//setDateDs1307(second, minute, hour, dayOfWeek, dayOfMonth, month, year);

lcd.begin(16, 2); // tells Arduino the LCD dimensions
  
}

void loop()
{

// here is the code that needs to be running all the time.
    
    lcd.begin(16, 2);
    lcd.setCursor(6, 0);
    lcd.print("L1");
    lcd.setCursor(10, 0);
    lcd.print("L2");
    lcd.setCursor(14, 0);
    lcd.print("L3");
     
 byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
 getDateDs1307(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);
 
 //lcd.clear(); // clear LCD screen
 
 lcd.setCursor(0,0);

 lcd.print(hour, DEC);
 lcd.print(":");
 if (minute<10)
 {
 lcd.print("0");
 }
 lcd.print(minute, DEC);

 if (second<10)
 {
 lcd.print("");
 }
 
 lcd.setCursor(0,1);

 switch(dayOfWeek){
 case 1:
 lcd.print("Sun");
 break;
 case 2:
 lcd.print("Mon");
 break;
 case 3:
 lcd.print("Tue");
 break;
 case 4:
 lcd.print("Wed");
 break;
 case 5:
 lcd.print("Thu");
 break;
 case 6:
 lcd.print("Fri");
 break;
 case 7:
 lcd.print("Sat");
 break;
 }

 delay(1000);

// check to see if it's time to switch the realy; that is, if the 
// difference between the current time and last time you switched 
// the relay is bigger than the interval at which you want to 
// switch the relay.
  
unsigned long currentMillis0 = millis();
unsigned long currentMillis1 = millis();
 
  if(currentMillis0 - previousMillis0 > interval0) {
   
    // save the last time relay0 switched
    
    previousMillis0 = currentMillis0;   

    lcd.setCursor(6, 1);
   
    // if the relay0 is off turn it on and vice-versa:
   
    if (relay0state == LOW)
      relay0state = HIGH;
      
    else
      relay0state = LOW;
  
        digitalWrite(relay0pin, relay0state);
       
  }
    
   if(currentMillis1 - previousMillis1 > interval1) {
    
     // save the last time relay1 switched 
    
    previousMillis1 = currentMillis1;   

 // if the relay1 is off turn it on and vice-versa:
   
    if (relay1state == LOW)
      relay1state = HIGH; 
      
      else
      relay1state = LOW;
   
    digitalWrite(relay1pin, relay1state);
   
  }
}

I have two questions, at the bottom of the code you will see there are two relays switching on and off at set timings, not usefull at the moment, but more of a test while I get the hang of it.

I have an LCD set to show L1, L2, L3 on line one, ignore L3 - no hooked this relay up yet), but I am trying to get it to display "on" on line 2 at the set positions corresponding with the L1, L2 when each relay is on its "high" cycle, I cant work it out though.... can anyone help point me?

Essentially I am trying to get along the lines of (example when relay0 is low and relay 1 is high)

00:00 L1 L2 L3
Fri ON

(example when relay0 is HIGH and relay 1 is HIGH)

00:00 L1 L2 L3
Fri ON ON

As said, once I have that I can figure out to add additional ones once I hook up....

Welcome to the forum.
I fixed your tags up.
Time related elements should be unsigned long, that is what millis and micros return.

Thanks, I spotted my mistake on that afterwards...

Just to make things a bit clearer, at the end of the day, I would like to have this little gadget with 3 time switches on it controlling the 3 (to be) relays, i.e.

06.00 Relay0 comes on
09.00 relay1 comes on
12.00 relay2 comes on
18.00 relay 2 goes off
20.00 relay 1 goes off
22.00 relay0 goes off

and that the display of L1, L2, L3 has a corresponding "on" for each relay that is on.

I want to try to get this far myself, hence the question is, where I have t two relays at:

  if(currentMillis0 - previousMillis0 > interval0) {
   
    // save the last time relay0 switched
    
    previousMillis0 = currentMillis0;   

    lcd.setCursor(6, 1);
   
    // if the relay0 is off turn it on and vice-versa:
   
    if (relay0state == LOW)
      relay0state = HIGH;
      
    else
      relay0state = LOW;
  
        digitalWrite(relay0pin, relay0state);
       
  }
    
   if(currentMillis1 - previousMillis1 > interval1) {
    
     // save the last time relay1 switched 
    
    previousMillis1 = currentMillis1;   

 // if the relay1 is off turn it on and vice-versa:
   
    if (relay1state == LOW)
      relay1state = HIGH; 
      
      else
      relay1state = LOW;
   
    digitalWrite(relay1pin, relay1state);
   
  }

I am trying to get a lcd.print("on") at lcd.setcursor(6,2) when relay0 is high and the same at (10,2) where relay1 is high...

I bet this makes no sense, but it gets me on my way, without "doing the whole lot for me" if that makes sense....

I am trying to get a lcd.print("on") at lcd.setcursor(6,2) when relay0 is high and the same at (10,2) where relay1 is high...

Where to print on the screen is not really part of the problem. What to print and when to print it are. You know the state of the relay. At some point, you set it high or low. So, that takes care of the what. All that's left is the when. That's really pretty simple, too. Do it every pass through loop().

Or, am I missing something?

Sorry, I'm very very new to this, I kinda get what you mean, but its the how do I code the if relay0 = high then put this word there, and the where should I put this..

I experimented putting bits in which seemed to validate ok, but did nothing when on there...

Im assuming would be along the lines of:

if (relay0state == HIGH)
lcd.setCursor(6,1);
lcd.print("on");

but as to where to put it in the whole scheme of things, I have no idea.. this is one of the things I am trying to get some help to do, so then I can start to understand the sections... I'm a kind of learn by seeing it once and playing about person than a theory person... I know I sound a bit dense :confused:

Im assuming would be along the lines of:

if (relay0state == HIGH)
lcd.setCursor(6,1);
lcd.print("on");

So, if the relay state is HIGH, move the cursor. If not, leave the cursor where it is. Then, print "on" on the LCD, someplace.

No, I really don't think that's it. As a newbie, you should ALWAYS use curly braces after an if statement. When you get enough experience, you can sometimes get away without them. On the other hand, my fingers have become so accustomed to typing

if()
{
}

and then figuring out what goes in the () and {} that I almost never have an if statement without curly braces.

As to where that code goes, if it were correct, that depends on when you want to write to the LCD. There are two possibilities. One is to write to the LCD on every pass through loop(). The other is to write to the LCD only when something changes.

If you prefer to ensure that the data on the LCD is always current, add code to print to the LCD at the end of loop.

If you will be rigorous about writing to the LCD whenever there is a change, then put the code where the change is made.

I think that the 'print the state when it changes' makes more sense, at least to me.

I heartily agree about the use of curly brackets (each on their own line) every time, even if there is only one statement dependant on the if.

Your code would be neater if you used an array to hold the names of the days of the week indexed by the number of the day. It would remove the need to test each value of dayOfWeek to convert it to the name.

ok, so I get the jist of it.. ish...

Yes current info is what Im after not the time and date bit is basically all just to test everything was working, so at present can be ignored...

So could anyone tell me what my if statement should be to determine the state of each relay and print accordingly, and where in my code to put it I would be much appreciative.

I tried putting

if (relay1state = HIGH){
lcd.setCursor(6,1);
lcd.print("on");}

in after the void loop() bit, but all it does it knock off relay 1 from actually switching....

You have one of the most classic C mistakes there:

if (relay1state = HIGH){

= is assignment. == is comparison.

You need

if (relay1state == HIGH){

I, and many others, find code like this:

if(someCondition)
{
   doSomething();
}

easier to read, and see the range, of then

if(someCondition){
  doSomething();};

I suggest that you try the former, for a while, too.

Also, you are going to position the cursor to the same place whether you are going to print "on" or "off", aren't you? If so, it doesn't belong in the if statement body. Only the conditional stuff does.

Now I understand!!

Will give it a whirl in the morning and see how I go!

Thanks for the help, got it all working. Few little tweaks to how it displays and actually put in the 3rd relay and all is working as I wanted (just now to work out rather than random switching of the relays how to do it on a proper times schedule from the RTC.

here's what I ended up with:

//LCD Realy test 1.2.0.3

#include <LiquidCrystal.h>
#include <Wire.h>
#define DS1307_I2C_ADDRESS 0x68

LiquidCrystal lcd(12, 11, 10, 5, 4, 3, 2);

const int relay0pin =  8; 
const int relay1pin =  9; 
const int relay2pin =  7; 

int relay0state = LOW; 
int relay1state = LOW;  
int relay2state = LOW;  

long previousMillis0 = 1;        // will store last time relay0 was updated
long previousMillis1 = 1;        // will store last time relay1 was updated     
long previousMillis2 = 1;        // will store last time relay1 was updated    

long interval0 = 1000;           // interval at which to switch (milliseconds)
long interval1 = 3000;           // interval at which to switch (milliseconds)
long interval2 = 2000;           // interval at which to switch (milliseconds)

    byte decToBcd(byte val)
 {
    return ( (val/10*16) + (val%10) );
 }

 

    byte bcdToDec(byte val)
 {
    return ( (val/16*10) + (val%16) );
 }

 void setDateDs1307(byte second, 
 byte minute,
 byte hour, 
 byte dayOfWeek,
 byte dayOfMonth, 
 byte month, 
 byte year)
 
 {

    Wire.beginTransmission(DS1307_I2C_ADDRESS);
    Wire.write(0);
    Wire.write(decToBcd(second)); 
    Wire.write(decToBcd(minute));
    Wire.write(decToBcd(hour));
    Wire.write(decToBcd(dayOfWeek));
    Wire.write(decToBcd(dayOfMonth));
    Wire.write(decToBcd(month));
    Wire.write(decToBcd(year));
    Wire.write(0x10); 
    Wire.endTransmission();
  
 }



 void getDateDs1307(byte *second,
 byte *minute,
 byte *hour,
 byte *dayOfWeek,
 byte *dayOfMonth,
 byte *month,
 byte *year)

 {

   Wire.beginTransmission(DS1307_I2C_ADDRESS);
   Wire.write(0);
   Wire.endTransmission();
   Wire.requestFrom(DS1307_I2C_ADDRESS, 7);
 
   *second = bcdToDec(Wire.read() & 0x7f);
   *minute = bcdToDec(Wire.read());
   *hour = bcdToDec(Wire.read() & 0x3f); // Need to change this if 12 hour am/pm
   *dayOfWeek = bcdToDec(Wire.read());
   *dayOfMonth = bcdToDec(Wire.read());
   *month = bcdToDec(Wire.read());
   *year = bcdToDec(Wire.read());
  
  }

void setup() 

  {

     pinMode(relay0pin, OUTPUT);   
     pinMode(relay1pin, OUTPUT);     
     pinMode(relay2pin, OUTPUT);
 
     byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
     Wire.begin();
     Serial.begin(9600);

     second = 45;
     minute = 31;
     hour = 20;
     dayOfWeek = 6;
     dayOfMonth = 19;
     month = 4;
     year = 13;
 
//setDateDs1307(second, minute, hour, dayOfWeek, dayOfMonth, month, year);

    lcd.begin(16, 2); // tells Arduino the LCD dimensions

  }

    void loop()
  
  {
 
    lcd.begin(16, 2);
    lcd.setCursor(11, 0);
    lcd.print("Light");
    lcd.setCursor (11,1);
    lcd.print("*");
    
    if (relay0state == HIGH)
  
  {
  
    lcd.setCursor(12,2);
    lcd.print("*");

  }
  
    if (relay1state == HIGH)
  
  {
  
    lcd.setCursor(13,2);
    lcd.print("*");

  } 
  
    if (relay2state == HIGH)
  
  {
  
    lcd.setCursor(14,2);
    lcd.print("*");

  } 
     
 byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
 getDateDs1307(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);
 
 lcd.setCursor(0,0);
 if (hour<10)
 
   {
   
     lcd.print("0");
   
   }
 
 lcd.print(hour, DEC);
 lcd.print(":");
 if (minute<10)
 
   {
 
     lcd.print("0");
 
   }
 
 lcd.print(minute, DEC);

 if (second<10)
 
   {
   
     lcd.print("");
   
   }
 
 lcd.setCursor(0,1);

 switch(dayOfWeek)
   
   {
 
     case 1:
     lcd.print("Sun");
     break;
     case 2:
     lcd.print("Mon");
     break;
     case 3:
     lcd.print("Tue");
     break;
     case 4:
     lcd.print("Wed");
     break;
     case 5:
     lcd.print("Thu");
     break;
     case 6:
     lcd.print("Fri");
     break;
     case 7:
     lcd.print("Sat");
     break;
 
   }

delay(1000);
  
unsigned long currentMillis0 = millis();
unsigned long currentMillis1 = millis();
unsigned long currentMillis2 = millis();
 
if(currentMillis0 - previousMillis0 > interval0) 
  
  {
    
    previousMillis0 = currentMillis0;   
 
    if (relay0state == LOW)
    
    relay0state = HIGH;
      
    else
    
    relay0state = LOW;
  
    digitalWrite(relay0pin, relay0state);
       
  }
    
if(currentMillis1 - previousMillis1 > interval1) 

  {
    
     previousMillis1 = currentMillis1;   
   
     if (relay1state == LOW)
     
     relay1state = HIGH; 
      
     else
     
     relay1state = LOW;
   
     digitalWrite(relay1pin, relay1state);
   
  }
  
if(currentMillis2 - previousMillis2 > interval2) 
  
  {
   
      previousMillis2 = currentMillis2;   
  
      if (relay2state == LOW)

      relay2state = HIGH;
      
      else

      relay2state = LOW;
  
      digitalWrite(relay2pin, relay2state);
       
  }
  }

Before you move on, this would be a good time to learn about arrays. Any time you start numbering variable names, it indicates a collection. That is what arrays are for.

const int relay0pin =  8; 
const int relay1pin =  9; 
const int relay2pin =  7; 

int relay0state = LOW; 
int relay1state = LOW;  
int relay2state = LOW;  

long previousMillis0 = 1;        // will store last time relay0 was updated
long previousMillis1 = 1;        // will store last time relay1 was updated     
long previousMillis2 = 1;        // will store last time relay1 was updated    

long interval0 = 1000;           // interval at which to switch (milliseconds)
long interval1 = 3000;           // interval at which to switch (milliseconds)
long interval2 = 2000;           // interval at which to switch (milliseconds)

should be

const int relayPin[] = { 8, 9, 7 }; 

int relayState[] = { LOW, LOW, LOW };  

unsigned long previousMillis[] = { 0, 0, 0 };        // will store last time relay was updated    

unsigned long interval[] = { 1000, 3000, 2000 };           // interval at which to switch (milliseconds)

I also fixed some of your types. In my world, time does not flow backwards, so signed values are inappropriate.

Using Tools + Auto Format would be a good thing, along with appropriate use of blank lines (you have far too many).