Use the new firmware update status messages

These new accepted Hub messages hook deeper into the updating lifecycle, as well as fixing issues where new firmware aren't sent to devices when an update is ignored or encounters errors.

This also moves the responsibility for rescheduling the update to Hub, which will resend the update shortly after the date sent by Link.

lib/nerves_hub_link.ex

@@ -55,14 +55,14 @@ defmodule NervesHubLink do
   @doc """
   Send update progress percentage for display in web
   """
-  @spec send_update_progress(non_neg_integer()) :: :ok
-  defdelegate send_update_progress(progress), to: Socket
+  @spec send_firmware_update_progress(non_neg_integer()) :: :ok
+  defdelegate send_firmware_update_progress(progress), to: Socket
 
   @doc """
-  Send an update status to web
+  Send a firmware status update status to hub
   """
-  @spec send_update_status(String.t() | atom()) :: :ok
-  defdelegate send_update_status(status), to: Socket
+  @spec send_firmware_update_status(status :: atom(), payload :: map()) :: :ok
+  defdelegate send_firmware_update_status(status, payload \\ %{}), to: Socket
 
   @doc """
   Send a file to the connected console

lib/nerves_hub_link/client.ex

@@ -174,12 +174,13 @@ defmodule NervesHubLink.Client do
     # TODO: nasty side effects here. Consider moving somewhere else
     case data do
       {:progress, percent} ->
-        NervesHubLink.send_update_progress(percent)
+        NervesHubLink.send_firmware_update_progress(percent)
 
       {:error, _, message} ->
-        NervesHubLink.send_update_status("fwup error #{message}")
+        NervesHubLink.send_firmware_update_status(:error, %{reason: message})
 
       {:ok, 0, _message} ->
+        NervesHubLink.send_firmware_update_status(:applying)
         initiate_reboot()
 
       _ ->

lib/nerves_hub_link/socket.ex

@@ -29,14 +29,17 @@ defmodule NervesHubLink.Socket do
     GenServer.cast(__MODULE__, :reconnect)
   end
 
-  @spec send_update_progress(non_neg_integer()) :: :ok
-  def send_update_progress(progress) do
-    GenServer.cast(__MODULE__, {:send_update_progress, progress})
+  @spec send_firmware_update_progress(percent_progress :: non_neg_integer()) :: :ok
+  def send_firmware_update_progress(percent_progress) do
+    GenServer.cast(
+      __MODULE__,
+      {:send_firmware_update_status, :progress, %{percent: percent_progress}}
+    )
   end
 
-  @spec send_update_status(String.t()) :: :ok
-  def send_update_status(status) do
-    GenServer.cast(__MODULE__, {:send_update_status, status})
+  @spec send_firmware_update_status(status :: atom(), payload :: map()) :: :ok
+  def send_firmware_update_status(status, payload \\ %{}) do
+    GenServer.cast(__MODULE__, {:send_firmware_update_status, status, payload})
   end
 
   @spec check_connection(atom()) :: boolean()
@@ -270,13 +273,39 @@ defmodule NervesHubLink.Socket do
     {:noreply, disconnect(socket)}
   end
 
-  def handle_cast({:send_update_progress, progress}, socket) do
-    _ = push(socket, @device_topic, "fwup_progress", %{value: progress})
+  def handle_cast({:send_firmware_update_progress, :progress, progress}, socket) do
+    _ =
+      push(socket, @device_topic, "firmware_update_status", %{
+        status: "progress",
+        percent: progress
+      })
+
+    {:noreply, socket}
+  end
+
+  def handle_cast({:send_firmware_update_status, :applying, payload}, socket) do
+    {:ok, push_ref} =
+      push(
+        socket,
+        @device_topic,
+        "firmware_update_status",
+        Map.merge(payload, %{status: "applying"})
+      )
+
+    _ = await_reply(push_ref)
+
     {:noreply, socket}
   end
 
-  def handle_cast({:send_update_status, status}, socket) do
-    _ = push(socket, @device_topic, "status_update", %{status: status})
+  def handle_cast({:send_firmware_update_status, status, payload}, socket) do
+    _ =
+      push(
+        socket,
+        @device_topic,
+        "firmware_update_status",
+        Map.merge(payload, %{status: to_string(status)})
+      )
+
     {:noreply, socket}
   end
 
@@ -366,7 +395,7 @@ defmodule NervesHubLink.Socket do
           into: %{},
           do: {name, to_string(ver)}
 
-    {:ok, join(socket, "extensions", available_extensions)}
+    {:ok, join(socket, @extensions_topic, available_extensions)}
   end
 
   ##

lib/nerves_hub_link/update_manager.ex

@@ -23,21 +23,26 @@ defmodule NervesHubLink.UpdateManager do
 
     @type status ::
             :idle
-            | {:fwup_error, String.t()}
-            | :update_rescheduled
             | {:updating, integer()}
+            | :applying
+
+    @type previous_status ::
+            :ignored
+            | :rescheduled
+            | :successful
+            | {:error, String.t()}
 
     @type t :: %__MODULE__{
             status: status(),
-            update_reschedule_timer: nil | :timer.tref(),
+            previous_status: previous_status(),
             download: nil | GenServer.server(),
             fwup: nil | GenServer.server(),
             fwup_config: FwupConfig.t(),
             update_info: nil | UpdateInfo.t()
           }
 
     defstruct status: :idle,
-              update_reschedule_timer: nil,
+              previous_status: nil,
               fwup: nil,
               download: nil,
               fwup_config: nil,
@@ -110,8 +115,8 @@ defmodule NervesHubLink.UpdateManager do
         _from,
         %State{} = state
       ) do
-    state = maybe_update_firmware(update, fwup_public_keys, state)
-    {:reply, state.status, state}
+    {result, state} = maybe_update_firmware(update, fwup_public_keys, state)
+    {:reply, result, state}
   end
 
   def handle_call(:currently_downloading_uuid, _from, %State{update_info: nil} = state) do
@@ -143,38 +148,38 @@ defmodule NervesHubLink.UpdateManager do
     {:reply, :ok, state}
   end
 
-  @impl GenServer
-  def handle_info({:update_reschedule, response, fwup_public_keys}, state) do
-    {:noreply,
-     maybe_update_firmware(response, fwup_public_keys, %State{
-       state
-       | update_reschedule_timer: nil
-     })}
-  end
-
   # messages from FWUP
+  @impl GenServer
   def handle_info({:fwup, message}, state) do
     _ = state.fwup_config.handle_fwup_message.(message)
 
     case message do
       {:ok, 0, _message} ->
         Logger.info("[NervesHubLink] FWUP Finished")
         :alarm_handler.clear_alarm(NervesHubLink.UpdateInProgress)
-        {:noreply, %State{state | fwup: nil, update_info: nil, status: :idle}}
+
+        {:noreply,
+         %State{
+           state
+           | fwup: nil,
+             update_info: nil,
+             status: :applying,
+             previous_status: :successful
+         }}
 
       {:progress, percent} ->
         {:noreply, %State{state | status: {:updating, percent}}}
 
       {:error, _, message} ->
         :alarm_handler.clear_alarm(NervesHubLink.UpdateInProgress)
-        {:noreply, %State{state | status: {:fwup_error, message}}}
+        {:noreply, %State{state | status: :idle, previous_status: {:error, message}}}
 
       _ ->
         {:noreply, state}
     end
   end
 
-  @spec maybe_update_firmware(UpdateInfo.t(), [binary()], State.t()) :: State.t()
+  @spec maybe_update_firmware(UpdateInfo.t(), [binary()], State.t()) :: {atom(), State.t()}
   defp maybe_update_firmware(
          %UpdateInfo{} = _update_info,
          _fwup_public_keys,
@@ -186,43 +191,43 @@ defmodule NervesHubLink.UpdateManager do
     # interrupt FWUP and let the task finish. After update and reboot, the
     # device will check-in and get an update message if it was actually new and
     # required
-    state
+    {:ignored, state}
   end
 
   defp maybe_update_firmware(%UpdateInfo{} = update_info, fwup_public_keys, %State{} = state) do
-    # Cancel an existing timer if it exists.
-    # This prevents rescheduled updates`
-    # from compounding.
-    state = maybe_cancel_timer(state)
-
     # possibly offload update decision to an external module.
     # This will allow application developers
     # to control exactly when an update is applied.
     # note: update_available is a behaviour function
     case state.fwup_config.update_available.(update_info) do
       :apply ->
-        start_fwup_stream(update_info, fwup_public_keys, state)
+        NervesHubLink.send_firmware_update_status(:updating)
+        %{state | status: {:updating, 0}}
+        {{:updating, 0}, start_fwup_stream(update_info, fwup_public_keys, state)}
 
       :ignore ->
-        state
+        NervesHubLink.send_firmware_update_status(:ignored)
+        Logger.info("[NervesHubLink] ignoring firmware update request")
+        {:ignored, %{state | status: :idle, previous_status: :ignored}}
 
-      {:reschedule, ms} ->
-        timer =
-          Process.send_after(self(), {:update_reschedule, update_info, fwup_public_keys}, ms)
-
-        Logger.info("[NervesHubLink] rescheduling firmware update in #{ms} milliseconds")
-        %{state | status: :update_rescheduled, update_reschedule_timer: timer}
-    end
-  end
+      {:reschedule, minutes, :minutes} ->
+        until =
+          DateTime.utc_now()
+          |> DateTime.add(minutes, :minute)
 
-  defp maybe_update_firmware(_, _, state), do: state
+        NervesHubLink.send_firmware_update_status(:reschedule, %{until: until})
+        Logger.info("[NervesHubLink] rescheduling firmware update in #{minutes} minutes")
+        {:rescheduled, %{state | status: :idle, previous_status: :rescheduled}}
 
-  defp maybe_cancel_timer(%{update_reschedule_timer: nil} = state), do: state
-
-  defp maybe_cancel_timer(%{update_reschedule_timer: timer} = state) do
-    _ = Process.cancel_timer(timer)
+      {:reschedule, ms} ->
+        until =
+          DateTime.utc_now()
+          |> DateTime.add(round(ms / 1_000), :second)
 
-    %{state | update_reschedule_timer: nil}
+        NervesHubLink.send_firmware_update_status(:reschedule, %{until: until})
+        Logger.info("[NervesHubLink] rescheduling firmware update in #{ms / 1_000} seconds")
+        {:rescheduled, %{state | status: :idle, previous_status: :rescheduled}}
+    end
   end
 
   @spec start_fwup_stream(UpdateInfo.t(), [binary()], State.t()) :: State.t()

test/nerves_hub_link/update_manager_test.exs

@@ -57,13 +57,8 @@ defmodule NervesHubLink.UpdateManagerTest do
       }
 
       {:ok, manager} = UpdateManager.start_link(fwup_config)
-      assert UpdateManager.apply_update(manager, update_payload, []) == :update_rescheduled
+      assert UpdateManager.apply_update(manager, update_payload, []) == :rescheduled
       assert_received :rescheduled
-      refute_received {:fwup, _}
-
-      assert_receive {:fwup, {:progress, 0}}, 250
-      assert_receive {:fwup, {:progress, 100}}
-      assert_receive {:fwup, {:ok, 0, ""}}
     end
 
     test "apply with fwup environment", %{update_payload: update_payload, devpath: devpath} do