Fix Firmware crashes blocking forever

farmbot_core/lib/asset.ex

@@ -50,6 +50,11 @@ defmodule Farmbot.Asset do
     Map.fetch!(fbos_config(), field)
   end
 
+  def update_fbos_config!(fbos_config \\ nil, params) do
+    FbosConfig.changeset(fbos_config || fbos_config(), params)
+    |> Repo.insert_or_update!()
+  end
+
   ## End FbosConfig
 
   ## Begin FirmwareConfig

farmbot_core/lib/asset_workers/fbos_config_worker.ex

@@ -5,6 +5,12 @@ defimpl Farmbot.AssetWorker, for: Farmbot.Asset.FbosConfig do
   alias Farmbot.Asset.FbosConfig
   alias Farmbot.BotState
 
+  import Farmbot.Config,
+    only: [
+      get_config_value: 3,
+      update_config_value: 4
+    ]
+
   def preload(%FbosConfig{}), do: []
 
   def start_link(%FbosConfig{} = fbos_config, _args) do
@@ -20,7 +26,6 @@ defimpl Farmbot.AssetWorker, for: Farmbot.Asset.FbosConfig do
     :ok = BotState.set_config_value(:auto_sync, fbos_config.auto_sync)
     :ok = BotState.set_config_value(:beta_opt_in, fbos_config.beta_opt_in)
     :ok = BotState.set_config_value(:disable_factory_reset, fbos_config.disable_factory_reset)
-    :ok = BotState.set_config_value(:firmware_hardware, fbos_config.firmware_hardware)
     :ok = BotState.set_config_value(:firmware_input_log, fbos_config.firmware_input_log)
     :ok = BotState.set_config_value(:firmware_output_log, fbos_config.firmware_output_log)
     :ok = BotState.set_config_value(:network_not_found_timer, fbos_config.network_not_found_timer)
@@ -28,6 +33,16 @@ defimpl Farmbot.AssetWorker, for: Farmbot.Asset.FbosConfig do
     :ok = BotState.set_config_value(:sequence_body_log, fbos_config.sequence_body_log)
     :ok = BotState.set_config_value(:sequence_complete_log, fbos_config.sequence_complete_log)
     :ok = BotState.set_config_value(:sequence_init_log, fbos_config.sequence_init_log)
+    _ = handle_firmware_hardware(fbos_config.firmware_hardware)
     {:noreply, fbos_config}
   end
+
+  def handle_firmware_hardware(target_hardware) do
+    current_hardware = get_config_value(:string, "settings", "firmware_hardware")
+
+    if current_hardware != target_hardware do
+      Logger.debug("Updating firmware_hardware from #{current_hardware} to #{target_hardware}")
+      update_config_value(:string, "settings", "firmware_hardware", target_hardware)
+    end
+  end
 end

farmbot_core/priv/config/migrations/20181204173345_add_firmware_flash_setting.exs

@@ -0,0 +1,10 @@
+defmodule Farmbot.Config.Repo.Migrations.AddFirmwareFlashSetting do
+  use Ecto.Migration
+  import Farmbot.Config, only: [update_config_value: 4]
+  import Farmbot.Config.MigrationHelpers
+
+  def change do
+    update_config_value(:string, "settings", "firmware_hardware", nil)
+    create_settings_config("firmware_needs_flash", :bool, true)
+  end
+end

farmbot_firmware/lib/farmbot_firmware.ex

@@ -166,7 +166,7 @@ defmodule Farmbot.Firmware do
     args = Keyword.put(args, :handle_gcode, fun)
 
     with {:ok, pid} <- GenServer.start_link(transport, args) do
-      Process.link(pid)
+      Process.monitor(pid)
       Logger.debug("Starting Firmware: #{inspect(args)}")
 
       state = %State{
@@ -182,6 +182,14 @@ defmodule Farmbot.Firmware do
     end
   end
 
+  def terminate(reason, state) do
+    for {pid, _code} <- state.command_queue, do: send(pid, reason)
+  end
+
+  def handle_info({:DOWN, _ref, :process, pid, reason}, %{transport_pid: pid} = state) do
+    {:stop, reason, state}
+  end
+
   # @spec handle_info(:timeout, state) :: {:noreply, state}
   def handle_info(:timeout, %{configuration_queue: [code | rest]} = state) do
     Logger.debug("Starting next configuration code: #{inspect(code)}")
@@ -192,8 +200,8 @@ defmodule Farmbot.Firmware do
         side_effects(new_state, :handle_output_gcode, [{state.tag, code}])
         {:noreply, new_state}
 
-      {:error, _} ->
-        {:noreply, state, @error_timeout_ms}
+      error ->
+        {:stop, error, state}
     end
   end
 
@@ -204,8 +212,8 @@ defmodule Farmbot.Firmware do
         side_effects(new_state, :handle_output_gcode, [{state.tag, code}])
         {:noreply, new_state}
 
-      {:error, _} ->
-        {:noreply, state, @error_timeout_ms}
+      error ->
+        {:stop, error, state}
     end
   end
 

farmbot_firmware/lib/farmbot_firmware/command.ex

@@ -43,6 +43,9 @@ defmodule Farmbot.Firmware.Command do
       {_, {:report_emergency_lock, []}} ->
         {:error, :emergency_lock}
 
+      {:error, reason} ->
+        {:error, reason}
+
       {_tag, _report} ->
         wait_for_command_result(tag, code, retries, err)
     end

farmbot_firmware/lib/farmbot_firmware/request.ex

@@ -65,6 +65,9 @@ defmodule Farmbot.Firmware.Request do
       {_, {:report_emergency_lock, []}} ->
         {:error, :emergency_lock}
 
+      {:error, reason} ->
+        {:error, reason}
+
       {_tag, report} ->
         wait_for_request_result_process(report, tag, code, result)
     after

farmbot_firmware/lib/farmbot_firmware/transports/uart_transport.ex

@@ -39,8 +39,12 @@ defmodule Farmbot.Firmware.UARTTransport do
   end
 
   def handle_call(code, _from, state) do
-    str = GCODE.encode(code)
-    r = UART.write(state.uart, str)
-    {:reply, r, state}
+    try do
+      str = GCODE.encode(code)
+      r = UART.write(state.uart, str)
+      {:reply, r, state}
+    rescue
+      error -> {:reply, {:error, error}, state}
+    end
   end
 end

farmbot_os/lib/tty_detector.ex

@@ -3,9 +3,9 @@ defmodule Farmbot.TTYDetector do
   require Logger
   alias Circuits.UART
 
-  import Farmbot.Config, only: [get_config_value: 3]
+  import Farmbot.Config, only: [get_config_value: 3, update_config_value: 4]
 
-  @expected_names ["ttyACM0"]
+  @expected_names ["ttyACM1"]
   @error_ms 5000
 
   def start_link(args) do
@@ -13,7 +13,11 @@ defmodule Farmbot.TTYDetector do
   end
 
   def init([]) do
-    {:ok, %{device: nil, needs_flash: false, open: false}, 0}
+    {:ok, %{device: nil, open: false, version: nil}, 0}
+  end
+
+  def terminate(_, _) do
+    System.cmd("killall", ["-9", "avrdude"], into: IO.stream(:stdio, :line))
   end
 
   def handle_info(:timeout, %{device: nil} = state) do
@@ -25,31 +29,19 @@ defmodule Farmbot.TTYDetector do
     end
   end
 
-  def handle_info(:timeout, %{device: device, needs_flash: true} = state) do
-    dir = Application.app_dir(:farmbot_core, ["priv"])
-    case get_config_value(:string, "settings", "firmware_hardware") do
-      "arduino" -> flash_fw(Path.join(dir, "arduino_firmware.hex"), state)
-      "farmduino" -> flash_fw(Path.join(dir, "farmduino.hex"), state)
-      "farmduino_k14" -> flash_fw(Path.join(dir, "farmduino_k14.hex"), state)
-      nil -> {:noreply, state, @error_ms}
-      other ->
-        Logger.error "Unknown arduino firmware #{other}"
-        {:stop, {:unknown_firmware, other}, state}
+  def handle_info(:timeout, %{device: _device, open: false} = state) do
+    case get_config_value(:bool, "settings", "firmware_needs_flash") do
+      true -> handle_flash(state)
+      false -> handle_open(state)
     end
   end
 
-  def handle_info(:timeout, %{device: device, needs_flash: false, open: false} = state) do
-    opts = [
-      device: device,
-      transport: Farmbot.Firmware.UARTTransport,
-      side_effects: Farmbot.Core.FirmwareSideEffects
-    ]
-    case Farmbot.Firmware.start_link(opts) do
-      {:ok, pid} -> 
-        Process.monitor(pid)
-        {:noreply, %{state | open: true}}
-      error -> 
-        {:stop, error, state}
+  def handle_info(:timeout, %{device: _device, open: true, version: nil} = state) do
+    case Farmbot.Firmware.request {:software_version_read, []} do
+    {:ok, {_, {:report_software_version, [version]}}} ->
+      {:noreply, %{state | version: version}}
+    _ ->
+      {:noreply, state, 5000}
     end
   end
 
@@ -57,7 +49,7 @@ defmodule Farmbot.TTYDetector do
     {:stop, reason, state}
   end
 
-  def handle_continue([{name, _} | rest], %{device: nil} = state)
+  def handle_continue([{name, _} | _rest], %{device: nil} = state)
     when name in @expected_names do
     {:noreply, %{state | device: name}, 0}
   end
@@ -70,12 +62,49 @@ defmodule Farmbot.TTYDetector do
     {:noreply, state, @error_ms}
   end
 
+  defp handle_flash(state) do
+    dir = Application.app_dir(:farmbot_core, ["priv"])
+    case get_config_value(:string, "settings", "firmware_hardware") do
+      "arduino" -> flash_fw(Path.join(dir, "arduino_firmware.hex"), state)
+      "farmduino" -> flash_fw(Path.join(dir, "farmduino.hex"), state)
+      "farmduino_k14" -> flash_fw(Path.join(dir, "farmduino_k14.hex"), state)
+      nil -> {:noreply, state, @error_ms}
+      other ->
+        Logger.error "Unknown arduino firmware #{other}"
+        {:stop, {:unknown_firmware, other}, state}
+    end
+  end
+
+  defp handle_open(state) do
+    opts = [
+      device: state.device,
+      transport: Farmbot.Firmware.UARTTransport,
+      side_effects: Farmbot.Core.FirmwareSideEffects
+    ]
+    case Farmbot.Firmware.start_link(opts) do
+      {:ok, pid} ->
+        # This might cause some sort of race condition.
+        hw = get_config_value(:string, "settings", "firmware_hardware")
+        Farmbot.Asset.update_fbos_config!(%{
+          firmware_path: state.device,
+          firmware_hardware: hw
+        })
+        :ok = Farmbot.BotState.set_config_value(:firmware_hardware, hw)
+        Process.monitor(pid)
+        {:noreply, %{state | open: true, version: nil}, 5000}
+      error ->
+        {:stop, error, state}
+    end
+  end
+
   defp flash_fw(fw_file, state) do
     args = ~w"-q -q -patmega2560 -cwiring -P#{dev(state.device)} -b115200 -D -V -Uflash:w:#{fw_file}:i"
     opts = [stderr_to_stdout: true, into: IO.stream(:stdio, :line)]
     res = System.cmd("avrdude", args, opts)
     case res do
-      {_, 0} -> {:noreply, %{state | needs_flash: false}, 0}
+      {_, 0} ->
+        update_config_value(:bool, "settings", "firmware_needs_flash", false)
+        {:noreply, state, 0}
       _ ->
         Logger.error("firmware flash failed")
         {:noreply, state, @error_ms}