Update EnigmaWorker to not die causing supervisor restarts.

* Tests for enigma cleanup

farmbot_core/lib/farmbot_core/asset/supervisor.ex

@@ -12,7 +12,8 @@ defmodule FarmbotCore.Asset.Supervisor do
     FbosConfig,
     PinBinding,
     Peripheral,
-    PersistentRegimen
+    PersistentRegimen,
+    Private.Enigma
   }
 
   def start_link(args) do
@@ -30,6 +31,7 @@ defmodule FarmbotCore.Asset.Supervisor do
       {AssetSupervisor, module: Peripheral},
       {AssetSupervisor, module: FarmwareInstallation},
       {AssetSupervisor, module: FarmwareEnv},
+      {AssetSupervisor, [module: Enigma, strategy: :transient]},
       AssetMonitor,
       EnigmaHandler,
     ]

farmbot_core/lib/farmbot_core/asset_monitor.ex

@@ -16,7 +16,8 @@ defmodule FarmbotCore.AssetMonitor do
     FarmwareEnv,
     Peripheral,
     PersistentRegimen,
-    PinBinding
+    PinBinding,
+    Private.Enigma
   }
 
   alias FarmbotCore.{AssetSupervisor, AssetWorker}
@@ -116,6 +117,7 @@ defmodule FarmbotCore.AssetMonitor do
     do: [
       Device,
       FbosConfig,
+      Enigma,
       FarmEvent,
       Peripheral,
       PersistentRegimen,

farmbot_core/lib/farmbot_core/asset_workers/private/enigma_worker.ex

@@ -12,24 +12,36 @@ defimpl FarmbotCore.AssetWorker, for: FarmbotCore.Asset.Private.Enigma do
   def preload(%Enigma{}), do: []
 
   def start_link(%Enigma{} = enigma, _args) do
-    GenServer.start_link(__MODULE__, %Enigma{} = enigma)
+    GenServer.start_link(__MODULE__, enigma)
   end
 
   def init(%Enigma{} = enigma) do
-    {:ok, %Enigma{} = enigma, 0}
+    {:ok, %{enigma: enigma, dead: false}, 0}
   end
 
-  def terminate(_, enigma) do
-    BotState.clear_enigma(enigma)
-    FarmbotCore.EnigmaHandler.handle_down(enigma)
+  def terminate(_reason, state) do
+    finish(state)
   end
 
-  def handle_info(:timeout, %Enigma{} = enigma) do
+  def handle_info(:timeout, %{dead: true} = state) do
+    {:noreply, state, :hibernate}
+  end
+
+  def handle_info(:timeout, %{enigma: enigma, dead: false} = state) do
     BotState.add_enigma(enigma)
     # Handle enigma and block stuff.
     case FarmbotCore.EnigmaHandler.handle_up(enigma) do
-     {:error, _} -> {:noreply, enigma, @error_retry_time_ms}
-     :ok -> {:stop, :normal, enigma}
+      {:error, _} ->
+        {:noreply, state, @error_retry_time_ms}
+
+      :ok ->
+        {:noreply, finish(%{state | dead: true})}
     end
   end
+
+  def finish(state) do
+    BotState.clear_enigma(state.enigma)
+    FarmbotCore.EnigmaHandler.handle_down(state.enigma)
+    state
+  end
 end

farmbot_core/lib/farmbot_core/enigma_handler.ex

@@ -1,8 +1,14 @@
 defmodule FarmbotCore.EnigmaHandler do
   use GenServer
 
-  @callback enigma_up(FarmbotCore.Assets.Private.Enigma.t) :: any
-  @callback enigma_down(FarmbotCore.Assets.Private.Enigma.t) :: any
+  @type enigma :: FarmbotCore.Assets.Private.Enigma.t()
+
+  @doc """
+  The `up` callback will cause the EnigmaWorker to terminate if the
+  callback returns `:ok`
+  """
+  @type enigma_up_callback :: (enigma -> :ok | {:error, term})
+  @type enigma_down_callback :: (enigma -> :ok | {:error, term})
 
   def start_link(args, opts \\ [name: __MODULE__]) do
     GenServer.start_link(__MODULE__, args, opts)

farmbot_core/priv/asset/migrations/20190319162822_add_enigmas_table.exs

@@ -7,7 +7,7 @@ defmodule FarmbotCore.Config.Repo.Migrations.AddEnigmasTable do
       add(:problem_tag, :string)
       add(:priority, :integer)
 
-      add(:monitor, :boolean)
+      add(:monitor, :boolean, default: true)
       timestamps(inserted_at: :created_at, type: :utc_datetime)
     end
   end

farmbot_core/test/asset_workers/private/enigma_worker_test.exs

@@ -2,7 +2,7 @@ defmodule FarmbotCore.Private.EnigmaWorkerTest do
   use ExUnit.Case, async: true
   alias FarmbotCore.{BotState, EnigmaHandler, Asset.Private.Enigma, AssetWorker}
 
-  test "enigmas existance is persisted to the bot's state" do
+  test "enigmas existence is persisted to the bot's state" do
     Process.flag(:trap_exit, true)
     uuid = Ecto.UUID.generate()
 
@@ -36,6 +36,46 @@ defmodule FarmbotCore.Private.EnigmaWorkerTest do
     refute enigmas[uuid]
   end
 
+  test "post-teardown cleanup" do
+    uuid = Ecto.UUID.generate()
+
+    enigma = %Enigma{
+      local_id: uuid,
+      priority: 100,
+      problem_tag: "yolo.woosh",
+      created_at: DateTime.utc_now()
+    }
+
+    bot_state = BotState.subscribe()
+    # Ensures this enigma hasn't been registered yet somehow
+    refute bot_state.enigmas[uuid]
+    test_pid = self()
+
+    up_fun = fn _ ->
+      send(test_pid, :up)
+      :ok
+    end
+
+    EnigmaHandler.register_up("yolo.woosh", up_fun)
+
+    down_fun = fn _ ->
+      send(test_pid, :down)
+      :ok
+    end
+
+    EnigmaHandler.register_down("yolo.woosh", down_fun)
+
+    # Start the enigma manager process
+    {:ok, pid} = AssetWorker.start_link(enigma)
+    assert_receive {BotState, %{changes: %{enigmas: enigmas}}}
+    assert enigmas[uuid]
+    assert_receive :up
+
+    assert_receive :down
+    assert_receive {BotState, %{changes: %{enigmas: enigmas}}}
+    refute enigmas[uuid]
+  end
+
   test "enigmas lifecycle events" do
     uuid = Ecto.UUID.generate()
 

farmbot_os/lib/farmbot_os/init/enigma_firmware_missing.ex

@@ -5,12 +5,25 @@ defmodule FarmbotOS.Init.EnigmaFirmwareMissing do
 
   require FarmbotCore.Logger
 
+  use Supervisor
+
+  @doc false
+  def start_link(args) do
+    Supervisor.start_link(__MODULE__, args, name: __MODULE__)
+  end
+
+  @doc false
+  def init([]) do
+    setup()
+    # This is wrong. TODO: Use tasks #shipit
+    :ignore
+  end
+
   def setup() do
     EnigmaHandler.register_up("firmware.missing", &enigma_up/1)
     EnigmaHandler.register_down("firmware.missing", &enigma_down/1)
 
-    needs_flash? =
-      FarmbotCore.Config.get_config_value(:string, "settings", "firmware_needs_flash")
+    needs_flash? = FarmbotCore.Config.get_config_value(:bool, "settings", "firmware_needs_flash")
 
     firmware_hardware = FarmbotCore.Asset.fbos_config(:firmware_hardware)
     situation = {needs_flash?, firmware_hardware}
@@ -38,15 +51,17 @@ defmodule FarmbotOS.Init.EnigmaFirmwareMissing do
     end
   end
 
+  # Returning :ok here will cause the enigma to be cleared.
+  # We don't need this callback currently, so return error.
   def enigma_up(_) do
-    :ok
+    {:error, :noop}
   end
 
   def enigma_down(_) do
     swap_transport(FarmbotCore.Asset.fbos_config(:firmware_path))
   end
 
-  def swap_transport(tty) do
+  defp swap_transport(tty) do
     # Swap transport on FW module.
     # Close tranpsort if it is open currently.
     _ = FarmbotFirmware.close_transport()

farmbot_os/lib/farmbot_os/init/supervisor.ex

@@ -11,7 +11,8 @@ defmodule FarmbotOS.Init.Supervisor do
     children =
       (config[:init_children] || []) ++
         [
-          FarmbotOS.Init.FSCheckup
+          FarmbotOS.Init.FSCheckup,
+          FarmbotOS.Init.EnigmaFirmwareMissing
         ]
 
     Supervisor.init(children, strategy: :one_for_all)

test/support/asset_fixtures.ex

@@ -95,9 +95,9 @@ defmodule Farmbot.TestSupport.AssetFixtures do
 
   def enigma() do
     Private.create_or_update_enigma!(%{
-        problem_tag: "firmware.missing",
-        priority: 100,
-        monitor: false
+      problem_tag: "firmware.missing",
+      priority: 100,
+      monitor: false
     })
   end
 end