What am I missing here with time calls?

**HELP!! ** (Sorry I’m starting to go a little mad here)

I’m slowly getting the hang of this stuff BUT for the life of me I can’t work out what i’ve missed in this code.

The code compiles OK and the RTC and display all work fine but nothing happens to the leds.

If i’ve made a complete mess of it, I’m trying to switch on and off leds with PWM at certain times

I hope someone can point me in the right direction before i rip anymore hair out and honestly I can’t afford to loose anymore :-/

//Initialisation Calls********
#include <LiquidCrystal.h>
#include <WProgram.h>
#include <Wire.h>
#include <DS1307.h>
#define led_1 3 // Output to led 1 on pwm pin 3
#define led_2 5 // Output to led 2 on digital pin 5
#define led_3 6 // Output to led 3 on pwm pin 6

LiquidCrystal lcd(13, 12, 11, 8, 7, 4, 2);

int value1 = 0;
int value2 = 0;
int value3 = 255;
int value4 = 255;

long time1 = 0;
long time2 = 0;
long time3 = 0;
long time4 = 0;

void setup()
{
Wire.begin(); // Starts 2 wire commuication with RTC module
Serial.begin(9600); // Starts Serial communication with PC
pinMode (led_1, OUTPUT); // Sets digital pin 3 as an output pin
pinMode (led_2, OUTPUT); // Sets digital pin 5 as an output pin
pinMode (led_3, OUTPUT); // Sets digital pin 6 as an output pin

time1 = millis();
time2 = millis();
time3 = millis();
time4 = millis();

//SET TIME & DATE***************

//RTC.stop();
//RTC.set(DS1307_SEC,1); //set the seconds
//RTC.set(DS1307_MIN,21); //set the minutes
//RTC.set(DS1307_HR,16); //set the hours
//RTC.set(DS1307_DOW,2); //set the day of the week
//RTC.set(DS1307_DATE,27); //set the date
//RTC.set(DS1307_MTH,7); //set the month
//RTC.set(DS1307_YR,9); //set the year
//RTC.start();

//*****************************************************************************************
// set LCD’s number of columns and rows:
lcd.begin(4, 20);

//Print a message to the LCD.
lcd.setCursor(4, 0);
lcd.print(“Led Test”);

}

void loop()

{
//LCD Print*************
// set the cursor to column 0, line 1:
lcd.setCursor(0, 1);
lcd.print(“Time: “); // time message
lcd.print(RTC.get(DS1307_HR,true)); //read the hour and also update all the values by pushing in true
lcd.print(”:”);
lcd.print(RTC.get(DS1307_MIN,false));//read minutes without update (false)
lcd.setCursor(0, 2);
lcd.print(“Date: “); // date message
lcd.print(RTC.get(DS1307_DATE,false));//read date
lcd.print(”/”);
lcd.print(RTC.get(DS1307_MTH,false));//read month
lcd.print("/");
lcd.print(RTC.get(DS1307_YR,false)); //read year

//Time Calls*************

if(millis()>time1)
{
if ((DS1307_HR > 7 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))analogWrite(led_1, value1);
if ((DS1307_HR > 7 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))time1 = millis()+360; // Time delay between each pwm fade in pulse
if ((DS1307_HR > 7 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))value1+=25; // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
if ((DS1307_HR > 7 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))if (value1>255)digitalWrite (led_1, HIGH); // Turn pin fully on once faded in
}

if(millis()>time2)
{
if ((DS1307_HR > 8 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))analogWrite(led_2, value2);
if ((DS1307_HR > 8 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))time2 = millis()+360; // Time delay between each pwm fade in pulse
if ((DS1307_HR > 8 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))value2+=25; // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
if ((DS1307_HR > 8 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))if (value2>255)digitalWrite (led_2, HIGH); // Turn pin fully on once faded in
}

if(millis()>time3)
{
if ((DS1307_HR > 9 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))analogWrite(led_1, value3);
if ((DS1307_HR > 9 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))time3 = millis()+360; // Time delay between each pwm fade in pulse
if ((DS1307_HR > 9 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))value3-=25; // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
if ((DS1307_HR > 9 )&&( DS1307_MIN >1 )&&( DS1307_MIN <=59 ))if (value3<=0)digitalWrite (led_1, LOW); // Turn pin fully on once faded in
}

if(millis()>time4)
{
if ((DS1307_HR > 9 )&&( DS1307_MIN >2 )&&( DS1307_MIN <=59 ))analogWrite(led_2, value4);
if ((DS1307_HR > 9 )&&( DS1307_MIN >2 )&&( DS1307_MIN <=59 ))time4 = millis()+360; // Time delay between each pwm fade in pulse
if ((DS1307_HR > 9 )&&( DS1307_MIN >2 )&&( DS1307_MIN <=59 ))value4-=25; // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
if ((DS1307_HR > 9 )&&( DS1307_MIN >2 )&&( DS1307_MIN <=59 ))if (value4<=0)digitalWrite (led_2, LOW); // Turn pin fully on once faded in
}

if ((DS1307_HR > 7 )&&( DS1307_MIN >1 )) digitalWrite (led_3, HIGH);
if ((DS1307_HR > 9 )&&( DS1307_MIN >1 )) digitalWrite (led_3, LOW);

}

Your code is hard to read; could you please use the “code” (#) button when posting, please?

if(millis()>time1)
 {
   if ((DS1307_HR > 7 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 ))analogWrite(led_1, value1);
   if ((DS1307_HR > 7 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 ))time1 = millis()+360;            // Time delay between each pwm fade in pulse
   if ((DS1307_HR > 7 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 ))value1+=25;                  // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
   if ((DS1307_HR > 7 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 ))if (value1>255)digitalWrite (led_1, HIGH); // Turn pin fully on once faded in
 }

You’ve written the same condition four times here - why not factorise and simplify your code?

if(millis()>time1) {
   if ((DS1307_HR > 7 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 )) {
       analogWrite(led_1, value1);
       time1 = millis()+360;        // Time delay between each pwm fade in pulse
       value1+=25;                  // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
       if (value1>255) {
           digitalWrite (led_1, HIGH); // Turn pin fully on once faded in
       }    
    }       
}

Thanks for your help AWOL, I didn’t know there was a code button but now i do

Well that looks a little easier and clearer than what i had but its still not happening when applying it to a time to switch on and off

I can digitalwrite to each pin (without the time) so its not wiring. Have i missed something in the time call?

Here is the code with AWOL’s sugestion and i just wanted to play with the code button :slight_smile:

//***************************Initialisation Calls***********************************
#include <LiquidCrystal.h>
#include <WProgram.h>
#include <Wire.h>
#include <DS1307.h> 
#define led_1            3                           // Output to led 1 on pwm pin 3
#define led_2              5                           // Output to led 2 on digital pin 5
#define led_3             6                           // Output to led 3 on pwm pin 6

LiquidCrystal lcd(13, 12, 11, 8, 7, 4, 2);

int value1 = 0;
int value2 = 0;
int value3 = 255;
int value4 = 255;

long time1 = 0;
long time2 = 0;
long time3 = 0;
long time4 = 0;


void setup()
{
  Wire.begin();                                    // Starts 2 wire commuication with RTC module
  Serial.begin(9600);                              // Starts Serial communication with PC
  pinMode (led_1,          OUTPUT);                // Sets digital pin 3 as an output pin
  pinMode (led_2,           OUTPUT);                // Sets digital pin 5 as an output pin
  pinMode (led_3,          OUTPUT);                // Sets digital pin 6 as an output pin

  time1 = millis();
  time2 = millis();
  time3 = millis();
  time4 = millis();

  //***************************SET TIME & DATE******************************************

  //RTC.stop();
  //RTC.set(DS1307_SEC,1);        //set the seconds
  //RTC.set(DS1307_MIN,21);     //set the minutes
  //RTC.set(DS1307_HR,16);       //set the hours
  //RTC.set(DS1307_DOW,2);       //set the day of the week
  //RTC.set(DS1307_DATE,27);       //set the date
  //RTC.set(DS1307_MTH,7);        //set the month
  //RTC.set(DS1307_YR,9);         //set the year
  //RTC.start();

 
  //*****************************************************************************************
  // set LCD's number of columns and rows: 
  lcd.begin(4, 20);

  //Print a message to the LCD.
  lcd.setCursor(4, 0);
  lcd.print("Led Test");

}

void loop()

{
  //**************************LCD Print***************************************
  // set the cursor to column 0, line 1:
  lcd.setCursor(0, 1);
  lcd.print("Time: ");                 // time message
  lcd.print(RTC.get(DS1307_HR,true)); //read the hour and also update all the values by pushing in true
  lcd.print(":");
  lcd.print(RTC.get(DS1307_MIN,false));//read minutes without update (false)
  lcd.setCursor(0, 2);
  lcd.print("Date: ");                 // date message
  lcd.print(RTC.get(DS1307_DATE,false));//read date
  lcd.print("/");
  lcd.print(RTC.get(DS1307_MTH,false));//read month
  lcd.print("/");
  lcd.print(RTC.get(DS1307_YR,false)); //read year 


  //********************************Time Calls*********************************************

 
  if(millis()>time1) {
   if ((DS1307_HR > 7 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 )) {
       analogWrite(led_1, value1);
       time1 = millis()+360;        // Time delay between each pwm fade in pulse
       value1+=25;                  // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
       if (value1>255) {
           digitalWrite (led_1, HIGH); // Turn pin fully on once faded in
       }
    }
}
 
if(millis()>time2) {
   if ((DS1307_HR > 8 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 )) {
       analogWrite(led_2, value2);
       time2 = millis()+360;        // Time delay between each pwm fade in pulse
       value2+=25;                  // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
       if (value2>255) {
           digitalWrite (led_2, HIGH); // Turn pin fully on once faded in
       }
    }
}
 
if(millis()>time3) {
   if ((DS1307_HR > 21 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 )) {
       analogWrite(led_1, value3);
       time3 = millis()+360;        // Time delay between each pwm fade in pulse
       value3-=25;                  // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
       if (value3>255) {
           digitalWrite (led_1, LOW); // Turn pin fully on once faded in
       }
    }
}
 
if(millis()>time4) {
   if ((DS1307_HR > 21 )&&( DS1307_MIN >1  )&&( DS1307_MIN <=59 )) {
       analogWrite(led_2, value4);
       time4 = millis()+360;        // Time delay between each pwm fade in pulse
       value4+=25;                  // Number of pulse counts from 0 to fully on ( 0,25,50,75,100 etc)
       if (value4>255) {
           digitalWrite (led_2, LOW); // Turn pin fully on once faded in
       }
    }
}
 

  if ((DS1307_HR > 7  )&&( DS1307_MIN >1  )) digitalWrite (led_3, HIGH);
  if ((DS1307_HR > 21 )&&( DS1307_MIN >1  )) digitalWrite (led_3, LOW);

}

Ah! The dangers of cut-and-paste.

       value3-=25;
       if (value3>255) {
           digitalWrite (led_1, LOW); // Turn pin fully on once faded in
       }

If you find yourself typing the same, or nearly the same code, it usually means it's time for an array or two, and a "for" loop. This usually helps you separate what you want to do and what you want to do it to.

Yeah that will teach me to be lazy. lol
I should’ve changed the comment too

Still won’t work even after its corrected to <=0 though

I don't have access to any hardware or IDE at the moment, but one thing that springs to mind is that after turning the LED fully on or off with a "digitalWrite", you carry on incrementing or decrementing the "int" value. Eventually (at +/- 32K) this will wrap, and invalidate your test.

Can you explain what you want to do, and what you're actually observing?

I wish i could just turn on a led.. lol

Basically i'm just playing with an Aduino to see what it can do and learn my way through it. I know my way around circuits but not programming, so its just an excersise for the brain

I thought some simple automation like turning something like a led on and off at set times would be an easy start.

Well, one thing you can do with programming that you can't always do with hardware is make it simpler. Don't try debugging too much at once; start simple and work up to where you want to be. Have you tried the PWM examples?

I’ve tried it with straight PWM and no time call/alarm and it works. Personally i think it has something to do with just the time side of things.

It’s 6am here, so after another coffee i’ll try it with a different rtc library to see if it makes a difference.

Hey thanks for all help and feedback AWOL by the way