nerves-time / nerves_time

Keep time in sync on Nerves devices
Other
23 stars 13 forks source link

adds support for registering time sync handlers, for example RTC #47

Closed dognotdog closed 4 years ago

dognotdog commented 5 years ago

This adds a mechanism for registering handlers at runtime for time sync that get called whenever time is updated through NTP. It is very simplistic, as it only allows adding, but not removing handlers, in the spirit of letting things crash in the unlikely case a registered handler dies and its handler function crashes.

Registering the callback in the application_started Shoehorn handler allows this to persist even when nerves_time is restarted intentionally or through crashes.

I've tested it with an RTC module that talks to an I2C RTC chip on rpi0.

A more holistic approach could integrate an RTC better, as right now nerves_time comes up first, and reads the timestamp file to set the system time, and then the RTC comes up with the main app and sets system time again.

See #3

fhunleth commented 5 years ago

@ConnorRigby - FYI

ConnorRigby commented 5 years ago

Wow thanks for this! it looks really good. One thing that i think might be useful is supplying a handler at compile time via config.exs. Reason being i feel like most folks will know ahead of time that they have an RTC module attached

ConnorRigby commented 5 years ago

one final thought is this doesn't seem to have support for loading time from a rtc device. Instead of an anon function, might it be easier to register a module that implements a behaviour?

dognotdog commented 5 years ago

@ConnorRigby using config.exs was my first approach, and it is easy enough to do that, but I don't know enough about elixir mechanisms for event dispatching: just a simple module reference with some random handler function didn't feel right, and I had a dumb side-effect crash if that handler was called before the GenServer for the RTC was up, whereas with it being set in application_start sidesteps that issue with both eyes closed (as the initial sync from file is ignored, and a real NTP sync doesn't happen until much later when everything's up). If there's a better pattern I'm all for it.

In my setup, the RTC just calls date to set the time when it notices systime being off on boot or when manually triggered, and ntpd doesn't seem to care much about that, so further integration might not be necessary in the spirit of keeping the interface minimal.

fhunleth commented 4 years ago

@dognotdog Sorry for the really long delay in getting hardware RTC support included. Connor PR'd a simple implementation and we'll be iterating from there.