Pages: 1 ... 4 5 [6] 7   Go Down
Author Topic: bug in the timealarm library  (Read 7979 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Yes, I can see that the new structure could need an enhanced write that takes the dtAlarmPeriod_t but I am not sure how it would actually be used.

Can you explain the use case for changing the tick handler callback?

And also can you explain the use case where the period type would be  changed?

Well I am building a weekly programmer like this one: except having many output and instead of having display and button it will have a serial connection (connected to a wifi computer handling php pages)

Using serial I need to set and reset alarm to the user desire :=) all handler are identical and I have a structure which give me information for each alarmid (which input to turn on or off) which sprinkler to enable for  how long......

beeing able to get/set the periodicity enable me to change on the fly alarm, at this moment I only can free an alarm and create a new one.

In this case I can only determine the id after the creation so if a user want to modify the periodicity of alarmid=3,
I have to free alarmid=3  then create anotherone having all parameter (zones/durations) identical except periodicity and give the new id to the user.

Concerning the handler, beeing able to get/write would enable me to have two different tye of program with different logic behind, some program would be simpleon/off the other sprinkler program, If I need to display the program to the user I need to have access to the handler information. at this moment as said before, all program do the same and the program has a type with a big if at the beginning.

on a different subject, the fact that now weelky and daily cannot be set the same way I had to put something like this:

Code:
if (day==0)
  Alarm.alarmRepeat(H,M,S,handler);
else
  Alarm.alarmRepeat(day,H,M,S,handler);

when before I was only doing
Code:
Alarm.alarmRepeat(day,H,M,S,handler);

I made the "if" to test your lib but I am not that happy with it, it leads to unnecessarly cumbersome code. I don't know what would be the best, expose the create function to the outside or check if the day is 0 and then create a daily alarm.

best regards

viknet





« Last Edit: July 04, 2011, 03:03:14 am by viknet » Logged

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Viknet, thanks for the information.

In your application where the user can create or change any alarm type and period on any alarm, I  prefer an approach that rather than allowing any value to be written to any timer, restricting the input to the current alarm type. If the user needs to change the alarm type then he would need to delete the old alarm and create a new one of the required type. This allows some detection if invalid commands are received.

The code is open source so there is nothing stopping you changing your copy of the library to suit your application, but I prefer that the public version provides more error checking capability on changes applied to a timer than you want for your app.

As I mentioned in post #47, the new code differs from the playground version in that it returns an error if time values greater a day are given to the method to create daily alarms.
I think having separate  methods for daily and weekly alarms with error checking if the arguments are out of range is worth the extra if statement that may be needed in a sketch.
I would be interested in hearing views on this point from anyone using the current Timealarm library. I suspect that viknet and draythomp are not typical users and they would have no problem adjusting their code to suit, but I wonder if this would be a problem for others?

Michael
Logged

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

michael,

I understand your point of view concerning alarms, but if the write(AlarmID_t ID, time_t value) exist then the write(AlarmID_t ID, time_t value,dtAlarmPeriod_t period) needd to exist, I can understand both way of thinking (also I prefer the last),

But beeing able to read/write the periodicity is a must if you can read/write the value, that was my major fear when you proposed the migration.

Of course I understand the fact that this is opensource and I fully understand that what I need is not necessarly what most programmers need (and don't worry If I need to change the lib I will do it),
your call for comment is the good way of doing it, but seeing that that I was the fisrt to discover the bug concerning Dow, I think that there is not that much people using this feature.
maybe your request for comment, need to be open in another to gain visibility.

regards

Viknet


best regards
 
Logged

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Viknet, rest assured that you will be able to have the functionality you need even if its not in the playground version.

I am keen to keep the playground version  simple and safe. But I recognise that there are useful capabilities for testing or specialist applications that go beyond this scope and  you may have noticed that the alpha code I posted here yesterday had the write, read, readType,  free, count and getNextTrigger as private methods unless a #define was used to expose them.
My intention is to hide all of those functions in the standard distribution and discourage their use with sketches that are indented for public distribution.  I doubt any of these functions are needed for the kinds of applications that the typical arduino user is making.  Experienced  users such as yourself can find the #define to turn these on but I would discourage the publishing of sketches that require these methods as I think its very easy for  inexperienced users to get into trouble  if they play around with this low level alarm logic.
I feel strongly that in the Arduino world its better to provide robust but simple capabilities that address 90% of the likely applications rather than something highly flexible but easy to abuse.
Perhaps there is room for two libraries, one limited to the public methods I have proposed and another with a different name that includes low level functions.
I would be interested to hear comments on this.

Michael
Logged

New River, Arizona
Offline Offline
God Member
*****
Karma: 16
Posts: 906
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Back when I was looking at how to control things around the house I got most of my ideas from devices available at the store.  The more expensive drip water controllers had days of the week and a couple of alarms allotted to each day.  So, a total of 14 alarms that I could program for any start time and duration.  Others had more alarms per day and multiple outputs.  They were all the same type and I couldn't change them to be something else.  The price on these things went from around U$20 for one alarm a day up to well over a hundred for more control.  None of them was remotely programmable nor reported what they were doing.  That's one reason why I started looking for something I could program to do exactly what I want.

I like the simple interface of create it, let it expire and then create another one.  But, that makes it hard to actually use because there is no central place to change it, you have to change the code tor create another one as well.  So, a repeat alarm is good.  Now some folks need to be able to cancel a current one (maybe it rained), but allow it to repeat again day after tomorrow.  Other folks want to do other things that I haven't even suspected were possible.  So, there is probably no end to the possible uses of alarms.  Timers are different in that you set it and it expires and does something.  The idea of a repeat is a good one so the programmer doesn't have to keep resetting it.  The ability to cancel it is good also so failsafe alarms can be created. 

And vicnet has a really good point that he was the one that found the initial problem with the daily alarm.  I would have run into it during a future phase of my work, but I was using a daily alarmOnce and recreating it in the callback because I might not have wanted it to go off next time exactly the same way.  Alarms and timers can be used as program flow control tools as well as turning on relays.  However, this means that few people used AlarmRepeat in a daily fashion and actually tested the code, including me.

This was partly because, at the time I started working on that project, there was practically no mention of TimeAlarm in the playground.  This really surprised me because things like sprinkler timers and Christmas lights would have this as a central component.  Much of the stuff we do around the house for real applications is alarm based.  And, most notably, almost everyone makes an arduino clock as one of their first projects.  I was concerned that the library was out of date and even posted a thread that only got two responses since I could have easily googled for it here.  The responders didn't realize I was trying to make sure that the library was valid and that I had read almost everything on the darn internet about it already. 

Net, the ability to create, modify and delete a daily alarm is necessary.  Additionally, being able to count and list them would make it versatile for people that want to create a sophisticated control system.  For example, if a user wants a menu interface to the alarms they create for something like a sprinkler controller, they will need to know how many there are, when they expire and what kind to put up a nice display.  The guy that did his controller for a dish washer could have easily used this kind of capability.

Please don't misunderstand, I realize that a line has to be drawn somewhere, features can't just be added forever.  But a recurring daily alarm is so darn nice to build into real-life control systems that some complexity is not only warranted, it's downright wonderful.  And, some of us (well, maybe only me) are using these little devices to do real life stuff.  Heck, I'm working on building one into my refrigerator to control it to my tastes instead of what the manufacturer thinks I need. 

So, put the primitive features at the top of the list of capabilities and lower down, the more sophisticated ones.  The people using it will read it and figure out what they need as they build the devices.  Keeping it simple will help the beginner, but people don't stay beginners for very long.  Simple is nice, but we out grow it pretty quickly.

Honestly, I've been keeping myself under control.  Automatic setback thermostats have a weekday vs weekend alarm.  Real expensive ones have holidays built in and are programmable (my power company has these available for a price).  Birthday alarms are really fun.  Monthly alarms are great for paying the power bill.  Odd vs even days make for a nice cycle in some projects.  Alarms that calculate sunrise and sunset are great for outdoor lights.  Should I go on.
Logged

Trying to keep my house under control http://www.desert-home.com/

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

This library has been implemented to suit the typical Arduino user, not an expert programmer.
Here are the basic capabilities supported in the current version for building typical applications:
-   Create  alarms  for a particular time of day that repeat daily or on a particular day of week.
-   Create a one time only alarms for particular time of day or dates
-   Create timers that trigger once only or repeat after an interval that can range from one second to one week
-   Disable and enable any of the created alarms or timers

Here are capabilities, some supported in the old version that could continue to be supported in the standard version or moved into the advanced category (they will probably be included in the standard version unless there is a good argument otherwise)
-   Determine if a given id is an alarm (time of day based) or a timer (duration based)
-   Read the current duration of a timer or trigger time of an alarm
-   Change the value (duration/trigger time) of an alarm/timer
-   Free up an alarm or timer to allow its resources to be used in the creation of another alarm or timer

And these are the advanced capabilities  that are being considered that can be enabled through a hash define:
-   Get the trigger time of the next alarm/timer
-   Get the number of allocated timers
-   Determine if a given alarm/timer id is allocated

My current thinking is that the typical Arduino user of this library will not need the advanced functionality listed above.  If a feature is only useful to programmers interested in making sophisticated control systems then IMO it does not belong in a library targeted for Arduino.  If you disagree with this point of view then perhaps we could discuss this as a matter of philosophy in a new thread in the general discussion section.  But although you have both made the case why you would like these features, I wonder if either of you would say that your needs are representative of typical  Arduino users of this library.

I would prefer not to have two libraries, one that is Arduino friendly, another for sophisticated users, but I would rather do that then add complex capabilities into the arduino library that are not relevant to the majority of arduino users.

As said in my earlier post, you are not going to lose the more advanced capabilities, they are available if you enable them through a hash define. But I do want to avoid the advanced features becoming part of the standard API for this library unless there is a strong use case relevant to more than a handful of users. I believe that almost all of the Arduino applications using this library can be built without the need for any of the advanced capabilities.

The first version of this library was posted here almost three years ago and has had 15768 reads.
The updated posting was here with 12947 reads.
I don't think you will a strong case for the features you are looking for in those threads.  If there are more recent threads requesting the more advanced functionality then please post the links. If functionality is useful for a lot of arduino users  then I will not hesitate to support it.

If it were not for the specialist requirement to test the library and to support you two that have contributed your time to this, I would not be inclined to include those more advanced capabilities at all. I do appreciate the time and effort you have devoted to this topic so please don’t take this post as criticism. Its just that having watched and participated in the Arduino community since 2007 I think simplicity is more important than rich functionality. Although I realize that each person can have a different interpretation of this, I do believe that in a good Arduino API, less is more.

That’s my view, lets see if we get wider feedback.

Michael
Logged

New River, Arizona
Offline Offline
God Member
*****
Karma: 16
Posts: 906
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

A define to turn them on is a reasonable idea IMHO.  Especially since anyone using very many alarms will be editing the max number anyway.  And, a small number of 'default' capabilities that are available without doing anything is reasonable also.  What I guess I'm asking is not to lose any of the features that have come about because someone needed them.  Sheltering them behind a define is a pretty good idea to keep complications down and not lose any inventiveness that has gone before.  That makes it especially easy for someone, once they get past a certain point in the learning curve, to go augment away.

By the way, I still haven't been able to break it.  The only failure so far was when it rolled over 2106 a couple of times.  I may be putting this latest code in something that is running for real just to push the envelope.
Logged

Trying to keep my house under control http://www.desert-home.com/

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

 I have run from 1971 to 2021 without any errors. Are you running the version that increments the time to just below the next trigger or the one that increments by 55 seconds?

Anyway, running in  one of your real apps would be a good test.


Quote
What I guess I'm asking is not to lose any of the features that have come about because someone needed them

What I was trying to say in my previous post is that a few people needing a feature is not it itself a justification for adding complexity to the API.
Logged

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I think simplicity is more important than rich functionality

This is the part I prefer I fully agree with you on this and having a two stage complexity that your propose is a very good idea.

I am already using the readType and free method extensively, and will continue to propose/implement myself other feature If I think I need them.

The new version is perfectly woking on my side without issue.

Going a bit further I have an issue with the
Code:
time_t TimeAlarmsClass::read(AlarmID_t ID)
the fact that is returns 01 when failing is a bit strange because if someone set and alarm to 1 seconde after mignight it wont be able to distinguish an error from a real alarm.

unfortunately, I have no answer other than some hackery (for exemple using 111111111 ie  07 / 09 / 73 @ 7:11:51pm EST) which is probably a lot less used than 1 seconde after midnight (but still not the right way of doing it)

This issue is not a big deal for me and I personaly don't care (because user can only set minute precise alarm) but I wanted to share this with you in case you have a better Idea.

Best regards

Vincent
Logged

New River, Arizona
Offline Offline
God Member
*****
Karma: 16
Posts: 906
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Actually, I've run both.  The tests that exceeded the upper limits were the just before trigger time ones.  I haven't gotten past 2014 on the 55 second tests.   I haven't done any real time testing since it takes so darn long.  I have it running right now in a real device, but the first alarm is not for several hours.  The timers seem to work fine in it though, I have a lot of them running in it.

Regarding complexity, I can't disagree, but it would be sad to lose working changes that other's might need and subsequently have to reinvent.

Y'know, if you were to post a note in the forum under 'project guidance' I bet a bunch of people would chime in for opinions.  Those folks don't seem reluctant to sound off.  However, they can be unkind as well at times.
Logged

Trying to keep my house under control http://www.desert-home.com/

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Going a bit further I have an issue with the
Code:
time_t TimeAlarmsClass::read(AlarmID_t ID)
the fact that is returns 01 when failing is a bit strange because if someone set and alarm to 1 seconde after mignight it wont be able to distinguish an error from a real alarm.

it returns 0l (thats 0 as a long) not 01

In fact, I have already made a change to define the following in TimeAlarms.h
    #define dtINVALID_TIME     0L
my latest version of read returns this constant. Because 0l (0) is not an a valid alarm value it can be distinguished from a real alarm




Logged

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Regarding complexity, I can't disagree, but it would be sad to lose working changes that other's might need and subsequently have to reinvent.


Perhaps we will have to agree to disagree on this point. I believe that keeping things simple is more important than satisfying every demand.


 
Logged

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

my mistake...............


sorry for misreading your code , one more question so: could we then setup a daily midnight alarm (AFAIK time goes from 23:59:59 to 0:0:0)


regards

Viknet
Logged

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

one more question so: could we then setup a daily midnight alarm (AFAIK time goes from 23:59:59 to 0:0:0)

regards

Viknet

The following will set an alarm to trigger every day at midnight:
    Alarm.alarmRepeat(24,0,0, MidnightAlarm);

this will trigger each day at 0:0:0
Logged

London
Offline Offline
Faraday Member
**
Karma: 8
Posts: 6240
Have fun!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

viknet and draythomp, I made some improvements to the TimeAlarms.cpp to remove some code (the new enumeration was duplicating information in some of the old flags in the data structure. The high level logic should be the same as the last code posted but this version will need a sanity check.  You can find the new code here

Note the Time.h header file has changed so copy that into your Time library directory
Copy the TimeAlarms/beta directory into the TimeAlarms library folder.

If you both get this running ok then I will open it up for wider inspection.

Michael
Logged

Pages: 1 ... 4 5 [6] 7   Go Up
Jump to: