the Client behaviour can now be used with use

this allows for people to use the defaults, and override the implementations they want without having to implement all of them.
the `ClientTest` no longer really makes sense, so I've removed it

README.md

@@ -308,13 +308,15 @@ mix nerves_hub.firmware publish --key devkey --deploy qa_deployment
 
 ### Conditionally applying updates
 
-It's not always appropriate to apply a firmware update immediately. Custom logic can be added to the device by implementing the `NervesHubLink.Client` behaviour and telling the NervesHubLink OTP application about it.
+It's not always appropriate to apply a firmware update immediately. Custom logic can be added to the device by implementing the `NervesHubLink.Client` behaviour, or extending `NervesHubLink.Client`, and telling the `NervesHubLink` OTP application to use it.
+
+__It's recommended to extend `NervesHubLink.Client` by using `use NervesHubLink.Client` as this will allow you to override individual functions instead of needing to implement the entire behaviour.__
 
 Here's an example implementation:
 
 ```elixir
 defmodule MyApp.NervesHubLinkClient do
-   @behaviour NervesHubLink.Client
+   use NervesHubLink.Client
 
    # May return:
    #  * `:apply` - apply the action immediately
@@ -344,7 +346,8 @@ See the previous section for implementing a `client` behaviour.
 
 ```elixir
 defmodule MyApp.NervesHubLinkClient do
-  @behaviour NervesHubLink.Client
+  use NervesHubLink.Client
+
   #  argument can be:
   #   {:ok, non_neg_integer(), String.t()}
   #   {:warning, non_neg_integer(), String.t()}
@@ -392,14 +395,14 @@ config :nerves_hub_link, remote_iex_timeout: 900 # 15 minutes
 
 You may also need additional permissions on NervesHub to see the device and to use the remote IEx feature.
 
-### Alarms 
+### Alarms
 
 This application can set and clear the following alarms:
 
 * `NervesHubLink.Disconnected`
   * set: An issue is preventing a connection to NervesHub or one just hasn't been made yet
   * clear: Currently connected to NervesHub
-* `NervesHubLink.UpdateInProgress` 
+* `NervesHubLink.UpdateInProgress`
   * set: A new firmware update is being downloaded or applied
   * clear: No updates are happening
 

config/test.exs

@@ -17,7 +17,7 @@ config :nerves_hub_link,
   ]
 
 config :nerves_hub_link,
-  client: NervesHubLink.ClientMock,
+  client: NervesHubLink.Support.TestClient,
   rejoin_after: 0,
   remote_iex: true
 

lib/nerves_hub_link/application.ex

@@ -2,7 +2,6 @@ defmodule NervesHubLink.Application do
   use Application
 
   alias NervesHubLink.ArchiveManager
-  alias NervesHubLink.Client
   alias NervesHubLink.Configurator
   alias NervesHubLink.Socket
   alias NervesHubLink.FwupConfig
@@ -15,8 +14,8 @@ defmodule NervesHubLink.Application do
       fwup_public_keys: config.fwup_public_keys,
       fwup_devpath: config.fwup_devpath,
       fwup_env: config.fwup_env,
-      handle_fwup_message: &Client.handle_fwup_message/1,
-      update_available: &Client.update_available/1
+      handle_fwup_message: &config.client.handle_fwup_message/1,
+      update_available: &config.client.update_available/1
     }
 
     children = children(config, fwup_config)

lib/nerves_hub_link/archive_manager.ex

@@ -12,7 +12,6 @@ defmodule NervesHubLink.ArchiveManager do
 
   use GenServer
 
-  alias NervesHubLink.Client
   alias NervesHubLink.Downloader
   alias NervesHubLink.Message.ArchiveInfo
 
@@ -23,6 +22,7 @@ defmodule NervesHubLink.ArchiveManager do
   @type t :: %__MODULE__{
           archive_info: nil | ArchiveInfo.t(),
           archive_public_keys: [binary()],
+          client: NervesHubLink.Client,
           data_path: Path.t(),
           download: nil | GenServer.server(),
           file_path: Path.t(),
@@ -33,6 +33,7 @@ defmodule NervesHubLink.ArchiveManager do
 
   defstruct archive_info: nil,
             archive_public_keys: [],
+            client: nil,
             data_path: nil,
             download: nil,
             file_path: nil,
@@ -84,6 +85,7 @@ defmodule NervesHubLink.ArchiveManager do
   def init(args) do
     state = %__MODULE__{
       archive_public_keys: args.archive_public_keys,
+      client: args.client,
       data_path: args.data_path
     }
 
@@ -124,7 +126,7 @@ defmodule NervesHubLink.ArchiveManager do
     # validate the file
 
     if valid_archive?(state.file_path, state.archive_public_keys) do
-      _ = Client.archive_ready(state.archive_info, state.file_path)
+      _ = state.client.archive_ready(state.archive_info, state.file_path)
     else
       Logger.error(
         "[NervesHubLink] Archive could not be validated, your public keys are configured wrong"
@@ -171,7 +173,7 @@ defmodule NervesHubLink.ArchiveManager do
 
     pid = self()
 
-    case Client.archive_available(info) do
+    case state.client.archive_available(info) do
       :download ->
         {:ok, download} = Downloader.start_download(info.url, &send(pid, {:download, &1}))
 

lib/nerves_hub_link/client.ex

@@ -1,17 +1,24 @@
 defmodule NervesHubLink.Client do
   @moduledoc """
-  A behaviour module for customizing if and when firmware updates get applied.
+  A behaviour module for customizing:
+  - if and when firmware updates get applied
+  - if and when archives get applied
+  - reconnection backoff logic
+  - and customizing how a device is identified and rebooted
 
   By default NervesHubLink applies updates as soon as it knows about them from the
   NervesHubLink server and doesn't give warning before rebooting. This let's
   devices hook into the decision making process and monitor the update's
   progress.
 
+  You can either implement all the callbacks for the `NervesHubLink.Client` behaviour,
+  or you can `use NervesHubLink.Client` and override the default implementation.
+
   # Example
 
   ```elixir
   defmodule MyApp.NervesHubLinkClient do
-    @behaviour NervesHubLink.Client
+    use NervesHubLink.Client
 
     # May return:
     #  * `:apply` - apply the action immediately
@@ -99,10 +106,7 @@ defmodule NervesHubLink.Client do
   @callback handle_error(any()) :: :ok
 
   @doc """
-  Optional callback when the socket disconnected, before starting to reconnect.
-
-  The return value is used to reset the next socket's retry timeout. `nil` uses
-  the default. The default is a call to `NervesHubLink.Backoff.delay_list/3`.
+  Callback when the socket disconnected, before starting to reconnect.
 
   You may wish to use this to dynamically change the reconnect backoffs. For instance,
   during a NervesHub deploy you may wish to change the reconnect based on your
@@ -118,139 +122,116 @@ defmodule NervesHubLink.Client do
   @callback identify() :: :ok
 
   @doc """
-  Optional callback to reboot the device when a firmware update completes
-
-  The default behavior is to call `Nerves.Runtime.reboot/0` after a successful update. This
-  is useful for testing and for doing additional work like notifying users in a UI that a reboot
-  will happen soon. It is critical that a reboot does happen.
+  Callback to reboot the device when a firmware update completes
   """
   @callback reboot() :: no_return()
 
-  @optional_callbacks [reconnect_backoff: 0, reboot: 0]
+  defmacro __using__(_opts) do
+    quote do
+      @behaviour NervesHubLink.Client
+      require Logger
 
-  @doc """
-  This function is called internally by NervesHubLink to notify clients.
-  """
-  @spec update_available(update_data()) :: update_response()
-  def update_available(data) do
-    case apply_wrap(mod(), :update_available, [data]) do
-      :apply ->
-        :apply
+      @impl true
+      def update_available(update_info) do
+        if update_info.firmware_meta.uuid == Nerves.Runtime.KV.get_active("nerves_fw_uuid") do
+          Logger.info("""
+          [NervesHubLink.Client] Ignoring request to update to the same firmware
 
-      :ignore ->
-        :ignore
+          #{inspect(update_info)}
+          """)
 
-      {:reschedule, timeout} when timeout > 0 ->
-        {:reschedule, timeout}
+          :ignore
+        else
+          :apply
+        end
+      end
 
-      wrong ->
-        Logger.error(
-          "[NervesHubLink] Client: #{inspect(mod())}.update_available/1 bad return value: #{inspect(wrong)} Applying update."
+      @impl true
+      def archive_available(archive_info) do
+        Logger.info(
+          "[NervesHubLink.Client] Archive is available for downloading #{inspect(archive_info)}"
         )
 
-        :apply
-    end
-  end
-
-  @spec archive_available(archive_data()) :: archive_response()
-  def archive_available(data) do
-    apply_wrap(mod(), :archive_available, [data])
-  end
-
-  @spec archive_ready(archive_data(), Path.t()) :: :ok
-  def archive_ready(data, file_path) do
-    _ = apply_wrap(mod(), :archive_ready, [data, file_path])
+        :ignore
+      end
 
-    :ok
-  end
+      @impl true
+      def archive_ready(archive_info, file_path) do
+        Logger.info(
+          "[NervesHubLink.Client] Archive is ready for processing #{inspect(archive_info)} at #{inspect(file_path)}"
+        )
 
-  @doc """
-  This function is called internally by NervesHubLink to notify clients of fwup progress.
-  """
-  @spec handle_fwup_message(fwup_message()) :: :ok
-  def handle_fwup_message(data) do
-    _ = apply_wrap(mod(), :handle_fwup_message, [data])
+        :ok
+      end
 
-    # TODO: nasty side effects here. Consider moving somewhere else
-    case data do
-      {:progress, percent} ->
+      @impl true
+      def handle_fwup_message({:progress, percent}) do
+        Logger.debug("FWUP PROG: #{percent}%")
         NervesHubLink.send_update_progress(percent)
+        :ok
+      end
 
-      {:error, _, message} ->
+      def handle_fwup_message({:error, _, message}) do
+        Logger.error("FWUP ERROR: #{message}")
         NervesHubLink.send_update_status("fwup error #{message}")
-
-      {:ok, 0, _message} ->
-        initiate_reboot()
-
-      _ ->
         :ok
-    end
-  end
+      end
 
-  @doc """
-  This function is called internally by NervesHubLink to identify a device.
-  """
-  def identify() do
-    apply_wrap(mod(), :identify, [])
-  end
+      def handle_fwup_message({:warning, _, message}) do
+        Logger.warning("FWUP WARN: #{message}")
+        :ok
+      end
 
-  @doc """
-  This function is called internally by NervesHubLink to initiate a reboot.
+      def handle_fwup_message({:ok, status, message}) do
+        Logger.info("FWUP SUCCESS: #{status} #{message}")
+        reboot()
+        :ok
+      end
 
-  After a successful firmware update, NervesHubLink calls this to start the
-  reboot process. It calls `c:reboot/0` if supplied or
-  `Nerves.Runtime.reboot/0`.
-  """
-  @spec initiate_reboot() :: :ok
-  def initiate_reboot() do
-    client = mod()
+      def handle_fwup_message(fwup_message) do
+        Logger.warning("Unknown FWUP message: #{inspect(fwup_message)}")
+        :ok
+      end
 
-    {mod, fun, args} =
-      if function_exported?(client, :reboot, 0),
-        do: {client, :reboot, []},
-        else: {Nerves.Runtime, :reboot, []}
+      @impl true
+      def handle_error(error) do
+        Logger.warning("[NervesHubLink] error: #{inspect(error)}")
+      end
 
-    _ = spawn(mod, fun, args)
-    :ok
-  end
+      @doc """
+      The default implementation checks if the `:reconnect_after_msec` config has been
+      configured, and is a list of values, otherwise `NervesHubLink.Backoff.delay_list/3`
+      is used with a minimum value of 1 second, maximum value of 60 seconds, and a 50% jitter.
+      """
+      @impl true
+      def reconnect_backoff() do
+        socket_config = Application.get_env(:nerves_hub_link, :socket, [])
+
+        backoff = socket_config[:reconnect_after_msec]
+
+        if is_list(backoff) do
+          backoff
+        else
+          NervesHubLink.Backoff.delay_list(1000, 60000, 0.50)
+        end
+      end
 
-  @doc """
-  This function is called internally by NervesHubLink to notify clients of fwup errors.
-  """
-  @spec handle_error(any()) :: :ok
-  def handle_error(data) do
-    _ = apply_wrap(mod(), :handle_error, [data])
-  end
+      @doc """
+      The default implementation calls `Nerves.Runtime.reboot/0` after a successful update. This
+      is useful for testing and for doing additional work like notifying users in a UI that a reboot
+      will happen soon. It is critical that a reboot does happen.
+      """
+      @impl true
+      def reboot() do
+        _ = spawn(Nerves.Runtime, :reboot, [])
+      end
 
-  @doc """
-  This function is called internally by NervesHubLink to notify clients of disconnects.
-  """
-  @spec reconnect_backoff() :: [integer()]
-  def reconnect_backoff() do
-    backoff =
-      if function_exported?(mod(), :reconnect_backoff, 0) do
-        apply_wrap(mod(), :reconnect_backoff, [])
-      else
-        nil
+      @impl true
+      def identify() do
+        Logger.info("[NervesHubLink] identifying")
       end
 
-    if is_list(backoff) do
-      backoff
-    else
-      NervesHubLink.Backoff.delay_list(1000, 60000, 0.50)
+      defoverridable NervesHubLink.Client
     end
   end
-
-  # Catches exceptions and exits
-  defp apply_wrap(mod, function, args) do
-    apply(mod, function, args)
-  catch
-    :error, reason -> {:error, reason}
-    :exit, reason -> {:exit, reason}
-    err -> err
-  end
-
-  defp mod() do
-    Application.get_env(:nerves_hub_link, :client, NervesHubLink.Client.Default)
-  end
 end