bug in the timealarm library

Projects Libraries
41

macro previousSunday and isExplicitAlarm have errors
previoussunday need to be

#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) :wink:

the isExplicitAlarm macro need to be

#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 :wink:

regards

42

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:

#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:

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.

43

the

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

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

44

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?

45

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

46

OK, testing again. Daily alarms seem to work except for Saturday

the call - saturdayAlarmID = Alarm.alarmRepeat(dowSaturday,17,45,0,onAlarm); will fire every time through the loop. I have all the latest changes in place. The next time you post a change, try to put the entire module in the box, my editing can cause errors.

There are a lot more tests to run though. I haven't tried jumping by hours to see how far it will run and I haven't made sure it doesn't miss any. Additionally, tests on a future date, like Feb 4, 2013 need to be tested. However, right now the saturday thing is messing me up.

47

Hi draythomp, good to see you back.
I think there is a better way to fix the DOW problem then the approach we have been taking. Using 7 days plus one in weekly calculations just does not feel right. I am looking into changing the code so that weekly alarms are calculated on periods of 7 days instead of 8 and the adjust for the fact that Sunday is day one by subtracting one from the DOW index on the call to create DOW alarms.

The alarm type macros would then be:
#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

The alarm duration macros would be as in the Time.h post above
#define elapsedSecsThisWeek(time) (elapsedSecsToday(time) + ((dayOfWeek(time)-1) * SECS_PER_DAY)
#define previousSunday(time) (time - elapsedSecsThisWeek(time))
#define nextSunday(time) ( previousSunday(time)+SECS_PER_WEEK)

And the calls to the create methods for alarmOnce and alarmRepeat that take a DOW argument would be modified to use the macros above to subtract one from the DOW index.

Both you and viknet have done a lot of work on this so far so I wanted to get feedback before making these changes.

Michael

edit: on reflection, that approach doesn't work for weekly alarms set for sunday (the values look the same as daily alarms). I am sure we can fix the code along the lines suggested by viknet but perhaps this is the time to replace the implicit calculations for determining the alarm period with an explicit index. There is room to store 16 index values in the AlarmMode_t structure so it would not consume any more RAM. Values could be: DAILY_ALARM, WEEKLY_ALARM and EXPLICIT_ALARM_TIME ( Monthly and Annual alarms could be added later if someone really needs these and wants to code them). This would simplify the code in updateNextTrigger but it is a significant change so I am interested in hearing views on this.

48

Further thoughts on library changes to better support setting alarm periods.

The proposal above uses an explicit variable to hold the alarm period type, this would privately indicate a daily or weekly alarms to the trigger logic and could be extended for biweekly, monthly or annual alarms if needed.
This could be implemented without changing the public API calls but the behaviour of code that calls the create methods would be as follows:

Calls to these existing methods would create daily alarms as now:
alarmRepeat( H, M, S, onTickHandler);
alarmOnce(H, M, S, onTickHandler);

Calls to this method also create daily alarms if the given value is not greater than a day. In the new functionality, values greater than a day would return an error
alarmRepeat(time_t value, onTickHndler);

Calls to this method also create a daily alarm if the given value is not greater than a day (for legacy support, the alarmOnce(H,M.S) method is preferred). In the new functionality values greater than a year will create an alarm at the specific time and date. Values greater than a day and less than a year would return an error. The general rule will be to set the clock to the current time before seting time based alarms, although the logic will work as long as the clock is set at a time later than Jan1 1971.

alarmOnce(time_t value, onTickHandler);

Calls to the following existing methods create alarms that trigger at the given day of week and time as now:
alarmRepeat(timeDayOfWeek_t DOW, H, M, S, onTickHandler);
alarmOnce( timeDayOfWeek_t DOW, H, M, S, onTickHandler);

In the new functionality these would be the only way to create weekly alarms.

In summary, all timer and daily alarm functionality should remain the same. The method using time_t argument for setting day of week alarms would no longer be supported (it will return an error).
The create method will only create time of day or weekly alarms if the clock time is on or after Jan 1 1971. (duration timers will work as now even if the clock is not set)

Does anyone see a problem with any of this?

49

draythomp

sorry to ask you this but could you check this, it is working on my side, and instead of asking you to do modification here are the full file in the three following post.

best regards:

time.h:

/*
  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 
#define previousMidnight(_time_) (( _time_ / SECS_PER_DAY) * SECS_PER_DAY)  // time at the start of the given day
#define previousSunday(_time_)  ((( max(_time_,SECS_PER_WEEK) / SECS_PER_DAY) * SECS_PER_DAY)  - (dayOfWeek(_time_)-1)*SECS_PER_DAY)  // time at the start of the given week
#define nextSunday(_time_) ( previousSunday(time)+SECS_PER_WEEK)  // time at the end of the given week
#define nextMidnight(_time_) ( previousMidnight(_time_)  + SECS_PER_DAY ) // time at the end of the given day 
#define elapsedSecsThisWeek(_time_)  (elapsedSecsToday(_time_) +  (dayOfWeek(_time_) * SECS_PER_DAY) )   

/* 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 */
50

TimeAlarms.h

#ifndef TimeAlarms_h
#define TimeAlarms_h

#include <inttypes.h>

#include "Time.h"

#define dtNBR_ALARMS 8

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 reset(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
                                      // returns  dtINVALID_ALARM_ID if not invoked from within an alarm handler 
};

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 */
51

TimeAlarms.cpp

//(c) Michael Margolis
//This is free software under the terms of the GNU LGPL

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 


//* Alarm Class Constructor

AlarmClass::AlarmClass()
{
  Mode.isAlarm =  Mode.isEnabled = Mode.isOneShot = 0;
  value = nextTrigger = 0;
  onTickHandler = NULL; // prevent a callback until this pointer is explicitly set 
}


//* Private Methods
#define isExplicitAlarm (value > (SECS_PER_WEEK+SECS_PER_DAY)) // alarm is at the given time and date
#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
  
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  // this value is only an hour or a weekday and an hour
      {
        if ( isDailyAlarm) //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 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
        {
          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
  }
}

//* Time Alarms Public Methods

TimeAlarmsClass::TimeAlarmsClass()
{
  isServicing = false;
  for(uint8_t id = 0; id < dtNBR_ALARMS; id++)
     Alarm[id].Mode.isAllocated = false;  // ensure  all Alarms are avialable for allocation  
}

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){   // as above with HMS arguments
   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 
   return create( (DOW * SECS_PER_DAY) + AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_ONESHOT );   
}

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 
   return create( (DOW * SECS_PER_DAY) + AlarmHMS(H,M,S), onTickHandler, IS_ALARM, IS_REPEAT );      
}
   
AlarmID_t TimeAlarmsClass::alarmRepeat(time_t value, OnTick_t onTickHandler){ // trigger daily at the given time
     return create( value, onTickHandler, IS_ALARM, IS_REPEAT );
}

AlarmID_t TimeAlarmsClass::alarmRepeat(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_ALARM, IS_REPEAT );
}



AlarmID_t TimeAlarmsClass::timerOnce(time_t value, OnTick_t onTickHandler){   // trigger once after the given number of seconds 
     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){ // trigger after the given number of seconds continuously
     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) ;  // only enable if value is non zero and a tick handler has been set
    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;
}

void TimeAlarmsClass::reset(AlarmID_t ID)
{
  if(ID < dtNBR_ALARMS && Alarm[ID].Mode.isAllocated)
  {
    Alarm[ID].Mode.isAllocated = false;
    Alarm[ID].onTickHandler = 0;
    Alarm[ID].value = 0;
    Alarm[ID].nextTrigger = 0;
  } 
}

// 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;  
}
 
// 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 
}


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
  }
}


//***********************************************************
//* Private Methods

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  
        }
        else      Serial.println("null 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
  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;
	  }  
  }
  return dtINVALID_ALARM_ID; // no IDs available
}

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

To avoid spinning our wheels on this, I would appreciate a response to post #46 and #47. These changes would make the code in posts #48-#50 redundant.
I value fresh ideas and open discussion on this library but want to avoid creating new issues when we fix the old ones. Using max to prevent overflow still can result in an alarm set at the wrong time. Indeed, any time of day alarms created before the clock is set is likely to result in alarms at the wrong times.

I would appreciate a response to the proposal in posts 46 and 47 before we spend more time testing. The issue raised in this thread has existed for 18 months or so I don’t think we need to rush a fix through.

Michael

53

mem,
sorry but I did not saw your edit on post 46 so I did not really understood the next one I prefered to post my lib as this to enable test by draythomp.

Concerning your proposal

I am sure we can fix the code along the lines suggested by viknet but perhaps this is the time to replace the implicit calculations for determining the alarm period with an explicit index. There is room to store 16 index values in the AlarmMode_t structure so it would not consume any more RAM. Values could be: DAILY_ALARM, WEEKLY_ALARM and EXPLICIT_ALARM_TIME ( Monthly and Annual alarms could be added later if someone really needs these and wants to code them). This would simplify the code in updateNextTrigger but it is a significant change so I am interested in hearing views on this.

I fully agree with this, I thought I was going to do it I spoke about it in another thread:
http://arduino.cc/forum/index.php/topic,63583.15.html
but in order not to break I prefer to first correct the issue with weekly alarm because everything is already done for this (and prefectly working on my side)

following your second post I agree with you but there is the need to expose an alarm.create method that would work for all alarm type (whatever the new parameter we have to supply)

your proposal would also need another alarm.read() method that would reply the value and the frequency (without breaking the old one)

that is a lot of work and test, and I am not sure we have the manpower/testing power/utility for this,

you will also see that I add an alarm.reset() method to fully disable an alarm (this is needed for alarm management).

The project I am working on need alarm to be added/set/read/destroyed using the serial port and that may explain a bit more my needs.

best regards

Viknet

54

vicnet, I'll be glad to give your pieces a try. Get to it in a bit.

mem, first, let me bring up that the usefulness of daily alarms is huge. I'm not sure why people aren't using the heck out of them and why this particular problem hasn't cropped up before except in my case where I reset the daily alarm each time the previous one triggered. But household timers: alarm clocks, thermostats, pool controllers, chicken feeders, light controllers, etc all want daily alarms. So, given that, I think a small change of philosophy is a good thing.

And, frankly, it doesn't make much sense to me to set a daily alarm in any fashion other than alarmRepeat( everyDarnMonday, 10:00AM,etc)
because that reads well in the code except in the instances where the daily time is calculated. In my experience there are extremely few instances where a daily time is calculated, sensors for sunrise, sunset, ph, drainage valves etc, prevail.

Net, having to use the calls with the day of week to set a daily alarm is not a problem, and not being able to use time_t shouldn't bother anyone. I can't imagine how one would use that reasonably anyway.

Also, I don't see a problem having to set the time before using the alarms. It's one silly call with easy to type parameters and one already has to have the time library included. No big deal.

55

vicnet, your modules update worked, at least so far. Saturday no longer fires every time through the loop. I'll run some tests now that include saturday and such to see how it goes.

56

thanks draythomp,

viknet, If the code you posted above is working for you then by all means use it but I am not yet ready to update the playground with this because it does not fully address underflow and does not address potential issues with setting alarms before the clock is set.

I realize that neither of these is likely to be an issue that most people will come across, but the bug I introduced when I added the weekly alarms indicates to me that that the trigger algorithm I implemented in the current version does not make it easy to add more alarm types.

The public alarm creation methods do not need to be changed to support the current alarm types. The private create method would take an additional argument for the alarm period enumeration.

The read method is not documented in the playground or in the distributed readme file so its only used by experienced developers. The existing read method would still return the value but if developers are using the value returned to infer the alarm period then it would be better if an explicit method was provided.

I had not noticed that you had added a method to free up an alarm. This could be added if useful although I would name it clear or free rather than reset.
I have already coded the changes needed to implement the proposal in #46 and #47 so the only work is in testing. If we think it useful to fully address the underflow and ’clock not yet set’ issues as well as make the code easier to enhance then would it not be best to do this now?

Michael

57

I want my count function. Of course, it doesn't need to be called count, but for me and others that will do something similar I think it will be useful.

C'mon, trying to set a timer and having it fail because you have dynamic code that can't track of how many are used kinda sucks. if you create a timer and it returns BAD, what do you do? Hang up in a loop until one comes free to the exclusion of draining the septic tank? The count function could be tied to a bell that rings because you added one too many timers and overdid it. It would also be useful to measure how loaded your device is becoming.

With the clear, reset, whatever routine one could keep assigning timers until one ran out and then clear them all out to get the count, but that's the only way outside the library to tell how many are currently being used. That seems a little silly when it is so easy to do in the library. Looking at the index value doesn't meet with the idea of libraries controlling their data, otherwise, once could just get the id and start poking at the alarm itself.

How many of us have implemented something to tell us how close we've gotten to the end of memory because our arrays and such have put us a hundred bytes from oblivion. Heck, if you're using streams you can run out and have exactly zero indication that there's a problem except for the burst of garbage on the serial output as you over write the stack.

If vicnet hadn't put in the free, he would have wanted a count, especially if he couldn't use the id as an index indicator.

You see, I envision a controller with sixty or seventy timers or alarms. They check on devices periodically, take actions, then retry in a few seconds if it didn't happen, measure ph in the pool and time HCl injectors over a period of days, pan cameras toward the neighbor lady that sunbathes in the afternoon on Tuesdays when the sun is out. It would be very bad if that alarm failed because it ran out.

58

draythomp :wink:

why did'nt you tried the following code ?

int count()
{
  int c=0;
for (int i=0;i<dtNBR_ALARMS;i++)
  if (Alarm.read(i)>=2) c++;
  return c;
}

AFAIK it is perfectly correct, I know it is usefull to know how many slot are left and this is working well, that's the reason why I see no use of putting this inside the library code :wink:

59

draythomp, although I find it difficult to imagine an application that is allocating and freeing so many timers, I certainly don’t want to be the one that causes your septic tank to overflow because you couldn’t get one more timer.

If you really are unable to determine the maximum number of timers you need until the system is running then I can see why you want the count. Am I right in thinking that it would be better to return the number of free timers rather than the count of used timers. Obviously you can derive one from the other using dtNBR_ALARMS, but would you prefer count to return the number used or free?

Michael

60

That's actually a pretty good question, but like you said one is easy from the other. So, it would be a teeny bit better to return the remaining number IMHO. This is not a hard and fast recommendation at all, just a guess. Perhaps it would be better to name it something like countersleft, remaining, not used or something. Count implies used and blows my argument about using the remaining number.

flip a coin???

Just to help you understand how I and one neighbor use the timers dynamically. He has a drip watering system that senses the moisture level in the soil a couple of inches below grade. When it gets dry enough, he turns on the drips, but he wants to limit how long they run because he doesn't want to drain the water tank and have the wives (I share the tank, he's not Mormon) mad because they can't make iced tea. So he turns on the drip and sets a timer for 1 hour from now to turn it off then sets another timer to check tomorrow for dryness. You can't sample the tank because it's buried up to the very top to keep it from getting hot. Maybe an ultra sonic from the lid, but that's another project. But meanwhile, the little arduino is doing a ton of other stuff of a similar nature. The ability to set a timer then just forget about it until it fires leaving the arduino free to do something else makes really cool control systems easy. There's hardware out there that is so complex that you really can't predict what it will do next so, you check it; an alarm that fires daily and then decides when to fire again based on the data is really useful. I don't have a clue why people don't do more of this. Heck a standing light that turns on when a coyote comes in range of the chicken coop then sets a timer to turn off in five minutes is dynamic, multiply that times one light at each corner and you start to see something of the situation. Of course, you want the lights independent, it confused the coyotes more.

Oh, so far, the testing has been working perfectly. I still have a few things to try out, but it has stepped through a few years of daily alarms with them all firing at the right time. I have not checked to see if any of them have missed, just that when they fire they fire on time. More later. And don't worry, if you change something and it needs to be tried, I'll crank up the little piece of code I'm using and do it again.

Besides, I have a vested interest in this because I use this library a lot for drip watering, pool control, blinking lights, etc. The arduino makes an incredible timer when you need one.