Ready for QA

farmbot_core/lib/farmbot_core.ex

@@ -27,7 +27,10 @@ defmodule FarmbotCore do
       FarmbotCore.FirmwareOpenTask,
       FarmbotCore.FirmwareEstopTimer,
       # Also error handling for a transport not starting ?
-      {FarmbotFirmware, transport: FarmbotFirmware.StubTransport, side_effects: FarmbotCore.FirmwareSideEffects},
+      {FarmbotFirmware,
+      transport: FarmbotFirmware.StubTransport,
+      side_effects: FarmbotCore.FirmwareSideEffects,
+      reset: FarmbotCore.FirmwareResetter},
       FarmbotCeleryScript.Scheduler
     ]
     config = (Application.get_env(:farmbot_ext, __MODULE__) || [])

farmbot_core/lib/farmbot_core/firmware_resetter.ex

@@ -0,0 +1,54 @@
+defmodule FarmbotCore.FirmwareResetter do
+  defmodule Stub do
+    require FarmbotCore.Logger
+
+    def fail do
+      m = "Reset function NOT FOUND. Please notify FarmBot support."
+      FarmbotCore.Logger.error(3, m)
+      {:error, m}
+    end
+
+    def open(_, _), do: fail()
+    def write(_, _), do: fail()
+  end
+
+  @gpio Application.get_env(:farmbot, __MODULE__, [])[:gpio] || Stub
+  alias FarmbotCore.Asset
+  require FarmbotCore.Logger
+
+  def reset(package \\ nil) do
+    pkg = package || Asset.fbos_config(:firmware_hardware)
+    FarmbotCore.Logger.debug(3, "Attempting to retrieve #{pkg} reset function.")
+    {:ok, fun} = find_reset_fun(pkg)
+    fun.()
+  end
+
+  def find_reset_fun("express_k10") do
+    FarmbotCore.Logger.debug(3, "Using special express reset function")
+    {:ok, fn -> express_reset_fun() end}
+  end
+
+  def find_reset_fun(_) do
+    FarmbotCore.Logger.debug(3, "Using default reset function")
+    {:ok, fn -> :ok end}
+  end
+
+  def express_reset_fun() do
+    try do
+      FarmbotCore.Logger.debug(3, "Begin MCU reset")
+      {:ok, gpio} = @gpio.open(19, :output)
+      :ok = @gpio.write(gpio, 0)
+      :ok = @gpio.write(gpio, 1)
+      Process.sleep(1000)
+      :ok = @gpio.write(gpio, 0)
+      FarmbotCore.Logger.debug(3, "Finish MCU Reset")
+      :ok
+    rescue
+      ex ->
+        message = Exception.message(ex)
+        msg = "Could not reset express firmware: #{message}"
+        FarmbotCore.Logger.error(3, msg)
+        :express_reset_error
+    end
+  end
+end

farmbot_ext/test/test_helper.exs

@@ -21,12 +21,12 @@ timeout = System.get_env("EXUNIT_TIMEOUT") || "5000"
 System.put_env("LOG_SILENCE", "true")
 
 ExUnit.start(assert_receive_timeout: String.to_integer(timeout))
+# Use this to stub out calls to `state.reset.reset()` in firmware.
+defmodule StubReset do
+  def reset(), do: :ok
+end
 
 defmodule Helpers do
-  # Maybe I don't need this?
-  # Maybe I could use `start_supervised`?
-  # https://hexdocs.pm/ex_unit/ExUnit.Callbacks.html#start_supervised/2
-
   @wait_time 180
   # Base case: We have a pid
   def wait_for(pid) when is_pid(pid), do: check_on_mbox(pid)

farmbot_firmware/lib/farmbot_firmware.ex

@@ -107,8 +107,7 @@ defmodule FarmbotFirmware do
     :command_queue,
     :caller_pid,
     :current,
-    :reset,
-    :reset_pid
+    :reset
   ]
 
   @type state :: %State{
@@ -123,8 +122,7 @@ defmodule FarmbotFirmware do
           command_queue: [{pid(), GCODE.t()}],
           caller_pid: nil | pid,
           current: nil | GCODE.t(),
-          reset: module(),
-          reset_pid: nil | pid()
+          reset: module()
         }
 
   @doc """
@@ -202,16 +200,7 @@ defmodule FarmbotFirmware do
     args = Keyword.merge(args, global)
     transport = Keyword.fetch!(args, :transport)
     side_effects = Keyword.get(args, :side_effects)
-    # This is probably the cause of
-    # https://github.com/FarmBot/farmbot_os/issues/1111
-    # FarmbotFirmware.NullReset (RPi3? Safe default?)
-    #  -OR-
-    # FarmbotOS.Platform.Target.FirmwareReset.GPIO (RPi0, RPi)
-    #  -OR-
-    # Use Application.get_env to find target?
-    # probably?
-    reset = Keyword.get(args, :reset) || FarmbotFirmware.NullReset
-
+    reset = Keyword.fetch!(args, :reset)
     # Add an anon function that transport implementations should call.
     fw = self()
     fun = fn {_, _} = code -> GenServer.cast(fw, code) end
@@ -225,7 +214,6 @@ defmodule FarmbotFirmware do
       side_effects: side_effects,
       status: :transport_boot,
       reset: reset,
-      reset_pid: nil,
       command_queue: [],
       configuration_queue: []
     }
@@ -242,24 +230,6 @@ defmodule FarmbotFirmware do
       GenServer.stop(state.transport_pid)
   end
 
-  def handle_info(:timeout, %{status: :transport_boot, reset_pid: nil} = state) do
-    case GenServer.start_link(state.reset, state.transport_args,
-           name: state.reset
-         ) do
-      {:ok, pid} ->
-        {:noreply, %{state | reset_pid: pid}}
-
-      # TODO(Rick): I have no idea what's going on here.
-      {:error, {:already_started, pid}} ->
-        {:noreply, %{state | reset_pid: pid}}
-
-      error ->
-        Logger.error("Error starting Firmware Reset: #{inspect(error)}")
-        Process.send_after(self(), :timeout, @transport_init_error_retry_ms)
-        {:noreply, state}
-    end
-  end
-
   # This will be the first message received right after `init/1`
   # It should try to open a transport every `transport_init_error_retry_ms`
   # until success.

farmbot_firmware/test/farmbot_firmware/command_test.exs

@@ -6,12 +6,16 @@ defmodule FarmbotFirmware.CommandTest do
   import ExUnit.CaptureLog
   @subject FarmbotFirmware.Command
 
-  @tag :capture_log
-  test "command() runs RPCs" do
-    arg = [transport: FarmbotFirmware.StubTransport]
+  def fake_pid() do
+    arg = [transport: FarmbotFirmware.StubTransport, reset: StubReset]
     {:ok, pid} = FarmbotFirmware.start_link(arg, [])
     send(pid, :timeout)
+    pid
+  end
 
+  @tag :capture_log
+  test "command() runs RPCs" do
+    pid = fake_pid()
     assert {:error, :emergency_lock} ==
              FarmbotFirmware.command(pid, {:command_emergency_lock, []})
 
@@ -22,9 +26,7 @@ defmodule FarmbotFirmware.CommandTest do
 
   @tag :capture_log
   test "command() refuses to run RPCs in :boot state" do
-    arg = [transport: FarmbotFirmware.StubTransport]
-    {:ok, pid} = FarmbotFirmware.start_link(arg, [])
-    send(pid, :timeout)
+    pid = fake_pid()
     {:error, message} = @subject.command(pid, {:a, {:b, :c}})
     assert "Can't send command when in :boot state" == message
   end

farmbot_firmware/test/farmbot_firmware_test.exs

@@ -11,7 +11,7 @@ defmodule FarmbotFirmwareTest do
   end
 
   def firmware_server do
-    arg = [transport: FarmbotFirmware.StubTransport]
+    arg = [transport: FarmbotFirmware.StubTransport, reset: StubReset]
     {:ok, pid} = FarmbotFirmware.start_link(arg, [])
     send(pid, :timeout)
     try_command(pid, {nil, {:command_emergency_lock, []}})

farmbot_os/lib/farmbot_os/sys_calls/flash_firmware.ex

@@ -1,27 +1,10 @@
 defmodule FarmbotOS.SysCalls.FlashFirmware do
   @moduledoc false
 
-  alias FarmbotCore.{Asset, Asset.Private}
+  alias FarmbotCore.{Asset, Asset.Private, FirmwareResetter}
   alias FarmbotFirmware
   alias FarmbotCore.FirmwareTTYDetector
 
-  defmodule Stub do
-    require FarmbotCore.Logger
-
-    def fail do
-      m = "No express function found. Please notify support."
-      FarmbotCore.Logger.error(3, m)
-      {:error, m}
-    end
-
-    def open(_, _), do: fail()
-    def write(_, _), do: fail()
-  end
-
-  # This only matter for express.
-  # When it's an express, use Circuits.GPIO.
-  @gpio Application.get_env(:farmbot, __MODULE__, [])[:gpio] || Stub
-
   import FarmbotFirmware.PackageUtils,
     only: [find_hex_file: 1, package_to_string: 1]
 
@@ -38,7 +21,7 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
          {:ok, tty} <- find_tty(),
          _ <-
            FarmbotCore.Logger.debug(3, "found tty: #{tty} for firmware flash"),
-         {:ok, fun} <- find_reset_fun(package),
+         {:ok, fun} <- FirmwareResetter.find_reset_fun(package),
          _ <-
            FarmbotCore.Logger.debug(
              3,
@@ -84,32 +67,4 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
         {:ok, tty}
     end
   end
-
-  def find_reset_fun("express_k10") do
-    FarmbotCore.Logger.debug(3, "Using special express reset function")
-    {:ok, fn -> express_reset_fun() end}
-  end
-
-  def find_reset_fun(_) do
-    FarmbotCore.Logger.debug(3, "Using default reset function")
-    {:ok, &FarmbotFirmware.NullReset.reset/0}
-  end
-
-  def express_reset_fun() do
-    try do
-      FarmbotCore.Logger.debug(3, "Begin MCU reset")
-      {:ok, gpio} = @gpio.open(19, :output)
-      :ok = @gpio.write(gpio, 0)
-      :ok = @gpio.write(gpio, 1)
-      Process.sleep(1000)
-      :ok = @gpio.write(gpio, 0)
-      FarmbotCore.Logger.debug(3, "Finish MCU Reset")
-      :ok
-    rescue
-      ex ->
-        message = Exception.message(ex)
-        Logger.error("Could not flash express firmware: #{message}")
-        :express_reset_error
-    end
-  end
 end

farmbot_os/test/farmbot_os/syscalls/resource_update_test.exs

@@ -31,7 +31,7 @@ defmodule FarmbotOS.SysCalls.ResourceUpdateTest do
     params = %{name: "Updated to {{ x }}"}
     assert :ok == ResourceUpdate.update_resource("Plant", 555, params)
     next_plant = PointLookup.point("Plant", 555)
-    assert "Updated to " == next_plant.name
+    assert String.contains?(next_plant.name, "Updated to ")
 
     bad_result1 = ResourceUpdate.update_resource("Plant", 0, params)
     error = "Plant.0 is not currently synced, so it could not be updated"