nerves-time / nerves_time

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

WIP: Move storage and retrieval of timestamp into handler #10

Closed jmerriweather closed 4 years ago

jmerriweather commented 6 years ago

This PR provides support to make handlers for the storage and retrieval of the timestamp, by default it still uses file metadata. Below i've made an example of using RTC.

Could we change nerves_time to use a Supervisor instead of an Application? This would let us pass in configuration when we add the supervisor it to our supervision tree. If we did that would it mess up shoehorn, as nerves_time wouldn't have a :mod anymore?

My test handler using rtc_ds3231 is as follows:

defmodule Nerves.Time.Ds3231 do
  use Nerves.Time.TimestampHandler

  alias ElixirALE.I2C

  require Logger

  defmodule State do
    @enforce_keys [:i2c_device, :i2c_address]
    defstruct [:i2c_device, :i2c_address, :device_pid]
  end

  @impl true
  def init() do
    i2c_device = Application.get_env(:rtc_ds3231, :i2c_device, nil)
    i2c_address = Application.get_env(:rtc_ds3231, :i2c_address, nil)
    {:ok, pid} = I2C.start_link(i2c_device, i2c_address)
    {:ok, %State{i2c_device: i2c_device, i2c_address: i2c_address, device_pid: pid}}
  end

  @impl true
  def update(state) do
    Logger.info("TODO: Update RTC Time")
  end

  @impl true
  def time(%{device_pid: pid}) do
    with rtc_bytes <- RtcDs3231.bytes_from_rtc(pid),
         erl_date <- RtcDs3231.rtcbytes_to_erl(rtc_bytes),
         {:ok, parsed_datetime} <- NaiveDateTime.from_erl(erl_date) do
      parsed_datetime
    else
      error ->
        Logger.error("#{inspect __MODULE__} failed to request current datetime from RTC: #{inspect error}")
    end
  end
end
fhunleth commented 6 years ago

nerves_time is a natural singleton, so I prefer it being an OTP application so that users don't have to remember to add it to their own supervision trees.

I have a laundry list of improvements to nerves_time that I really, really thought that I'd get to by now, and it makes me sad that I haven't since they'd help in your efforts. I think that you started down the right route of making the part that persists the time to be configurable. Let's chat to see if we can work together to make RTC integration happen.

jmerriweather commented 6 years ago

yeah agreed, moving to a Supervisor would make it very confusing for new people.

I'd love to work on improving nerves_time

fhunleth commented 4 years ago

Closing since merging #51.

fhunleth commented 4 years ago

@jmerriweather Sorry for the really long delay in getting hardware RTC support included. The plan is to put hardware RTC implementations in separate repositories and use the behaviour interface for getting nerves_time to use them.

jmerriweather commented 4 years ago

@fhunleth no worries, #51 looks good 👍