smz / Arduino-RTCtime

A time.h compliant library that makes using the DS1307 and DS3231 Real Time Clock modules really simple.
10 stars 8 forks source link

Testing and discussion #1

Open smz opened 7 years ago

smz commented 7 years ago

This issue is now locked and kept open only for announcements

If you find a real issue (a.k.a. "bug"), please open a new issue or push a PR if you have one ready.

@keyeh and @Makuna,

I added examples to my library: I will be really grateful if you could give a look/ride at it and at the whole library.

I only have an Uno and a DS3231, so any test with different hardware will be particularly useful.

Any advice/criticism/correction is wholeheartedly welcome.

Thanks!

keyeh commented 7 years ago

Great work! I'll test this with Arduino Nano and ESP8266.

smz commented 7 years ago

@keyeh: Thanks, Kevin, appreciated!

keyeh commented 7 years ago

Hmm.. I seemed to have misunderstood your library, I thought you were talking about the Time library by Paul Stoffregen, which most timing or scheduling on Arduino is based on.

But I did try your library with an ESP8266 and DS1307 RTC and it appears to be working.

smz commented 7 years ago

Nope, I was referring at the standard time library, the one you get with an #include <time.h> and you can find in your Arduino IDE install folder under \Arduino\hardware\tools\avr\avr\include, BUT, I'am already using other excellent stuff by Paul Stoffregen, and I will surely have a look at his library and see if/how I can "fit" it into my RTC library... (meaning making the necessary adjustment to my library to play it nicely with it)

OT: Are you related with GrowNodes? I see both you and them are from San Louis Obispo, and they forked my library. In case, I would gladly volunteer as a beta-tester of their very interesting equipment... :wink:

smz commented 7 years ago

Oh, yes, sorry, I was forgetting: THANK-YOU for testing, really appreciated!

keyeh commented 7 years ago

I'm using Paul's Time library for NtpClientLib and Chronos, it would be great if there were a drop-in RTC library too, but converting epochs isn't that hard.

And yep, I'm the software guy for Grow Nodes. We're running an early beta test in California soon (a few months hopefully)

smz commented 7 years ago

Ehi, we are in sync: I was just looking at Paul's Time library!

Haven't read the code, but headers files and documentation only.

TBH I'm so far unimpressed. Here are some random considerations:

Will the *nix Epoch based getter/setter will be useful for you? In case, I'll possibly add them, but I have to consider if using two different time bases inside the same library couldn't create confusion to users (and I also have to check if there could be any nasty side effect, but I don't think so...)

Cheers! Sergio

P.S: growing tomatoes for the winter time is possibly the only use I could legally have for your boxes in this country. Too bad...

smz commented 7 years ago

... and if you already have anything that you would like added/changed in my lib, just push a PR and I'll gladly consider it, really.

smz commented 7 years ago

@keyeh, I've added *nix time based setter/getter: SetTimeUX() and GetTimeUX()

SetTimeUX accepts either a time_t or a *time_t, GetTimeUX() of course only returns a time_t

It should be easy for you to use those two functions as the setter/getter in Paul's library: just add the glue...

I hope this can help you...

P.S.: the DS1307 version is untested (but it should reasonably work...)

keyeh commented 7 years ago

I'm a Javascript/Ruby developer and I'm new to C++. Thanks for your work on this library.

I believe most beginners like myself will be using Paul's Time library as that's what seems to be the "official one for Arduino" -- it's widely used and the first result on Google. I'm calling it TimeLib from now on.

Having a getter/setter that is compatible with TimeLib is great for beginners. It's a good idea to document that your library is compatible with, but does not require TimeLib:

Set RTC time from TimeLib: Rtc.SetTimeUX(now()); Set TimeLib time from RTC: setTime(Rtc.GetTimeUX());

keyeh commented 7 years ago

I'll let you know when I test DS1307 with SetTimeUX() and GetTimeUX() on ESP8266. May be a while before I do.

smz commented 7 years ago

Thanks, @keyeh.

I'm still undecided if I'll leave the UX setters/getters: they could generate confusion. Think of this:

#define MY_TIMEZONE_IN_SECONDS (-8 * ONE_HOUR)  // California
set_zone(MY_TIMEZONE_IN_SECONDS);
time_t now = Rtc.GetTime();
Serial.print(ctime(&now));

The above will correctly print your time, assuming your RTC is correctly set to run in UTC time as all well behaving RTCs should, but:

#define MY_TIMEZONE_IN_SECONDS (-8 * ONE_HOUR)  // California
set_zone(MY_TIMEZONE_IN_SECONDS);
time_t now = Rtc.GetTimeUX();
Serial.print(ctime(&now));

while formally correct will print a totally wrong time.

I think that sticking to the standard C library (or, to be more precise, its AVR implementation) offers great benefits (at least to me) compared to what is usually used in the Arduino ecosystem, one of them being the possibility to account for different Time Zones and DST (yes, the DST implementation is incomplete, but I think I can find solutions for that and in any case there is already support for US and Europe DST).

What I dislike in the AVR C time library is its Epoch, but this is a far lesser issue to me...

smz commented 7 years ago

An historical interlude

First thing, a clarification: I'm totally new to the Arduino world, I've put my hands on my first Arduino a couple of weeks ago, just because my Honeywell programmable home thermostat died, I didn't want to throw away money on a new one (and possibly see it broken again in less than an year), and decided to build my own one using an Arduino (and possibly control it from my home intranet web server).

My thermostat needed to keep the time, I found that my Arduino IDE 1.8.1 does have an implementation of the Standard C Time Library with which I'm familiar from the oooold times, but lacked the "drivers" to read from my RTC hardware (a DS3231). As they say, the rest is history: I found the Rtc library, by @Makuna, modified it, and here we are.

Only at a second time, thanks to @keyeh, I discovered that most programmers in the Arduino world don't use the standard C Time library, but a different one, the Time library, by @PaulStoffregen and @michaelmargolis.

Now, what I'm writing might be obvious for those who are using the Arduino since a long time, but I was puzzled why the C Time Library wasn't used, so I did my research and I think I found where the reasons lies:

Arduino IDE does include the Atmel AVR GNU Toolchain, but that toolchain didn't include the C Time library until version 3.5.2 which seems to have been released on 2016-04-19 and Arduino IDE adopted as the next 3.5.3 version with its 1.6.10 version of 2016-07-29 (see the Release Notes) [Edit: @feilipu clarified here below that on its hand the AVR Toolchain uses the AVR Libc which didn't include the time functions until v2.0.0., first included in Atmel 3.5.2]

This explains, I think, why most people here do not use the standard C Time library, but different libraries, with different API.

As I've just came in touch with the Arduino ecosystem and I have previous experience with the standard C Time library, I honestly feel more comfortable sticking with it, but I perfectly understand that the same is true also for those who do not use it and instead prefer using what they are accustomed with.

It seems that the Time library found in the Atmel toolchain was originally developed by Michael Duane and is now maintained here in GitHub as Arduino_RTC_Library by Phillip Stevens, @feilipu. See also its entry on platformio.org.

It seems to me that the latest and only release of the above pre-dates the one included by Atmel/Arduino, but on the other hand there are several new modifications in the master branch.

What still leaves me a little bitter taste in the mouth is the different Epoch adopted (when compared to the *nix implementation), but on the other hand this will move the Year 2038 problem forward to 2108 (if my math is correct...)

My apologies for "pinging" the persons I cited above, but I think it is important to offer them the right to correct me if I wrote something wrong.

Regards,

Sergio

feilipu commented 7 years ago

AVR Libc did not include Std C time until v2.0.0. For ages (years) a pre release version sat around in various repos.

I needed a Std C version and therefore made my library based on the prerelease code. The only difference to Std C is that I've made a specific implementation of how it is driven by a RTC timer using a crystal available in Arduino.

As soon as avr-Libc v2.0.0 is published in the Debian / Ubuntu archives, I'll be moving my library to be just the physical implementation for the library.

I won't be using any time libraries that are not Std C compatible.

smz commented 7 years ago

Thanks for clarifying @feilipu and thanks for your work: you've just been starred!

You says that Std C time was included with AVR code v.2.0.0, but I can't find it even in the 3.5.1 version I linked above: are we talking different things?

smz commented 7 years ago

... and as I've the opportunity to ask, in your opinion is there any important reason to use the lib as it is in your master branch, or can I stay with the one that comes with latest Arduino IDE?

PaulStoffregen commented 7 years ago

The Time library by Michael Margolis, which I seem to have inherited for maintenance, goes back to the very early days of Arduino. At least for the foreseeable future, it's in deep maintenance mode with the primary goal of preserving compatibility for the many programs which have used it. The API differs from standard C or posix, and it will remain that way. No changes that break existing programs will be merged.

feilipu commented 7 years ago

Yes. They are different things.

AVR Libc is at v2.0.0. It is the basis for all things AVR in open source. Arduino uses it. Atmel supports it, although they also have their own libraries.

The Atmel tool chain v3.5.1 doesn't have AVR Libc v2.0.0 support.

smz commented 7 years ago

Thanks to both of you, guys! 👍

feilipu commented 7 years ago

I would use the AVR Libc version in the Arduino IDE, as it is the standard implementation.

You might like to diff my changes, as this will help if you want to use the RTC Timer present (usually Timer 2) in most AVR ATmega devices.

smz commented 7 years ago

My apologies: there was a BAD bug affecting the DS1307 as the month was read without decrementing it, and so the date read was always one month ahead. Nasty things could happen in December, I guess, so upgrade ASAP to version 1.0.2, if you're using this lib with a DS1307

keyeh commented 7 years ago

Keep in mind, Arduino compatible code can run on non-AVR processors. Try not to do anything AVR specific with this library.

smz commented 7 years ago

Hello @keyeh! Thanks for the hint: that's something I didn't thought of, TBH. I haven't any non-AVR Arduino at hand now: do you see anything wrong in the code right now?

Makuna commented 7 years ago

I didn't spot anything that was AVR specific. My library (the code this was based on) works on several other hardware platforms including the esp8266.

smz commented 7 years ago

I've just published the 1.0.3 fix release bringing forward two small changes from @makuna upstream work. Thanks again, @makuna!

smz commented 6 years ago

I'm locking down this "issue" and keep it open only for announcements. For any new issue, please open... a new issue! :smiley: