Go Down

Topic: bug in the timealarm library (Read 34530 times) previous topic - next topic

viknet

#30
Jun 30, 2011, 07:59 pm Last Edit: Jun 30, 2011, 08:03 pm by viknet Reason: 1
sorry mem, not that I want to argue with you, but I think you missed my point :-)
I continue to think that the
Code: [Select]
Mode.isEnabled = 0; // values more than a week but less than today have expired so the alarm is disabled

will never be run (and that the previous 'if' statement is useless) could you check the simplification of the code made below ?

 
Code: [Select]
   if( value > SECS_PER_WEEK )
     { ... }
     else //VIK: value <= SECS_PER_WEEK for all the command within the else statement (MARKED as an X)
  X   {
  X     if ( value <= SECS_PER_DAY)
  X     { ... }
  X     else //VIK: value > SECS_PER_DAY
  X     {
  X       if ( value <= SECS_PER_WEEK)  //VIK: this condition is always true and useless (IMHO)
  X       {  ...  }
  X     }
  X   }


Best regards.

P.S.

I will upgrade the code in the playground once we sort this out :-)

mem

vikent, we are not arguing, we are clarifying  ;)

My post was referring to the   Mode.isEnabled = 0 code at the end of the method. As you say, the earlier test for value <= SECS_PER_WEEK is redundant.

Please post your revised code here in this thread so it can be reviewed before going live. I will update the playground with these changes (along with the new method to get the alarm id) when we are sure that the fix hasn't broken any of the other functionality.

Many thanks for working in this, I will note your contribution in the code when I update it in the playground.

I hope draythomp is still around so he can try it out. I think he wins the prize for sketchs with the most alarms.


Michael



viknet

#32
Jul 01, 2011, 12:24 am Last Edit: Jul 01, 2011, 12:26 am by viknet Reason: 1
hello, mem,
do you agree with this code :

Code: [Select]
void AlarmClass::updateNextTrigger()
{  
 if( (value != 0) && Mode.isEnabled )
 {
   time_t time = now();
   if(Mode.isAlarm && nextTrigger <= time )   // update alarm if next trigger is not yet in the future
   {
       // value is less than a day for daily alarm
       // value is between a day and a week+day for a weekly alarm
       // value is more than SECS_PER_WEEK+SECS_PER_DAY to represent an alarm in the future
       // in this case value is a LOT more than SECS_PER_WEEK+SECS_PER_DAY
       // beware you cannot set an alarm for the first 8 days in 1970 (you should'nt need unless you are Marty McFly)
       //
       // All this issues may need to be addressed using
       // a second private field instead of relaying on the value
       //
     
     if( value > (SECS_PER_WEEK +SECS_PER_DAY) ) // is the value a specific date and time in the future
       {
         nextTrigger = value;  // yes, trigger on this value  
       }
     else    // this value is only an hour or a weekday and an hour
     {
       if ( value <= SECS_PER_DAY) //if the value is a daily alarm
       {
         if( value + previousMidnight(now()) <= time)
         {
           nextTrigger = value + nextMidnight(time); // if time has passed then set for tomorrow
         }
         else
         {
           nextTrigger = value + previousMidnight(time);  // set the date to today and add the time given in value  
         }
       }
       else //else the value is a weekly alarm
       {
         if( (value - SECS_PER_DAY + previousSunday(now())) <= time)
         {
           nextTrigger = value - SECS_PER_DAY + nextSunday(time); // if day has passed then set for the next week. because weekly start at  SECS_PER_DAY  withdraw this amount
         }
         else
         {
           nextTrigger = value - SECS_PER_DAY + previousSunday(time);  // set the date to this week today and add the time given in value
         }
       }
     }
   }
   if(Mode.isAlarm == false)
   {
     // its a timer
     nextTrigger = time + value;  // add the value to previous time (this ensures delay always at least Value seconds)
   }
 }
 else
 {
   Mode.isEnabled = 0;  // Disable if the value is 0
 }
}


I also find a small but not so easy to correct bug:

the previousSunday(_time_) macro is not safe when used with time beeing the first few days of 1970. this is not a big issue because this time are not usefull,
but sometimes you might initialized your alarm before setting the correct time and in this case the bug is triggered and the nexttrigger is undefined.

I don't know how to nicely correct the issue, in the previousSunday(_time_) we could add a "max(old macro,0 )" to be sure it never goes beyond 0.
I have done it and it is not working work probably because time is an unsigned integer.
so I have changed my macro to :
Code: [Select]
#define previousSunday(_time_)  ((( max(_time_,SECS_PER_WEEK) / SECS_PER_DAY) * SECS_PER_DAY)  - (dayOfWeek(_time_)-1)*SECS_PER_DAY
It is much more failsafe but a bit cumbersome :-)

I also tested your AlarmID_t TimeAlarmsClass::getTriggeredAlarmId() and it is working as advertised :-)

So we are probably ready to update the playground now that the argument is over  ;)

regards

Viknet (Vincent Valdy in real life if you want to quote my contribution)

mem

#33
Jul 01, 2011, 10:52 am Last Edit: Jul 01, 2011, 10:55 am by mem Reason: 1
Vincent, thanks for that.

I wonder why your test for weekly alarms is SECS_PER_WEEK +SECS_PER_DAY and not  just SECS_PER_WEEK?

I simplified the previousSunday macro by using the existing elapsedSecsThisWeek macro in Time.h
Code: [Select]

#define previousSunday(_time_)  (_time_ - elapsedSecsThisWeek(_time_))  // time at the start of the week for the given time
#define nextSunday(_time_) ( previousSunday(time)+SECS_PER_WEEK)  // time at the end of the week for the given time


I think we can address potential macro underflow problems in documentation. I will add the following to the FAQ:
Q: Can the clock and alarms be set for times in the past?
A: The algorithm to handle alarms works for times after Jan 1 1971 (i.e. at least one year after time 0)

Here is the updateNextTrigger method for testing, I have added some private macros to determine the alarm type that should make the code easier to enhance in future. This adds some redundancy in the tests but is easier to read and enhance. I have not tested these, feel free to fix them if necessary.
Code: [Select]
//**************************************************************
//* Private Methods

// private macros to determine the alarm type
// these may be replaced with the use of a private field if more alarm types are needed
#define isDailyAlarm    (value <= SECS_PER_DAY) //  value is less than a day for daily alarm
#define isWeeklyAlarm   (value > SECS_PER_DAY && value <= SECS_PER_WEEK)  // more than a day and up to one week
#define isExplicitAlarm (value > SECS_PER_WEEK) // alarm is at the given time and date      

void AlarmClass::updateNextTrigger()
{  
 if( (value != 0) && Mode.isEnabled )
 {
   time_t time = now();
   if(Mode.isAlarm && nextTrigger <= time )   // update alarm if next trigger is not yet in the future
   {      
     if(isExplicitAlarm ) // is the value a specific date and time in the future
     {
       nextTrigger = value;  // yes, trigger on this value  
     }
     else if(isDailyAlarm)  //if this is a daily alarm
     {
       if( value + previousMidnight(now()) <= time)
       {
         nextTrigger = value + nextMidnight(time); // if time has passed then set for tomorrow
       }
       else
       {
         nextTrigger = value + previousMidnight(time);  // set the date to today and add the time given in value  
       }
     }
     else if(isWeeklyAlarm)  // if this is a weekly alarm
     {
       if( (value - SECS_PER_DAY + previousSunday(now())) <= time)
       {
         nextTrigger = value - SECS_PER_DAY + nextSunday(time); // if day has passed then set for the next week. because weekly start at  SECS_PER_DAY  withdraw this amount
       }
       else
       {
         nextTrigger = value - SECS_PER_DAY + previousSunday(time);  // set the date to this week today and add the time given in value
       }
     }
     else  // its none of the above (a time that is
     {
       Mode.isEnabled = 0;  // Disable the alarm
     }  
   }
   if(Mode.isAlarm == false)
   {
     // its a timer
     nextTrigger = time + value;  // add the value to previous time (this ensures delay always at least Value seconds)
   }
 }
 else
 {
   Mode.isEnabled = 0;  // Disable if the value is 0
 }
}



viknet

hello mem,
Quote
I wonder why your test for weekly alarms is SECS_PER_WEEK +SECS_PER_DAY and not  just SECS_PER_WEEK?


day does not start from 0 but from 1 so the highest "value" of a valid weekly alarm is dowSaturday*SECS_PER_DAY+AlarmHMS(23,59,59)
dowSaturday*SECS_PER_DAY=SECS_PER_WEEK and AlarmHMS(23,59,59) is SECS_PER_DAY.

This would not work if day were starting at 0, because for value between 0 and  SECS_PER_DAY you would'nt know if it was a daily alarm or a weekly sunday alarm.

I will check your macro this evening, concerning the underflow issue, the problem is that it is quite common to start sketch and assign a weekly repeating alarm before having the time correctly set (through serial in my case) so you have to manually issue during setup a setTime (SECS_PER_YEAR) for exemple to start in 1971, I did that before correcting the macro but it is not "end-programmer" friendly.
if you don't modify the macro nor issue the setTime the the nextrigger is wrongly calculated way way in the future and the alarm is never triggered (or will be but in many many years)


regards

Viknet

mem

I forgot that I changed the start of week to day 1, the first version of this library started the week at day 0.

Bear in mind that the library defines time_t  as unsigned, so the max function may not be the right solution to underflow.  Setting alarms before setting the time creates problems other than just the underflow so one solution is to modify the create method so it will only create recurring day/week alarms if the current time has been set to a value  on or after Jan 1 1971. This assumes its better to prevent an alarm than create one that is likely to go off on the wrong time or day because the clock was set after the alarm was created.

The duration Timers could ignore this check as they are not susceptible to underflow. 

Thoughts?

Michael

viknet

Quote
Bear in mind that the library defines time_t  as unsigned, so the max function may not be the right solution to underflow

I know, that was my first approach, but it did not work, so I move the max to
max(_time_,SECS_PER_WEEK), this is always more than SECS_PER_WEEK so within the calculation a negative value could never append.
it just not really accurate because it give the next sunday instead of the previous during the first 3 days of 1970 (because the fourth day is a sunday and the macro become accurate again).
Any weekly alarm for the (thursday friday or saturday) set before the fourth of january 1970 will only trigger the next week everything else is OK

beware that
Code: [Select]
#define elapsedSecsThisWeek(_time_)  (elapsedSecsToday(_time_) +  (dayOfWeek(_time_) * SECS_PER_DAY) )   

Is not accurate, because DayOfWeek Starts at 1. the correct macro needs to be:
Code: [Select]
#define elapsedSecsThisWeek(_time_)  (elapsedSecsToday(_time_) +  ((dayOfWeek(_time_)-1) * SECS_PER_DAY) )   

you could then define
Code: [Select]
#define previousSunday(_time_)  (max(_time_,SECS_PER_WEEK) - elapsedSecsThisWeek(_time_))

Concerning the idea of checking time is after Jan 1 1971 it is also very acceptable.

Best regards

Viknet

mem

I have not had a chance to run these but here is the revised code for testing. I hope draythomp  and others will give this a good workout.

Here is the TimeAlarms.h file:

Code: [Select]

#ifndef TimeAlarms_h
#define TimeAlarms_h
//  TimeAlarms.h - Arduino Time alarms header for use with Time library 

#include <inttypes.h>

#include "Time.h"

#define dtNBR_ALARMS 6

typedef enum { dtMillisecond, dtSecond, dtMinute, dtHour, dtDay } dtUnits_t;

typedef struct  {
    uint8_t isAllocated            :1 ;  // the alarm is avialable for allocation if false
    uint8_t isEnabled              :1 ;  // the timer is only actioned if isEnabled is true
    uint8_t isOneShot              :1 ;  // the timer will be de-allocated after trigger is processed
    uint8_t isAlarm                :1 ;  // time of day alarm if true, period timer if false 
}
    AlarmMode_t   ;

typedef uint8_t AlarmID_t;
typedef AlarmID_t AlarmId;  // Arduino friendly name
#define dtINVALID_ALARM_ID 255


class AlarmClass;  // forward reference
typedef void (*OnTick_t)();  // alarm callback function typedef

// class defining an alarm instance, only used by dtAlarmsClass
class AlarmClass

private:
public:
  AlarmClass();
  OnTick_t onTickHandler; 
  void updateNextTrigger();
  time_t value;
  time_t nextTrigger;
  AlarmMode_t Mode;
};

// class containing the collection of alarms
class TimeAlarmsClass
{
private:
   AlarmClass Alarm[dtNBR_ALARMS];
   void serviceAlarms();
   uint8_t isServicing;
   uint8_t servicedAlarmId; // the alarm currently being serviced
   AlarmID_t create( time_t value, OnTick_t onTickHandler, uint8_t isAlarm, uint8_t isOneShot, uint8_t isEnabled=true);
   
public:
  TimeAlarmsClass();
  // functions to create alarms and timers
  AlarmID_t alarmRepeat(time_t value, OnTick_t onTickHandler);                    // trigger daily at given time of day
  AlarmID_t alarmRepeat(const int H,  const int M,  const int S, OnTick_t onTickHandler); // as above, with hms arguments
  AlarmID_t alarmRepeat(const timeDayOfWeek_t DOW, const int H,  const int M,  const int S, OnTick_t onTickHandler); // as above, with day of week

  AlarmID_t alarmOnce(time_t value, OnTick_t onTickHandler);                     // trigger once at given time of day
  AlarmID_t alarmOnce( const int H,  const int M,  const int S, OnTick_t onTickHandler);  // as above, with hms arguments
  AlarmID_t alarmOnce(const timeDayOfWeek_t DOW, const int H,  const int M,  const int S, OnTick_t onTickHandler); // as above, with day of week
 
  AlarmID_t timerOnce(time_t value, OnTick_t onTickHandler);   // trigger once after the given number of seconds
  AlarmID_t timerOnce(const int H,  const int M,  const int S, OnTick_t onTickHandler);   // As above with HMS arguments
 
  AlarmID_t timerRepeat(time_t value, OnTick_t onTickHandler); // trigger after the given number of seconds continuously
  AlarmID_t timerRepeat(const int H,  const int M,  const int S, OnTick_t onTickHandler);   // As above with HMS arguments
 
  void delay(unsigned long ms);
   
  // utility methods
  uint8_t getDigitsNow( dtUnits_t Units);         // returns the current digit value for the given time unit
  void waitForDigits( uint8_t Digits, dtUnits_t Units); 
  void waitForRollover(dtUnits_t Units);

  // low level methods
  void enable(AlarmID_t ID);
  void disable(AlarmID_t ID);
  void write(AlarmID_t ID, time_t value);    // write the value (and enable) the alarm with the given ID
  time_t read(AlarmID_t ID);                 // return the value for the given timer
  AlarmID_t getTriggeredAlarmId();  //returns the currently triggered  alarm id
};

extern TimeAlarmsClass Alarm;  // make an instance for the user

/*==============================================================================
* MACROS
*============================================================================*/

/* public */
#define waitUntilThisSecond(_val_) waitForDigits( _val_, dtSecond)
#define waitUntilThisMinute(_val_) waitForDigits( _val_, dtMinute)
#define waitUntilThisHour(_val_)   waitForDigits( _val_, dtHour)
#define waitUntilThisDay(_val_)    waitForDigits( _val_, dtDay)
#define waitMinuteRollover() waitForRollover(dtSecond)
#define waitHourRollover()   waitForRollover(dtMinute)
#define waitDayRollover()    waitForRollover(dtHour)

#define AlarmHMS(_hr_, _min_, _sec_) (_hr_ * SECS_PER_HOUR + _min_ * SECS_PER_MIN + _sec_)

#endif /* TimeAlarms_h */









mem

#38
Jul 01, 2011, 09:49 pm Last Edit: Jul 01, 2011, 11:38 pm by mem Reason: 1
The Time.h File:
Code: [Select]
/*
 time.h - low level time and date functions
*/

#ifndef _Time_h
#define _Time_h

#include <inttypes.h>

typedef unsigned long time_t;

typedef enum {timeNotSet, timeNeedsSync, timeSet
}  timeStatus_t ;

typedef enum {
   dowInvalid, dowSunday, dowMonday, dowTuesday, dowWednesday, dowThursday, dowFriday, dowSaturday
} timeDayOfWeek_t;

typedef enum {
   tmSecond, tmMinute, tmHour, tmWday, tmDay,tmMonth, tmYear, tmNbrFields
} tmByteFields;  

typedef struct  {
 uint8_t Second;
 uint8_t Minute;
 uint8_t Hour;
 uint8_t Wday;   // day of week, sunday is day 1
 uint8_t Day;
 uint8_t Month;
 uint8_t Year;   // offset from 1970;
} tmElements_t, TimeElements, *tmElementsPtr_t;

//convenience macros to convert to and from tm years
#define  tmYearToCalendar(Y) ((Y) + 1970)  // full four digit year
#define  CalendarYrToTm(Y)   ((Y) - 1970)
#define  tmYearToY2k(Y)      ((Y) - 30)    // offset is from 2000
#define  y2kYearToTm(Y)      ((Y) + 30)  

typedef time_t(*getExternalTime)();
//typedef void  (*setExternalTime)(const time_t); // not used in this version


/*==============================================================================*/
/* Useful Constants */
#define SECS_PER_MIN  (60UL)
#define SECS_PER_HOUR (3600UL)
#define SECS_PER_DAY  (SECS_PER_HOUR * 24UL)
#define DAYS_PER_WEEK (7UL)
#define SECS_PER_WEEK (SECS_PER_DAY * DAYS_PER_WEEK)
#define SECS_PER_YEAR (SECS_PER_WEEK * 52UL)
#define SECS_YR_2000  (946684800UL) // the time at the start of y2k

/* Useful Macros for getting elapsed time */
#define numberOfSeconds(_time_) (_time_ % SECS_PER_MIN)  
#define numberOfMinutes(_time_) ((_time_ / SECS_PER_MIN) % SECS_PER_MIN)
#define numberOfHours(_time_) (( _time_% SECS_PER_DAY) / SECS_PER_HOUR)
#define dayOfWeek(_time_)  ((( _time_ / SECS_PER_DAY + 4)  % DAYS_PER_WEEK)+1) // 1 = Sunday
#define elapsedDays(_time_) ( _time_ / SECS_PER_DAY)  // this is number of days since Jan 1 1970
#define elapsedSecsToday(_time_)  (_time_ % SECS_PER_DAY)   // the number of seconds since last midnight
// The following macros are used in calculating alarms and assume the clock will not underflow
// Always set the correct time before settting alarms
#define previousMidnight(_time_) (( _time_ / SECS_PER_DAY) * SECS_PER_DAY)  // time at the start of the given day
#define nextMidnight(_time_) ( previousMidnight(_time_)  + SECS_PER_DAY ) // time at the end of the given day
#define elapsedSecsThisWeek(_time_)  (elapsedSecsToday(_time_) +  ((dayOfWeek(_time_)-1) * SECS_PER_DAY) )   // note that week starts on day 1
#define previousSunday(_time_)  (_time_ - elapsedSecsThisWeek(_time_))  // time at the start of the week for the given time
#define nextSunday(_time_) ( previousSunday(time)+SECS_PER_WEEK)  // time at the end of the week for the given time


/* Useful Macros for converting elapsed time to a time_t */
#define minutesToTime_t ((M)) ( (M) * SECS_PER_MIN)  
#define hoursToTime_t   ((H)) ( (H) * SECS_PER_HOUR)  
#define daysToTime_t    ((H)) ( (D) * SECS_PER_DAY)
#define weeksToTime_t   ((W)) ( (W) * SECS_PER_WEEK)  

/*============================================================================*/
/*  time and date functions   */
int     hour();            // the hour now
int     hour(time_t t);    // the hour for the given time
int     hourFormat12();    // the hour now in 12 hour format
int     hourFormat12(time_t t); // the hour for the given time in 12 hour format
uint8_t isAM();            // returns true if time now is AM
uint8_t isAM(time_t t);    // returns true the given time is AM
uint8_t isPM();            // returns true if time now is PM
uint8_t isPM(time_t t);    // returns true the given time is PM
int     minute();          // the minute now
int     minute(time_t t);  // the minute for the given time
int     second();          // the second now
int     second(time_t t);  // the second for the given time
int     day();             // the day now
int     day(time_t t);     // the day for the given time
int     weekday();         // the weekday now (Sunday is day 1)
int     weekday(time_t t); // the weekday for the given time
int     month();           // the month now  (Jan is month 1)
int     month(time_t t);   // the month for the given time
int     year();            // the full four digit year: (2009, 2010 etc)
int     year(time_t t);    // the year for the given time

time_t now();              // return the current time as seconds since Jan 1 1970
void    setTime(time_t t);
void    setTime(int hr,int min,int sec,int day, int month, int yr);
void    adjustTime(long adjustment);

/* date strings */
#define dt_MAX_STRING_LEN 9 // length of longest date string (excluding terminating null)
char* monthStr(uint8_t month);
char* dayStr(uint8_t day);
char* monthShortStr(uint8_t month);
char* dayShortStr(uint8_t day);

/* time sync functions */
timeStatus_t timeStatus(); // indicates if time has been set and recently synchronized
void    setSyncProvider( getExternalTime getTimeFunction); // identify the external time provider
void    setSyncInterval(time_t interval); // set the number of seconds between re-sync

/* low level functions to convert to and from system time                     */
void breakTime(time_t time, tmElements_t &tm);  // break time_t into elements
time_t makeTime(tmElements_t &tm);  // convert time elements into time_t


#endif /* _Time_h */



the TimeAlarms.cpp file is in the next post

mem

the TimeAlarms.cpp file:
Code: [Select]

//  TimeAlarms.cpp - Arduino Time alarms for use with Time library  
//  1 July 2011 - added fix for weekly alarms contributed by Vincent Valdy

extern "C" {
#include <string.h> // for memset
}

#include <WProgram.h>
#include "TimeAlarms.h"
#include "Time.h"

#define IS_ONESHOT  true
#define IS_REPEAT   false
#define IS_ALARM    true
#define IS_TIMER    false

AlarmClass::AlarmClass()
{
 Mode.isAlarm =  Mode.isEnabled = Mode.isOneShot = 0;
 value = nextTrigger = 0;
 onTickHandler = NULL;  
}


// private macros to determine the alarm type
// these may be replaced with the use of a private field if more alarm types are needed
#define isDailyAlarm    (value <= SECS_PER_DAY) //  value is less than a day for daily alarm
#define isWeeklyAlarm   (value > SECS_PER_DAY && value <= SECS_PER_WEEK+SECS_PER_DAY )  // more than a day and up to one week
#define isExplicitAlarm (value > SECS_PER_WEEK) // alarm is at the given time and date      

void AlarmClass::updateNextTrigger()
{  
 if( (value != 0) && Mode.isEnabled )
 {
   time_t time = now();
   if(Mode.isAlarm && nextTrigger <= time )   // update alarm if next trigger is not yet in the future
   {      
     if(isExplicitAlarm ) // is the value a specific date and time in the future
     {
       nextTrigger = value;  // yes, trigger on this value  
     }
     else if(isDailyAlarm)  //if this is a daily alarm
     {
       if( value + previousMidnight(now()) <= time)
       {
         nextTrigger = value + nextMidnight(time); // if time has passed then set for tomorrow
       }
       else
       {
         nextTrigger = value + previousMidnight(time);  // set the date to today and add the time given in value  
       }
     }
     else if(isWeeklyAlarm)  // if this is a weekly alarm
     {
       if( (value - SECS_PER_DAY + previousSunday(now())) <= time)
       {
         nextTrigger = value - SECS_PER_DAY + nextSunday(time);
       }
       else
       {
         nextTrigger = value - SECS_PER_DAY + previousSunday(time);  
       }
     }
     else  // its none of the above (a time that is
     {
       Mode.isEnabled = 0;  // Disable the alarm
     }  
   }
   if(Mode.isAlarm == false)
   {
     // its a timer
     nextTrigger = time + value;  
   }
 }
 else
 {
   Mode.isEnabled = 0;  // Disable if the value is 0
 }
}

//**************************************************************
//* Time Alarms Public Methods

TimeAlarmsClass::TimeAlarmsClass()
{
 isServicing = false;
 for(uint8_t id = 0; id < dtNBR_ALARMS; id++)
    Alarm[id].Mode.isAllocated = false;
}

AlarmID_t TimeAlarmsClass::alarmOnce(time_t value, OnTick_t onTickHandler){   // trigger once at the given time of day
  return create( value, onTickHandler, IS_ALARM, IS_ONESHOT );
}

AlarmID_t TimeAlarmsClass::alarmOnce(const int H,  const int M,  const int S,OnTick_t onTickHandler){  
  return create( AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_ONESHOT );
}

AlarmID_t TimeAlarmsClass::alarmOnce(const timeDayOfWeek_t DOW, const int H,  const int M,  const int S, OnTick_t onTickHandler){  // as above, with day of week
  unsigned long dayOffset =  ( 7 + DOW - dayOfWeek(now())) %7;
  return create( (dayOffset * SECS_PER_DAY) + AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_ONESHOT );  
}
 
AlarmID_t TimeAlarmsClass::alarmRepeat(time_t value, OnTick_t onTickHandler){
    return create( value, onTickHandler, IS_ALARM, IS_REPEAT );
}

AlarmID_t TimeAlarmsClass::alarmRepeat(const int H,  const int M,  const int S, OnTick_t onTickHandler){
    return create( AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_REPEAT );
}

AlarmID_t TimeAlarmsClass::alarmRepeat(const timeDayOfWeek_t DOW, const int H,  const int M,  const int S, OnTick_t onTickHandler){  // as above, with day of week
  unsigned long dayOffset =  ( 7 + DOW - dayOfWeek(now())) %7;
  return create( (dayOffset * SECS_PER_DAY) + AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_REPEAT );      
}
 
AlarmID_t TimeAlarmsClass::timerOnce(time_t value, OnTick_t onTickHandler){  
    return create( value, onTickHandler, IS_TIMER, IS_ONESHOT );
}

AlarmID_t TimeAlarmsClass::timerOnce(const int H,  const int M,  const int S, OnTick_t onTickHandler){   // As above with HMS arguments
 return create( AlarmHMS(H,M,S), onTickHandler, IS_TIMER, IS_ONESHOT );
}
 
AlarmID_t TimeAlarmsClass::timerRepeat(time_t value, OnTick_t onTickHandler){ // trigger after the given number of seconds continuously
    return create( value, onTickHandler, IS_TIMER, IS_REPEAT);
}

AlarmID_t TimeAlarmsClass::timerRepeat(const int H,  const int M,  const int S, OnTick_t onTickHandler){
    return create( AlarmHMS(H,M,S), onTickHandler, IS_TIMER, IS_REPEAT);
}

void TimeAlarmsClass::enable(AlarmID_t ID)
{
 if(ID < dtNBR_ALARMS && Alarm[ID].Mode.isAllocated){
   Alarm[ID].Mode.isEnabled = (Alarm[ID].value != 0) && (Alarm[ID].onTickHandler != 0) ;  
   Alarm[ID].updateNextTrigger(); // trigger is updated whenever  this is called, even if already enabled
 }
}

void TimeAlarmsClass::disable(AlarmID_t ID)
{
 if(ID < dtNBR_ALARMS && Alarm[ID].Mode.isAllocated)
   Alarm[ID].Mode.isEnabled = false;
}

// write the given value to the given alarm
void TimeAlarmsClass::write(AlarmID_t ID, time_t value)
{
 if(ID < dtNBR_ALARMS && Alarm[ID].Mode.isAllocated){
   Alarm[ID].value = value;
   enable(ID);
 }
}

// return the value for the given alarm
time_t TimeAlarmsClass::read(AlarmID_t ID)
{
 if(ID < dtNBR_ALARMS && Alarm[ID].Mode.isAllocated)
   return Alarm[ID].value ;
 else
   return 0l;  
}

AlarmID_t TimeAlarmsClass::getTriggeredAlarmId()  //returns the currently triggered  alarm id
// returns  dtINVALID_ALARM_ID if not invoked from within an alarm handler
{
 if(isServicing)
      return  servicedAlarmId;  // new private data member used instead of local loop variable i in serviceAlarms();
 else
    return dtINVALID_ALARM_ID; // valid ids only available when servicing a callback
}

// following functions are not Alarm ID specific.
void TimeAlarmsClass::delay(unsigned long ms)
{
 unsigned long start = millis();
 while( millis() - start  <= ms)
   serviceAlarms();
}

void TimeAlarmsClass::waitForDigits( uint8_t Digits, dtUnits_t Units)
{
 while(Digits != getDigitsNow(Units) )
 {
   serviceAlarms();
 }
}

void TimeAlarmsClass::waitForRollover( dtUnits_t Units)
{
 while(getDigitsNow(Units) == 0  ) // if its just rolled over than wait for another rollover                            
   serviceAlarms();
 waitForDigits(0, Units);
}

uint8_t TimeAlarmsClass::getDigitsNow( dtUnits_t Units)
{
 time_t time = now();
 if(Units == dtSecond) return numberOfSeconds(time);
 if(Units == dtMinute) return numberOfMinutes(time);
 if(Units == dtHour) return numberOfHours(time);
 if(Units == dtDay) return dayOfWeek(time);
 return 255;  // This should never happen
}


void TimeAlarmsClass::serviceAlarms()
{
 if(! isServicing)
 {
   isServicing = true;
   for( servicedAlarmId = 0; servicedAlarmId < dtNBR_ALARMS; servicedAlarmId++)
   {
     if( Alarm[servicedAlarmId].Mode.isEnabled && (now() >= Alarm[servicedAlarmId].nextTrigger)  )
     {
       OnTick_t TickHandler = Alarm[servicedAlarmId].onTickHandler;
       if(Alarm[servicedAlarmId].Mode.isOneShot)
         Alarm[servicedAlarmId].Mode.isEnabled = Alarm[servicedAlarmId].Mode.isAllocated = false;  // free the ID if mode is OnShot
       else  
          Alarm[servicedAlarmId].updateNextTrigger();
       if( TickHandler != NULL) {        
         (*TickHandler)();     // call the handler  
       }
     }
   }
   isServicing = false;
 }
}

AlarmID_t TimeAlarmsClass::create( time_t value, OnTick_t onTickHandler,boolean isAlarm, boolean isOneShot, boolean isEnabled ){  // returns true if has been registerd ok
 if( ! (isAlarm && now() < SECS_PER_YEAR)) // only create alarm ids if the time is at least Jan 1 1971
 {  
for(uint8_t id = 0; id < dtNBR_ALARMS; id++)
   {
       if(Alarm[id].Mode.isAllocated == false)
    {
    // here if there is an Alarm id is available
     Alarm[id].Mode.isAllocated = true;
     Alarm[id].onTickHandler = onTickHandler;
     Alarm[id].Mode.isAlarm = isAlarm;
     Alarm[id].Mode.isOneShot = isOneShot;
     Alarm[id].value = value;
     isEnabled ?  enable(id) : disable(id);  
     return id;  // alarm created ok
    }  
   }
 }
 return dtINVALID_ALARM_ID; // no IDs available or time is invalid
}

// make one instance for the user to use
TimeAlarmsClass Alarm = TimeAlarmsClass() ;



Lets see how we get on with testing this

Michael

viknet

macro previousSunday and isExplicitAlarm  have errors
previoussunday need to be
Code: [Select]
#define previousSunday(_time_)  (max(_time_,SECS_PER_WEEK) - elapsedSecsThisWeek(_time_))  // time at the start of the week for the given time

(you could remove the max, but in this case keep _time_ not SECS_PER_WEEK) ;)


the isExplicitAlarm macro need to be
Code: [Select]
#define isExplicitAlarm (value > (SECS_PER_WEEK+SECS_PER_DAY)) // alarm is at the given time and date     


sorry but I have spent wayyyyyy to much time with your code during the last days ;)


regards



draythomp

There are problems.  I incorporated viknet's macros from post right above because the day of week alarms were firing every second.  After they were incorporated, the DOW alarms are firing on the wrong day and apparently doing a call(0) causing the board to reboot on the second try.  I started the tests at 1/1/1971 to avoid the various problems with the first year.

code used:
Code: [Select]
#include <Time.h>
#include <TimeAlarms.h>

AlarmID_t morningAlarmID, afternoonAlarmID, eveningAlarmID;
AlarmID_t tuesdayAlarmID, saturdayAlarmID;
char buf[200];

void setup()
{
  Serial.begin(57600);
  Serial.println("Initializing...");
  setTime(0,0,0,1,1,1971);
  morningAlarmID   = Alarm.alarmRepeat(8,30,0, onAlarm);
  afternoonAlarmID = Alarm.alarmRepeat(13,25,0,onAlarm);
  eveningAlarmID   = Alarm.alarmRepeat(17,45,0,onAlarm);
  tuesdayAlarmID   = Alarm.alarmRepeat(dowTuesday,12,31,0,onAlarm);
//  saturdayAlarmID  = Alarm.alarmRepeat(dowSaturday,1,45,0,onAlarm);
  pinMode(13, OUTPUT);     
  Serial.println("Begin test...");
}

void onAlarm()
{
  // alarm callback function
  AlarmId id = Alarm. getTriggeredAlarmId();

  if(id == morningAlarmID) {
    if(hour() != 8 || minute() != 30){
      sprintf(buf,"Morning Alarm Failure: Time %d:%d:%d %d/%d/%d", hour(),minute(),second(),month(),day(),year());
      Serial.println(buf);
    }
  }
  else if (id == afternoonAlarmID) { 
    if(hour() != 13 || minute() != 25){
      sprintf(buf,"Afternoon Alarm Failure: Time %d:%d:%d %d/%d/%d", hour(),minute(),second(),month(),day(),year());
      Serial.println(buf);
    }
  }
  else if (id == eveningAlarmID) { 
    if(hour() != 17 || minute() != 45){
      sprintf(buf,"Evening Alarm Failure: Time %d:%d:%d %d/%d/%d", hour(),minute(),second(),month(),day(),year());
      Serial.println(buf);
    }
  } 
  else if (id == tuesdayAlarmID) { 
    if(hour() != 1 || minute() != 45 || weekday() != dowTuesday){
      sprintf(buf,"Tuesday Alarm Failure: Time %s %d:%d:%d %d/%d/%d", dayStr(day()),hour(),minute(),second(),month(),day(),year());
      Serial.println(buf);
    }
  } 
  else if (id == saturdayAlarmID) { 
    if(hour() != 17 || minute() != 45 || weekday() != dowSaturday){
      sprintf(buf,"Saturday Alarm Failure: Time %s %d:%d:%d %d/%d/%d", dayStr(day()),hour(),minute(),second(),month(),day(),year());
      Serial.println(buf);
    }
  } 
  else { 
    Serial.println("Invalid Alarm ID");
    sprintf(buf,"Time %s %d:%d:%d %d/%d/%d", dayStr(day()),hour(),minute(),second(),month(),day(),year());
    Serial.println(buf);
  }
}

int savedMonth = 0;
int savedHour = 0;

void loop()
{
  if(savedHour != hour()){  //just to let me know it's alive
    savedHour = hour();
    digitalWrite(13, digitalRead(13)==HIGH?LOW:HIGH);   // set the LED on
  }
  if(savedMonth != month()){  // how far it's gotten
    savedMonth = month();
    sprintf(buf,"Time %d:%d:%d %d/%d/%d", hour(),minute(),second(),day(),month(),year());
    Serial.println(buf);
  }

  Alarm.delay(0);
  adjustTime(55);
//  sprintf(buf,"Time %d:%d:%d %d/%d/%d", hour(),minute(),second(),day(),month(),year());
//  Serial.println(buf);
 
}


results:
Code: [Select]
Initializing...
Begin test...
Time 0:0:0 1/1/1971
Tuesday Alarm Failure: Time Friday 12:31:18 1/6/1971
Initializing...
Begin test...
Time 0:0:0 1/1/1971
Tuesday Alarm Failure: Time Friday 12:31:18 1/6/1971
Initializing...
Begin test...
Time 0:0:0 1/1/1971
Tuesday Alarm Failure: Time Friday 12:31:18 1/6/1971
Initializing...


The daily alarms seem to be working fine.

viknet

the
Code: [Select]
AlarmID_t TimeAlarmsClass::alarmOnce(const timeDayOfWeek_t DOW, const int H,  const int M,  const int S, OnTick_t onTickHandler){  // as above, with day of week
   unsigned long dayOffset =  ( 7 + DOW - dayOfWeek(now())) %7;
   return create( (dayOffset * SECS_PER_DAY) + AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_ONESHOT );   
}


is also wrong, for alarmonce and alarmrepeat it should be

Code: [Select]
AlarmID_t TimeAlarmsClass::alarmOnce(const timeDayOfWeek_t DOW, const int H,  const int M,  const int S, OnTick_t onTickHandler){  // as above, with day of week
   return create( (DOW * SECS_PER_DAY) + AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_ONESHOT );   
}



regards

draythomp

vicnet's latest changes to DOW alarm don't help much.

tuesdayAlarmID   = Alarm.alarmOnce(dowTuesday,12,31,0,onAlarm);

fires on Thursday 12:31:12 1/5/1971  using the same test program shown above.

And alarmRepeat still cause the arduino to reboot, apparently on the second time it triggers.  Is everyone working from the same source?

draythomp

I think I have the source mixed up somehow.  Regrouping.....

Go Up