Fix Firmware flashing on boot on express boards

farmbot_core/config/config.exs

@@ -42,6 +42,8 @@ config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: []
 
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 5
 
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+
 config :farmbot_core, FarmbotCore.AssetWorker.FarmbotCore.Asset.FbosConfig,
   firmware_flash_attempt_threshold: 5
 

farmbot_ext/config/config.exs

@@ -6,6 +6,8 @@ config :logger, handle_otp_reports: true, handle_sasl_reports: true
 config :farmbot_celery_script, FarmbotCeleryScript.SysCalls,
   sys_calls: FarmbotCeleryScript.SysCalls.Stubs
 
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+
 import_config "ecto.exs"
 import_config "farmbot_core.exs"
 import_config "lagger.exs"

farmbot_firmware/lib/farmbot_firmware.ex

@@ -129,7 +129,9 @@ defmodule FarmbotFirmware do
     :command_queue,
     :caller_pid,
     :current,
-    :vcr_fd
+    :vcr_fd,
+    :reset,
+    :reset_pid
   ]
 
   @type state :: %State{
@@ -144,7 +146,9 @@ defmodule FarmbotFirmware do
           command_queue: [{pid(), GCODE.t()}],
           caller_pid: nil | pid,
           current: nil | GCODE.t(),
-          vcr_fd: nil | File.io_device()
+          vcr_fd: nil | File.io_device(),
+          reset: module(),
+          reset_pid: nil | pid()
         }
 
   @doc """
@@ -200,6 +204,10 @@ defmodule FarmbotFirmware do
     GenServer.call(server, {:open_transport, module, args})
   end
 
+  def reset(server \\ __MODULE__) do
+    GenServer.call(server, :reset)
+  end
+
   @doc """
   Sets the Firmware server to record input and output GCODES
   to a pair of text files.
@@ -233,6 +241,7 @@ defmodule FarmbotFirmware do
     args = Keyword.merge(args, global)
     transport = Keyword.fetch!(args, :transport)
     side_effects = Keyword.get(args, :side_effects)
+    reset = Keyword.fetch!(args, :reset)
 
     vcr_fd =
       case Keyword.get(args, :vcr_path) do
@@ -256,6 +265,8 @@ defmodule FarmbotFirmware do
       transport_args: transport_args,
       side_effects: side_effects,
       status: :transport_boot,
+      reset: reset,
+      reset_pid: nil,
       command_queue: [],
       configuration_queue: [],
       vcr_fd: vcr_fd
@@ -273,6 +284,19 @@ 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} ->
+        Logger.debug("Firmware reset #{state.reset} started. #{inspect(state.transport_args)}")
+        {: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.
@@ -357,6 +381,11 @@ defmodule FarmbotFirmware do
     {:noreply, state}
   end
 
+  def handle_call(:reset, _from, state) do
+    r = state.reset.reset()
+    {:reply, r, state}
+  end
+
   # Closing the transport will purge the buffer of queued commands in both
   # the `configuration_queue` and in the `command_queue`.
   def handle_call(:close_transport, _from, %{status: s} = state) when s != :transport_boot do
@@ -388,7 +417,12 @@ defmodule FarmbotFirmware do
     # Add an anon function that transport implementations should call.
     fw = self()
     fun = fn {_, _} = code -> GenServer.cast(fw, code) end
-    transport_args = Keyword.put(args, :handle_gcode, fun)
+
+    transport_args =
+      state.transport_args
+      |> Keyword.merge(args)
+      |> Keyword.merge(handle_gcode: fun)
+
     next_state = %{state | transport: module, transport_args: transport_args}
 
     send(self(), :timeout)

farmbot_firmware/lib/farmbot_firmware/null_reset.ex

@@ -0,0 +1,15 @@
+defmodule FarmbotFirmware.NullReset do
+  @moduledoc """
+  Does nothing in reference to resetting the firmware port
+  """
+  @behaviour FarmbotFirmware.Reset
+  use GenServer
+
+  @impl FarmbotFirmware.Reset
+  def reset(), do: :ok
+
+  @impl GenServer
+  def init(_args) do
+    {:ok, %{}}
+  end
+end

farmbot_firmware/lib/farmbot_firmware/reset.ex

@@ -1,4 +1,4 @@
-defmodule FarmbotFirmware.UARTTransport.Reset do
+defmodule FarmbotFirmware.Reset do
   @moduledoc """
   Behaviour to reset the UART connection into 
   bootloader mode for firmware upgrades.

farmbot_firmware/lib/farmbot_firmware/transports/uart_transport.ex

@@ -14,8 +14,9 @@ defmodule FarmbotFirmware.UARTTransport do
   def init(args) do
     device = Keyword.fetch!(args, :device)
     handle_gcode = Keyword.fetch!(args, :handle_gcode)
+    reset = Keyword.fetch!(args, :reset)
     {:ok, uart} = UART.start_link()
-    {:ok, %{uart: uart, device: device, open: false, handle_gcode: handle_gcode}, 0}
+    {:ok, %{uart: uart, device: device, open: false, handle_gcode: handle_gcode, reset: reset}, 0}
   end
 
   def terminate(_, state) do
@@ -26,7 +27,7 @@ defmodule FarmbotFirmware.UARTTransport do
     opts = [active: true, speed: 115_200, framing: {UART.Framing.Line, separator: "\r\n"}]
 
     with :ok <- open(state.uart, state.device, opts),
-         :ok <- reset(state.uart, state.device, opts) do
+         :ok <- reset(state) do
       {:noreply, %{state | open: true}}
     else
       {:error, reason} ->
@@ -51,8 +52,8 @@ defmodule FarmbotFirmware.UARTTransport do
     {:reply, r, state}
   end
 
-  def reset(_uart_pid, _device_path, _opts) do
-    if module = config()[:reset] do
+  def reset(state) do
+    if module = state[:reset] do
       module.reset()
     else
       :ok
@@ -62,8 +63,4 @@ defmodule FarmbotFirmware.UARTTransport do
   def open(uart_pid, device_path, opts) do
     UART.open(uart_pid, device_path, opts)
   end
-
-  defp config do
-    Application.get_env(:farmbot_firmware, __MODULE__)
-  end
 end

farmbot_os/config/config.exs

@@ -66,6 +66,8 @@ config :farmbot, FarmbotOS.Platform.Supervisor,
     FarmbotOS.Platform.Host.Configurator
   ]
 
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+
 import_config("lagger.exs")
 
 if Mix.target() == :host do

farmbot_os/config/target/rpi.exs

@@ -2,11 +2,9 @@ use Mix.Config
 
 config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: ["ttyUSB0", "ttyAMA0"]
 
-config :farmbot_firmware, FarmbotFirmware.UARTTransport,
-  reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
 
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [
-    FarmbotOS.Platform.Target.RTCWorker,
-    FarmbotOS.Platform.Target.FirmwareReset.GPIO
+    FarmbotOS.Platform.Target.RTCWorker
   ]

farmbot_os/config/target/rpi0.exs

@@ -4,11 +4,9 @@ config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: ["ttyUSB0
 
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 50
 
-config :farmbot_firmware, FarmbotFirmware.UARTTransport,
-  reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
 
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [
-    FarmbotOS.Platform.Target.RTCWorker,
-    FarmbotOS.Platform.Target.FirmwareReset.GPIO
+    FarmbotOS.Platform.Target.RTCWorker
   ]

farmbot_os/config/target/rpi3.exs

@@ -2,8 +2,7 @@ use Mix.Config
 
 config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: ["ttyUSB0", "ttyACM0"]
 
-config :farmbot_firmware, FarmbotFirmware.UARTTransport,
-  reset: FarmbotOS.Platform.Target.FirmwareReset.NULL
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotOS.Platform.Target.FirmwareReset.NULL
 
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [

farmbot_os/lib/avrdude.ex

@@ -4,6 +4,7 @@ defmodule Avrdude do
   """
 
   @uart_speed 115_200
+  require FarmbotCore.Logger
 
   @spec flash(Path.t(), Path.t(), (() -> :ok)) :: {number, any()}
   def flash(hex_path, tty_path, reset_fun) do
@@ -28,7 +29,22 @@ defmodule Avrdude do
     ]
 
     # call the function for resetting the line before executing avrdude.
-    :ok = reset_fun.()
-    MuonTrap.cmd("avrdude", args, into: IO.stream(:stdio, :line))
+    call_reset_fun(reset_fun)
+    MuonTrap.cmd("avrdude", args, into: IO.stream(:stdio, :line), stderr_to_stdout: true)
   end
+
+  def call_reset_fun(reset_fun) do
+    try do
+      reset_fun.()
+    catch
+      error_type, error ->
+        FarmbotCore.Logger.error(1, """
+        Error calling reset function: #{inspect(reset_fun)} 
+        error type: #{error_type}
+        error: #{inspect(error)}
+        """)
+    end
+  end
+
+  :ok
 end

farmbot_os/lib/farmbot_os/sys_calls/flash_firmware.ex

@@ -13,8 +13,11 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
 
     with {:ok, hex_file} <- find_hex_file(package),
          {:ok, tty} <- find_tty(),
+         _ <- FarmbotCore.Logger.debug(3, "found tty: #{tty} for firmware flash"),
          {:ok, fun} <- find_reset_fun(package),
+         _ <- FarmbotCore.Logger.debug(3, "closing firmware transport before flash"),
          :ok <- FarmbotFirmware.close_transport(),
+         _ <- FarmbotCore.Logger.debug(3, "starting firmware flash"),
          {_, 0} <- Avrdude.flash(hex_file, tty, fun) do
       FarmbotCore.Logger.success(2, "Firmware flashed successfully!")
 
@@ -49,11 +52,13 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
   end
 
   defp find_reset_fun(_) do
-    config = Application.get_env(:farmbot_firmware, FarmbotFirmware.UARTTransport)
+    config = Application.get_env(:farmbot_firmware, FarmbotFirmware)
 
     if module = config[:reset] do
+      Logger.error("using reset function: #{inspect(config)}")
       {:ok, &module.reset/0}
     else
+      Logger.error("no reset function is going to be used #{inspect(config)}")
       {:ok, fn -> :ok end}
     end
   end

farmbot_os/platform/target/firmware_reset/gpio.ex

@@ -2,28 +2,28 @@ defmodule FarmbotOS.Platform.Target.FirmwareReset.GPIO do
   @moduledoc """
   Uses GPIO pin 19 to reset the firmware.
   """
-  @behaviour FarmbotFirmware.UARTTransport.Reset
+  @behaviour FarmbotFirmware.Reset
 
   use GenServer
+  require Logger
 
-  @impl FarmbotFirmware.UARTTransport.Reset
-  def reset do
-    GenServer.call(__MODULE__, :reset)
-  end
-
-  @doc false
-  def start_link(args) do
-    GenServer.start_link(__MODULE__, args, name: __MODULE__)
+  @impl FarmbotFirmware.Reset
+  def reset(server \\ __MODULE__) do
+    Logger.debug("calling gpio reset/0")
+    GenServer.call(server, :reset)
   end
 
   @impl GenServer
-  def init(_) do
+  def init(_args) do
+    Logger.debug("initializing gpio thing for firmware reset")
     {:ok, gpio} = Circuits.GPIO.open(19, :output)
     {:ok, %{gpio: gpio}}
   end
 
   @impl GenServer
   def handle_call(:reset, _from, state) do
+    Logger.warn("doing firmware gpio reset")
+
     with :ok <- Circuits.GPIO.write(state.gpio, 1),
          :ok <- Circuits.GPIO.write(state.gpio, 0) do
       {:reply, :ok, state}

farmbot_os/platform/target/firmware_reset/null.ex

@@ -1,9 +0,0 @@
-defmodule FarmbotOS.Platform.Target.FirmwareReset.NULL do
-  @moduledoc """
-  Does nothing in reference to resetting the firmware port
-  """
-  @behaviour FarmbotFirmware.UARTTransport.Reset
-
-  @impl FarmbotFirmware.UARTTransport.Reset
-  def reset, do: :ok
-end