ArduinoCore-API icon indicating copy to clipboard operation
ArduinoCore-API copied to clipboard

Rename RealtimeClock to ArduinoTime

Open mbanzi opened this issue 8 years ago • 7 comments

The RealtimeClock class is actually managing time so it should be called ArduinoTime

we could also provide a software only implementation which can be useful even if only based on millis() (there are a number of use cases where one needs to manage time, maybe syncronising via NTP every etc)

mbanzi avatar Mar 26 '16 16:03 mbanzi

In about a week, I should be able to work on this time stuff. At the moment I'm wrapping up a new shield product release with an IMU and other stuff, so it's demanding most of attention right now. Hopefully this time API isn't highly urgent?

However, here's a lengthy message about the Time library, originally by Michael Margolis, which I somehow ended up sort-of maintaining.

Michael's design does exactly what you're suggesting, keeping time with millis() and allowing occasional sync to external sources. NTP over the internet and GPS are the 2 common cases.

A recurring problem with the Time library is confusion when syncing to a RTC. Michael and I have talked about this in person at Maker Faire at least twice. I've been tentatively planning to expand the Time library to distinguish between 2 different types of time sources. I've been called them "online" and "external", but perhaps other names would be better. When an "online" device like a RTC is present, keeping time separately with millis and syncing them is terribly confusing. But people do easily understand and expect this when they know the source is "external", like NTP over the internet. My main point is there's 2 different types of time sources and people do seem to intuitively understand the difference.

Atomic access is a real issue with the current Time library. The simple functions year(), month(), day(), hour(), minute(), second() are convenient, but when called in sequence, especially when mixed with code that does I2C or SPI communication to update a display, it's possible to get the minute when seconds is 59, and then later get the second after it's rolled over to zero... for a 1 minute display error! Theoretically this error could happen between any of the functions, but a 1 hour error is 60X less likely, 1 day 24X less common, etc. Often I've considered adding buffer and millisecond timestamp inside the Time library, to remember the most recent 32 bit time returned by these functions and intentionally return the "old" time under certain circumstances, like detecting if all 6 or either group of 3 have or haven't been read, and not "too many" milliseconds have elapsed to make the buffer "too old".

I know the simplicity of these functions is very compelling, but for a permanent Arduino API, perhaps we should think about ways to solve the atomic access problem, yet still end up with something simple and easy to learn & use? Maybe a C++ object of some sort?

Another issue Michael raised when we've talked about "online" versus "external" time sources the inefficient nature of I2C communication to many RTC chips. Solving the atomic access issue would also go a long way towards solving the excessive communication issue if each call to year(), month(), day(), hour(), minute(), second() accessed a single atomically acquired time, rather than causing 6 separate I2C communications.

Previously I was concerned about the use of short, common words as functions in the global namespace. But I just did a few quick tests. It doesn't seem to be such a big problem.

One final thought. On Teensy, I've been using this in my platform.txt file in the linker step:

-Wl,--defsym=__rtc_localtime={extra.time.local}

This embeds the build time into the hex file in a way that's always updated right before upload, even if none of the files needed to be recompiled. Then startup code can detect if a RTC isn't initialized and use this number to automatically set it to the timestamp when the code was compiled. In the common case where someone casually uploads an Arduino sketch, it allows their RTC or software-based timekeeping to automatically have the correct time, at least within the seconds for upload and startup. When they power cycle (without a battery-backed RTC), of course the time gets reset to when they built the code, but at least in those initial experiences while uploading, things "just work" with the time set fairly close to accurate. Might be worthwhile to add this to Arduino's platform.txt files. Like other linker symbols, it only gets allocated to bytes in the HEX file if actual code uses it.

Whew, long message. Hopefully some of this helps? In a week or so, I can actually work with the code...

PaulStoffregen avatar Mar 26 '16 17:03 PaulStoffregen

The RealtimeClock class is actually managing time so it should be called ArduinoTime

this brings a more wider question: are we going to provide the time API (and also the other API listed in this repository) as separate libraries? or are we going to include them in the Arduino Core?

If we go with the library path, then I agree, it should be renamed ArduinoTime. Otherwise, if we are putting it inside the core, I'd avoid the Arduino prefix, that leads to Time, but Time.h conflicts with time.h from the standard C lib, that's the reason why I opted for RealTimeClock.

cmaglie avatar Mar 29 '16 14:03 cmaglie

Whew, long message. Hopefully some of this helps? In a week or so, I can actually work with the code...

Paul, thanks for taking the time to write this message that contains really valuable information.

A recurring problem with the Time library is confusion when syncing to a RTC. Michael and I have talked about this in person at Maker Faire at least twice. I've been tentatively planning to expand the Time library to distinguish between 2 different types of time sources. I've been called them "online" and "external", but perhaps other names would be better. When an "online" device like a RTC is present, keeping time separately with millis and syncing them is terribly confusing. But people do easily understand and expect this when they know the source is "external", like NTP over the internet. My main point is there's 2 different types of time sources and people do seem to intuitively understand the difference.

If I undestand correctly in your proposal the current time is kept by the CPU via software, using millis() or other techniques, and it's synced (more or less often) with a time-service that can be an RTC or a NTP server or any other precise time-reference. The time functions (seconds, minutes, etc.) instead will read the time from the CPU. Is that right?

At the moment the Time API in this repository is structured in a slightly different way: the hour, minute, seconds functions uses the RTC to get the current time (a software-RTC that emulates a real RTC can surely be added for boards that doesn't have one). This means that there is no synchronization involved, and the user is expected to set the correct time by himself. The RTC (whatever it is, internal, I²C, software...) must implement the following interface:

class RealTimeClock {
  virtual unsigned long getTime() = 0;
  virtual bool setTime(unsigned long t) = 0;
};

and the time functions will performs all the calculations after reading the current timestamp from the SystemRTC:

extern RealTimeClock *arduinoSystemRTC;

The synchronization with a time-service is, IMHO, another different thing: what you call "external" source is actually a time-service that provides the real current time. This is deeply different from an RTC because the time-service will always provide you the real current time by means of other sources that are usually slow (GPS/Network/DFC77/a human being that enters the current time...). An RTC instead, if not set, is not able to provide the current time.

cmaglie avatar Mar 29 '16 15:03 cmaglie

Atomic access is a real issue with the current Time library. The simple functions year(), month(), day(), hour(), minute(), second() are convenient, but when called in sequence, especially when mixed with code that does I2C or SPI communication to update a display, it's possible to get the minute when seconds is 59, and then later get the second after it's rolled over to zero... for a 1 minute display error! Theoretically this error could happen between any of the functions, but a 1 hour error is 60X less likely, 1 day 24X less common, etc. Often I've considered adding buffer and millisecond timestamp inside the Time library, to remember the most recent 32 bit time returned by these functions and intentionally return the "old" time under certain circumstances, like detecting if all 6 or either group of 3 have or haven't been read, and not "too many" milliseconds have elapsed to make the buffer "too old".

Caching the last timestamp for a fixed amount of time looks like a reasonable solution to me and the implementation seems also straightforward. On the documentation we could state that the time is updated, say, every 100ms and that all the time elements should be read altogether within that time frame to avoid inconsistencies.

cmaglie avatar Mar 29 '16 15:03 cmaglie

Some random thoughts... Paul and Cristian, I like what you said about the distinction between a time-source (e.g. GPS or NTP or something that provides the current time) and a real-time clock (something that can count time accurately but doesn't know the current time). It seems like this API requires both. Maybe there should be a separate TimeSource interface (with a single getTime() or now() function)? Those time sources could even automatically register themselves with the core Arduino time functions; e.g. FooGPS.begin() could call setSyncProvider(this). That would configure the core to occasionally update the system RTC's time from that external time source.

On Tue, Mar 29, 2016 at 8:32 AM, Cristian Maglie [email protected] wrote:

Whew, long message. Hopefully some of this helps? In a week or so, I can actually work with the code...

Paul, thanks for taking the time to write this message that contains really valuable information.

A recurring problem with the Time library is confusion when syncing to a RTC. Michael and I have talked about this in person at Maker Faire at least twice. I've been tentatively planning to expand the Time library to distinguish between 2 different types of time sources. I've been called them "online" and "external", but perhaps other names would be better. When an "online" device like a RTC is present, keeping time separately with millis and syncing them is terribly confusing. But people do easily understand and expect this when they know the source is "external", like NTP over the internet. My main point is there's 2 different types of time sources and people do seem to intuitively understand the difference.

If I undestand correctly in your proposal the current time is kept by the CPU via software, using millis() or other techniques, and it's synced (more or less often) with a time-service that can be an RTC or a NTP server or any other precise time-reference. The time functions (seconds, minutes, etc.) instead will read the time from the CPU. Is that right?

At the moment the Time API in this repository is structured in a slightly different way: the hour, minute, seconds functions uses the RTC to get the current time (a software-RTC that emulates a real RTC can surely be added for boards that doesn't have one). This means that there is no synchronization involved, and the user is expected to set the correct time by himself. The RTC (whatever it is, internal, I²C, software...) must implement the following interface:

class RealTimeClock { virtual unsigned long getTime() = 0; virtual bool setTime(unsigned long t) = 0; };

and the time functions will performs all the calculations after reading the current timestamp from the SystemRTC:

extern RealTimeClock *arduinoSystemRTC;

The synchronization with a time-service is, IMHO, another different thing: what you call "external" source is actually a time-service that provides the real current time. This is deeply different from an RTC because the time-service will always provide you the real current time by means of other sources that are usually slow (GPS/Network/DFC77/a human being that enters the current time...). An RTC instead, if not set, is not able to provide the current time.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/arduino/ArduinoCore-API/issues/3#issuecomment-202956683

damellis avatar Mar 29 '16 17:03 damellis

As I mentioned elsewhere I see ArduinoTime as the high level arduino time API

then you can subclass it for the different implementations.. by default on UNO it’s a software only implementation (which can be synched as you mention)

on other platform it’s implemented using the on board RTC if the platform has one (ZERO, CURIE, DUE and Linux on libArduino)

what is important it that when the code starts the user has a time object that gives access to time and date.

m

On 29 Mar 2016, at 13:11, David A. Mellis [email protected] wrote:

Some random thoughts... Paul and Cristian, I like what you said about the distinction between a time-source (e.g. GPS or NTP or something that provides the current time) and a real-time clock (something that can count time accurately but doesn't know the current time). It seems like this API requires both. Maybe there should be a separate TimeSource interface (with a single getTime() or now() function)? Those time sources could even automatically register themselves with the core Arduino time functions; e.g. FooGPS.begin() could call setSyncProvider(this). That would configure the core to occasionally update the system RTC's time from that external time source.

On Tue, Mar 29, 2016 at 8:32 AM, Cristian Maglie [email protected] wrote:

Whew, long message. Hopefully some of this helps? In a week or so, I can actually work with the code...

Paul, thanks for taking the time to write this message that contains really valuable information.

A recurring problem with the Time library is confusion when syncing to a RTC. Michael and I have talked about this in person at Maker Faire at least twice. I've been tentatively planning to expand the Time library to distinguish between 2 different types of time sources. I've been called them "online" and "external", but perhaps other names would be better. When an "online" device like a RTC is present, keeping time separately with millis and syncing them is terribly confusing. But people do easily understand and expect this when they know the source is "external", like NTP over the internet. My main point is there's 2 different types of time sources and people do seem to intuitively understand the difference.

If I undestand correctly in your proposal the current time is kept by the CPU via software, using millis() or other techniques, and it's synced (more or less often) with a time-service that can be an RTC or a NTP server or any other precise time-reference. The time functions (seconds, minutes, etc.) instead will read the time from the CPU. Is that right?

At the moment the Time API in this repository is structured in a slightly different way: the hour, minute, seconds functions uses the RTC to get the current time (a software-RTC that emulates a real RTC can surely be added for boards that doesn't have one). This means that there is no synchronization involved, and the user is expected to set the correct time by himself. The RTC (whatever it is, internal, I²C, software...) must implement the following interface:

class RealTimeClock { virtual unsigned long getTime() = 0; virtual bool setTime(unsigned long t) = 0; };

and the time functions will performs all the calculations after reading the current timestamp from the SystemRTC:

extern RealTimeClock *arduinoSystemRTC;

The synchronization with a time-service is, IMHO, another different thing: what you call "external" source is actually a time-service that provides the real current time. This is deeply different from an RTC because the time-service will always provide you the real current time by means of other sources that are usually slow (GPS/Network/DFC77/a human being that enters the current time...). An RTC instead, if not set, is not able to provide the current time.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/arduino/ArduinoCore-API/issues/3#issuecomment-202956683

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/arduino/ArduinoCore-API/issues/3#issuecomment-203004221

mbanzi avatar Mar 29 '16 17:03 mbanzi

Ok I've had some time to tinker on this one, hope that the latest changes reflect better how I would like to see this API evolve and that those changes matches your expectations too! :-)

  1. I've added a class called TimeProvider https://github.com/arduino/ArduinoCore-API/commit/bde41a3517675ab48f32eff135b5c7b28f6ea3bb

we now have two classes: RealTimeClock and TimeProvider.

RealTimeClock represents a plain "clock": you set the time and, once set, it tells you the current time with a fairly reasonable precision. This is the typical RTC (hardware or software) that is able to keep the current time, with a reasonable precision, once you have set it up and until you keep it powered.

TimeProvider represents a "current time source": it's a provider that you can query to ask for the real current time. Typical examples are NTP, GPS, DFC77 and every source that is synchronized with a very precise reference clock.

The use case I have in mind is to use TimeProvider (when available) to query the real current time to initialize the RealTimeClock. If a TimeProvider is not available, the RealTimeClock can still be used but requires the user to manually enter the current time.

  1. I've added a SoftwareRTC implementation. https://github.com/arduino/ArduinoCore-API/commit/58dae9e92e0bab2bc3057177f398f2c973c7273d

This is an untested draft, it's just to see how well it fits. The idea is that boards or CPU that doesn't have an hardware RTC may create a SoftwareRTC instance in their variant file to make it available to users. We should also figure out a way to let the compiler optimize the build and remove the SoftwareRTC instance when not used.

cmaglie avatar Apr 08 '16 14:04 cmaglie