Please help with TimeAlarms problems

I have used TimeAlarms successfully on another project where I alarm 5 times a day, every day. This project I want to alarm on certain days as well as every day, however, for some reason, it has been alarming inappropriately on the incorrect day (as well as on the correct day). I've exhausted my troubleshooting capability, so I'd appreciate some help.

I've made a full featured plant waterer that waters 2 separate plants at different times as well providing a means to water manually. Additionally, I can place it in Auto made that then reads sensors to determine if it needs watering, so it will then water on an as needed basis. My problem here is in the Timer mode. It shows that the correct time is set, including the day, however it will water at the correct time but not the correct day. Additionally, I am using ARDUINO 0022 IDE. I have not taken the time to learn how to modify Time.h to use with the version 1.0 IDE. I'm using Sparkfun's Real Time Clock module.

Here's my code with only the salient lines (out of a total of 1000) reprinted. I am not a proficient coder so I'm sure it can be done more efficiently.

Can anyone tell why it is not working properly? It does not fail consistently so I'm having difficulty troubleshooting it.

/*  USE ARDUINO 0022 ONLY WITH THIS SKETCH */


#include <Time.h>  
#include <TimeAlarms.h>
#include <Wire.h>  //For DS1037
#define DS1307_ADDRESS 0x68//For DS1037

timeDayOfWeek_t  Water1aDay = dowWednesday;   //Time for first watering for Plant1 -Day
             int Water1aHr = 8;           //Time for first watering for Plant1 -HR
            int Water1aMin = 00;           //Time for first watering for Plant1 - Min
timeDayOfWeek_t Water1bDay = dowSaturday;  //Time for second watering for Plant1 -Day
             int Water1bHr = 20;           //Time for second watering for Plant1 -HR
            int Water1bMin = 00;           //Time for second watering for Plant1 - Min

timeDayOfWeek_t Water2aDay = dowWednesday;   //Time for first watering for Plant2 -Day
             int Water2aHr = 9;           //Time for first watering for Plant2 -HR
            int Water2aMin = 00;           //Time for first watering for Plant2 - Min
timeDayOfWeek_t Water2bDay = dowSaturday;  //Time for second watering for Plant2 -Day
             int Water2bHr = 21;           //Time for second watering for Plant2 -HR
            int Water2bMin = 00;           //Time for second watering for Plant2 - Min


// Convert binary coded decimal to normal decimal numbers for DS1037
 byte bcdToDec(byte val)  {
 return ( (val/16*10) + (val%16) );
  } 

   unsigned long asec = millis();
 unsigned long lastOn = millis();
unsigned long lastOff = millis();
   unsigned long timer=millis();  //timer for returning to top
      

  



void setup()
{
  Serial.begin(9600);
  
  
  Wire.begin();
  //setDateTime();   //sets time on DS1037  Comment out this after 
                   //running one time.  Set time in Subroutine at end of code
  // Reset the register pointer
  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.send(0);
  Wire.endTransmission();

  //Get time from DS1037
  Wire.requestFrom(DS1307_ADDRESS, 7);

    int asecond = bcdToDec(Wire.receive());
    int aminute = bcdToDec(Wire.receive());
      int ahour = bcdToDec(Wire.receive() & 0b111111); //24 hour time
   int aweekDay = bcdToDec(Wire.receive()); //1-7 -> sunday - Saturday
  int amonthDay = bcdToDec(Wire.receive());  //1-31??
     int amonth = bcdToDec(Wire.receive());
      int ayear = bcdToDec(Wire.receive());

   int StartHr = ahour;  
   int StartMn = aminute;  
   int StartSe = asecond;  
  int StartMth = amonth;
  int StartWeek = aweekDay;
  int StartDay =  amonthDay;  
   int StartYr =  ayear;  
  
  // Set up Time.h to current time based on DS1037 
  setTime(StartHr,StartMn,StartSe,StartDay,StartMth,StartYr);   
 //From TimeAlarm example:(changed header as described in readme
  //to include 20 or 23 alarms instead of the 6 in the original file
  Alarm.alarmRepeat(Water1aDay,Water1aHr,Water1aMin,0, Timer1);  // First watering per week
  Alarm.alarmRepeat(Water1bDay,Water1bHr,Water1bMin,0, Timer1);  // Second watering per week
  Alarm.alarmRepeat(Water2aDay,Water2aHr,Water2aMin,0, Timer2);  // First watering per week
  Alarm.alarmRepeat(Water2bDay,Water2bHr,Water2bMin,0, Timer2);  // Second watering per week
  
  //Check moisture level 3X per day for Auto Mode at 10/11AM, 2/3PM and 6/7PM
  Alarm.alarmRepeat( 10,0,0, Auto1);  // First check
  Alarm.alarmRepeat(14,0,0, Auto1);  // Second check
  Alarm.alarmRepeat(18,0,0, Auto1);  // third check
  Alarm.alarmRepeat( 11,0,0, Auto2);  // First check
  Alarm.alarmRepeat(15,0,0, Auto2);  // Second check
  Alarm.alarmRepeat(19,0,0, Auto2);  // third check
 
 //The following alarm code has not worked properly so I have commented it out
 //reset internal clock once a week since internal clock isn't as accurate as DS1037
 //Alarm.alarmRepeat(dowSunday, 0,0,0, SetInternalClock);  // Change time to = DS1037 every sunday at midmight
 
 
  //LED blinks faster just before watering time
  Alarm.alarmRepeat(Water1aDay,Water1aHr,Water1aMin-5,0, LEDblk5);  //  Fastest blink rate
  Alarm.alarmRepeat(Water1bDay,Water1bHr,Water1bMin-5,0, LEDblk5);  //  Fastest blink rate
  Alarm.alarmRepeat(Water2aDay,Water2aHr,Water2aMin-5,0, LEDblk5);  //  Fastest blink rate
  Alarm.alarmRepeat(Water2bDay,Water2bHr,Water2bMin-5,0, LEDblk5);  //  Fastest blink rate
  
  
}

void loop()
{

 /***************************************************************/ 
  Alarm.delay(100);   //Check alarms 
  digitalClockDisplay();    //display clock on Serial Port
   
      
  //Check if switch is off;
  
 if(digitalRead(AutoMode) && digitalRead(TimerMode) == 1)  //is switch is off? ie both open
                                                           // 1=open. 0=grounded
 {  
        
   OFF();
 }
 
 
 // Is a pump active?
   if (AlarmFlag1 ==1)  //If 1 is active, go to sub to see if time to shut off
 {
   Timer1();
 }
 
   if (AlarmFlag2 ==1)  //If 2 is active, go to sub to see if time to shut off
 {
   Timer2();
 }
 
   if (ActiveAutoMode1 ==1)  //If auto is active, go to sub to see if time to shut off
 {
   Auto1();
 }
 
   if (ActiveAutoMode2 ==1)  //If auto is active, go to sub to see if time to shut off
 {
   Auto2();
 }
 
   
 }  //end Void loop



 
 // Timer1 Mode subroutine.  Sub is called when a timer alarm is activated
 
 void Timer1()
 {
   //Is it in Timer Mode?  If not, skip to end
   if(digitalRead(TimerMode) == LOW)
   {
   //Timer mode operation here
   if (AlarmFlag1 == 0)  // is this the first time?
   {
     AlarmFlag1 = 1;  //Alarm just occured
     TimeOn1 = millis();  //start timer
     blkOn = 750;      //slow LED blink 
     blkOff = 750;      //slow LED blink 
     digitalWrite(valve1,HIGH);  //turn the valve on
     digitalWrite(pump,HIGH);  //turn on pump
   }
   else  //if not the first time
   {
     if ( millis() - pumpTime1>TimeOn1 )  // is it time to turn off?
     {
     blkOn = 0;      //no LED blink 
     blkOff = 0;      //no LED blink 
     digitalWrite(ledPin, HIGH);  //turn on LED
     digitalWrite (pump,LOW);   //turn pump off
     digitalWrite(valve1,LOW);  //turn off valve
     AlarmFlag1 = 0;  //turn off flag
     TimeOn1 = 0;  //reset timer
     }
   
    }
   }  //in Timer Mode?
 }  //end of timer1
 /**************************************************************/
 
 // Timer2 Mode subroutine
 
void Timer2()
{
 /* Same as Timer1() */
 
}




 void setDateTime(){//for setting time on DS1037 from bildr.org/?s=ds1307
  Serial.println("setDateTime called");
  byte second =      0; //0-59
  byte minute =     4; //0-59
  byte hour =        18; //0-23
  byte weekDay =     3; //1-7  ALWAYS SET ACCORDING TO monthDay
  byte monthDay =    14; //1-31
  byte month =       4; //1-12
  byte year  =       15; //0-99

  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.send(0); //stop Oscillator

  Wire.send(decToBcd(second));
  Wire.send(decToBcd(minute));
  Wire.send(decToBcd(hour));
  Wire.send(decToBcd(weekDay));
  Wire.send(decToBcd(monthDay));
  Wire.send(decToBcd(month));
  Wire.send(decToBcd(year));
  Wire.send(0); //start 
  Wire.endTransmission();

}

Delta_G:
Write always this way:

if(current_time - older_time > interval)

Always use subtraction when calculating an interval of time.

Yes, thank you. I kind of knew that but had delayed fixing it. But I don't understand the difference between yours and mine. I assume this is the same as yours:??

if(millis() - TimeOn1 > pumpTime1)

If it is, I don't understand how that fixes the issue.

Thanks for your help.

It is unwise to troubleshoot a program with known issues. The whole point is that you don't know what is wrong. If you fix the things you know are wrong, what's the worst that can happen? ... and you never know, the program might work (since you didn't know what was wrong, remember?). If not, the work is done and you don't have to do it later.

But more importantly, it burdens people who are trying to help by finding errors. Because then they have to stop and ask themselves about the same errors, and repeat the same thinking (and perhaps ruling out) that you have done.

Delta_G:
Written the way I wrote it it would be immune to any rollover issues.

Got it! I edited the code in my original post.

Thanks.

I edited the code in my original post.

I do wish people would not do that because it is confusing for anyone reading the thread later.

You appear to have 14 alarms set. Have you modified TimeAlarms.h as suggested in the readme ?

Q: How many alarms can be created?
A: Up to six alarms can be scheduled.
The number of alarms can be changed in the TimeAlarms header file (set by the constant dtNBR_ALARMS,
note that the RAM used equals dtNBR_ALARMS * 11)

There is a good chance that you are running out of memory

By the way, I do not understand your reference to needing to modify Time.h to work with IDE 1.0.x Can you please expand on that ?

UKHeliBob:
I do wish people would not do that because it is confusing for anyone reading the thread later.

You appear to have 14 alarms set. Have you modified TimeAlarms.h as suggested in the readme ?There is a good chance that you are running out of memory

By the way, I do not understand your reference to needing to modify Time.h to work with IDE 1.0.x Can you please expand on that ?

Yes, I did modify the number of alarms allowed as detailed in the readme. I have used it successfully modified for 22 alarms (but did not have any alarms for specific days of the week as I'm using here - only alarms every day). How do I check to see if I'm out of memory? When I load it, the IDE says I'm using 16768 bytes out of 32256. Is that the memory you're talking about?

Relative to Time.h: When I run this program with IDE 1.0 it shows errors with the time function as documented here: Arduino Playground - HomePage

"Update: newer versions of Time, TimeAlarms, and DS1307RTC are available, featuring more updates for newer version of Arduino and compatibility with Arduino Due.
The code is derived from the earlier Playground DateTime library but is updated to provide an API that is more flexible and easier to use.

This old download does not work on Arduino 1.6.1."

Thanks for your help.

Delta_G:
As to the problem with timeAlarms, I don't know. I wouldn't use a library for something that simple.

I guess I didn't think of doing it myself. Are there any pitfalls to using a statement like this in my loop() where it would 'alarm' within 60 seconds of when I want it to:

if ( weekday() == 2 && hour()  == 8 && minute() == 0)
{
Timer1();
}

But this will trip a number of times while within that minute so I guess I'd have to use a flag to note the first time and reset the flag after 1 minute? Otherwise, if I include " && seconds() == 0 " the processor may be busy at that precise time and not trip.

Thanks

so in that case maybe I would need to use an interrup?

No. The value for the second will be the same for 1000 milliseconds. If your code doesn't call loop() more than once a second, you are doing something wrong.

Timer1();

This is analogous to digitalWrite() being called f2134(). There is NOTHING to suggest what this function does, which makes it difficult to know that it is even the right function to call.

PaulS:
No. The value for the second will be the same for 1000 milliseconds. If your code doesn't call loop() more than once a second, you are doing something wrong.

Timer1();

This is analogous to digitalWrite() being called f2134(). There is NOTHING to suggest what this function does, which makes it difficult to know that it is even the right function to call.

No, I'm worried it will call Timer1() subroutine more than once during that 1000ms and therefore duplicate what is in Timer1() sub.

Timer1() is the sub that starts the watering and monitors the lenght of time for watering - see code in OP.

Delta_G:
So yeah, you'll need some sort of flag to let you know it's already happened this time round. Be sure to include code to reset the flag later.

So wouldn't waterThePlants() be a better name for said function?

I was formulating my response. It would have been exactly the same.