Merge branch 'staging' into remove-dnsmasq

CHANGELOG.md

@@ -8,6 +8,9 @@
  * Unit test additions (+2.7% coverage :tada:)
  * Updates to build instructions for third party developers
  * Bug fix for criteria-based groups that have only one filter criteria.
+ * Bug fix for express bots involving timeout during remote firmware flash
+ * Remove VCR again (for now)
+ * Increase farmware timeout to 20 minutes (use at own risk)
 
 # 9.2.1
 

FEATURE_MIN_VERSIONS.json

@@ -5,6 +5,7 @@
   "assertion_block": "8.0.0",
   "backscheduled_regimens": "6.4.0",
   "change_ownership": "6.3.0",
+  "criteria_groups": "9.2.2",
   "diagnostic_dumps": "6.4.4",
   "endstop_invert": "6.4.1",
   "express_k10": "8.0.0",

VERSION

@@ -1 +1 @@
-9.2.2-rc8
+9.2.2

docs/target_development/releasing_target_firmware.md

@@ -20,15 +20,22 @@ string.
 ```bash
 cd $FARMBOT_OS_ROOT_DIRECTORY
 git checkout staging
-git fetch --all && git reset --hard origin/staging
+
+# Ensure you don't accidentally publish local changes
+# that have not gone through CI:
+git fetch --all
+git reset --hard origin/staging
+
 # update the CHANGELOG, but DO NOT put the `rc`
 # on the semver string.
 $EDITOR CHANGELOG.md
-echo 10.5.6-rc30 > VERSION
+
-git add CHANGELOG.md VERSION
+echo 1.2.3-rc4 > VERSION
-git commit -m "Release v10.5.6-rc30"
+
-git tag v$(cat VERSION)
+git add -A
-git push origin staging v$(cat VERSION)
+git commit -am "Release v10.5.6-rc30"
+git tag v1.2.3-rc4
+git push origin v1.2.3-rc4
 ```
 
 or call the helper script:

farmbot_core/lib/farmbot_core.ex

@@ -30,7 +30,7 @@ defmodule FarmbotCore do
       {FarmbotFirmware, transport: FarmbotFirmware.StubTransport, side_effects: FarmbotCore.FirmwareSideEffects},
       FarmbotCeleryScript.Scheduler
     ]
-    config = Application.get_env(:farmbot_ext, __MODULE__) || []
+    config = (Application.get_env(:farmbot_ext, __MODULE__) || [])
-Keyword.get(config, :children, default)
+    Keyword.get(config, :children, default)
   end
 end

farmbot_core/lib/farmbot_core/asset.ex

@@ -323,8 +323,7 @@ defmodule FarmbotCore.Asset do
         # the DB / API.
         sorted = CriteriaRetriever.run(point_group)
           |> sort_points(sort_by || "xy_ascending")
-          |> Enum.map(&Map.fetch!(&1, :id))
+          |> Enum.map(fn point -> point.id end)
-
         %{ point_group | point_ids: sorted }
       other ->
         # Swallow all other errors

farmbot_core/lib/farmbot_core/asset/criteria_retriever.ex

@@ -22,7 +22,7 @@ defmodule FarmbotCore.Asset.CriteriaRetriever do
   # We will not query any string/numeric fields other than these.
   # Updating the PointGroup / Point models may require an update
   # to these fields.
-  @numberic_fields ["radius", "x", "y", "z"]
+  @numberic_fields ["radius", "x", "y", "z", "pullout_direction"]
   @string_fields ["name", "openfarm_slug", "plant_stage", "pointer_type"]
 
   @doc """
@@ -114,7 +114,8 @@ defmodule FarmbotCore.Asset.CriteriaRetriever do
   def finalize({fragments, criteria}) do
     x = Enum.join(fragments, " AND ")
     sql = "SELECT id FROM points WHERE #{x}"
-    {:ok, query} = Repo.query(sql, List.flatten(criteria))
+    query_params = List.flatten(criteria)
+    {:ok, query} = Repo.query(sql, query_params)
     %Sqlite.DbConnection.Result{ rows: rows } = query
     List.flatten(rows)
   end
@@ -140,7 +141,9 @@ defmodule FarmbotCore.Asset.CriteriaRetriever do
       op = day_criteria["op"] || "<"
       time = Timex.shift(Timex.now(), days: -1 * days)
 
-      { pg, accum ++ [{"created_at", op, time}] }
+      inverted_op = if op == ">" do "<" else ">" end
+
+      { pg, accum ++ [{"created_at", inverted_op, time}] }
     end
   end
 

farmbot_core/lib/farmbot_core/asset/point.ex

@@ -13,20 +13,21 @@ defmodule FarmbotCore.Asset.Point do
       foreign_key: :asset_local_id
     )
 
+    field(:discarded_at, :utc_datetime)
+    field(:gantry_mounted, :boolean)
     field(:meta, :map)
+    field(:monitor, :boolean, default: true)
     field(:name, :string)
     field(:openfarm_slug, :string)
     field(:plant_stage, :string)
     field(:planted_at, :utc_datetime)
     field(:pointer_type, :string)
+    field(:pullout_direction, :integer)
     field(:radius, :float)
+    field(:tool_id, :integer)
     field(:x, :float)
     field(:y, :float)
     field(:z, :float)
-    field(:tool_id, :integer)
-    field(:discarded_at, :utc_datetime)
-    field(:gantry_mounted, :boolean)
-    field(:monitor, :boolean, default: true)
     timestamps()
   end
 
@@ -42,6 +43,8 @@ defmodule FarmbotCore.Asset.Point do
       tool_id: point.tool_id,
       discarded_at: point.discarded_at,
       gantry_mounted: point.gantry_mounted,
+      openfarm_slug: point.openfarm_slug,
+      pullout_direction: point.pullout_direction,
       x: point.x,
       y: point.y,
       z: point.z
@@ -51,22 +54,24 @@ defmodule FarmbotCore.Asset.Point do
   def changeset(point, params \\ %{}) do
     point
     |> cast(params, [
+      :created_at,
+      :discarded_at,
+      :gantry_mounted,
       :id,
       :meta,
+      :monitor,
       :name,
       :plant_stage,
       :planted_at,
       :pointer_type,
+      :pullout_direction,
+      :openfarm_slug,
       :radius,
+      :tool_id,
+      :updated_at,
       :x,
       :y,
       :z,
-      :tool_id,
-      :gantry_mounted,
-      :discarded_at,
-      :monitor,
-      :created_at,
-      :updated_at
     ])
     |> validate_required([])
   end

farmbot_core/lib/farmbot_core/firmware_open_task.ex

@@ -77,21 +77,21 @@ defmodule FarmbotCore.FirmwareOpenTask do
         {:noreply, increment_attempts(%{state | timer: timer})}
 
       firmware_hardware == "none" && needs_open? ->
-        FarmbotCore.Logger.debug 3, "Firmware needs to be closed"
+        FarmbotCore.Logger.debug 3, "Closing firmware..."
         unswap_transport()
         Config.update_config_value(:bool, "settings", "firmware_needs_open", false)
         timer = Process.send_after(self(), :open, 5000)
         {:noreply, %{state | timer: timer, attempts: 0}}
 
       needs_open? ->
-        FarmbotCore.Logger.debug 3, "Firmware needs to be opened"
+        FarmbotCore.Logger.debug 3, "Opening firmware..."
         case swap_transport(firmware_path) do
           :ok ->
             Config.update_config_value(:bool, "settings", "firmware_needs_open", false)
             timer = Process.send_after(self(), :open, 5000)
             {:noreply, %{state | timer: timer, attempts: 0}}
           other ->
-            FarmbotCore.Logger.debug 3, "Firmware failed to open: #{inspect(other)}"
+            FarmbotCore.Logger.debug 3, "Not ready to open yet, will retry in 5s (#{inspect(other)})"
             timer = Process.send_after(self(), :open, 5000)
             {:noreply, %{state | timer: timer, attempts: 0}}
         end

farmbot_core/priv/asset/migrations/20200411204400_add_pullout_direction_to_point.exs

@@ -0,0 +1,12 @@
+defmodule FarmbotCore.Asset.Repo.Migrations.AddPulloutDirectionToPoint do
+  use Ecto.Migration
+
+  def change do
+    alter table("points") do
+      # 0 means "NONE"
+      add(:pullout_direction, :integer, default: 0)
+    end
+
+    execute("UPDATE points SET updated_at = \"1970-11-07 16:52:31.618000\"")
+  end
+end

farmbot_core/test/asset/criteria_retriever_test.exs

@@ -13,7 +13,7 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
 
   @fake_point_group %PointGroup{
     criteria: %{
-      "day" => %{"op" => "<", "days_ago" => 4},
+      "day" => %{"op" => ">", "days_ago" => 4},
       "string_eq" => %{
         "openfarm_slug" => ["five", "nine"],
         "meta.created_by" => ["plant-detection"]
@@ -482,7 +482,6 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
         "string_eq" => %{}
       },
       id: 201,
-      local_id: "30856f5e-1f97-4e18-b5e0-84dc7cd9bbf0",
       name: "Test (Broke?)",
       point_ids: whitelist,
       sort_type: "xy_ascending",
@@ -494,4 +493,70 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
     assert Enum.count(whitelist) == Enum.count(results)
     Enum.map(whitelist, fn id -> assert Enum.member?(results, id) end)
   end
+
+  test "edge case: Filter by crop type" do
+    Repo.delete_all(PointGroup)
+    Repo.delete_all(Point)
+    ok = point!(%{id: 1, pointer_type: "Plant", openfarm_slug: "spinach"})
+    point!(%{id: 2, pointer_type: "Plant", openfarm_slug: "beetroot"})
+    point!(%{id: 3, pointer_type: "Weed", openfarm_slug: "thistle"})
+    point!(%{id: 4, pointer_type: "Weed", openfarm_slug: "spinach"})
+
+    pg = %PointGroup{
+      :id => 241,
+      :point_ids => [],
+      :criteria => %{
+        "day" => %{
+          "op" => "<",
+          "days_ago" => 0
+        },
+        "string_eq" => %{
+          "pointer_type" => ["Plant"],
+          "openfarm_slug" => ["spinach"]
+        },
+        "number_eq" => %{},
+        "number_lt" => %{},
+        "number_gt" => %{}
+      }
+    }
+
+    ids = CriteriaRetriever.run(pg) |> Enum.map(fn p -> p.id end)
+    assert Enum.member?(ids, ok.id)
+    assert Enum.count(ids) == 1
+  end
+
+  test "edge case: Filter by slot direction" do
+    Repo.delete_all(PointGroup)
+    Repo.delete_all(Point)
+
+    ok = point!(%{id: 1, pointer_type: "ToolSlot", pullout_direction: 3})
+    point!(%{id: 2, pointer_type: "Weed", pullout_direction: 3})
+    point!(%{id: 3, pointer_type: "ToolSlot", pullout_direction: 4})
+    point!(%{id: 4, pointer_type: "GenericPointer", pullout_direction: 2})
+
+    pg = %PointGroup{
+      :id => 242,
+      :name => "Filter by slot direction",
+      :point_ids => [],
+      :sort_type => "xy_ascending",
+      :criteria => %{
+        "day" => %{
+          "op" => "<",
+          "days_ago" => 0
+        },
+        "string_eq" => %{
+          "pointer_type" => ["ToolSlot"]
+        },
+        "number_eq" => %{
+          "pullout_direction" => [3]
+        },
+        "number_lt" => %{},
+        "number_gt" => %{}
+      }
+    }
+
+    ids = CriteriaRetriever.run(pg) |> Enum.map(fn p -> p.id end)
+    assert Enum.member?(ids, ok.id)
+    assert Enum.count(ids) == 1
+  end
 end

farmbot_ext/test/farmbot_ext/amqp/auto_sync_channel_test.exs

@@ -78,7 +78,7 @@ defmodule AutoSyncChannelTest do
     # Helpers.expect_log("Failed to connect to AutoSync channel: :whatever")
     # Helpers.expect_log("Disconnected from AutoSync channel: :normal")
     pid = generate_pid()
-    IO.puts("   =  = = ==RICK: This test blinks and you should fix it.")
+    IO.puts("   =====RICK: This test blinks and you should fix it.")
     assert %{chan: nil, conn: nil, preloaded: true} == AutoSyncChannel.network_status(pid)
     GenServer.stop(pid, :normal)
   end
@@ -155,5 +155,6 @@ defmodule AutoSyncChannelTest do
     end)
 
     Helpers.wait_for(pid)
+    Process.sleep(1000)
   end
 end

farmbot_ext/test/test_helper.exs

@@ -27,7 +27,7 @@ defmodule Helpers do
   # Maybe I could use `start_supervised`?
   # https://hexdocs.pm/ex_unit/ExUnit.Callbacks.html#start_supervised/2
 
-  @wait_time 60
+  @wait_time 180
   # Base case: We have a pid
   def wait_for(pid) when is_pid(pid), do: check_on_mbox(pid)
   # Failure case: We failed to find a pid for a module.

farmbot_firmware/lib/farmbot_firmware.ex

@@ -78,28 +78,6 @@ defmodule FarmbotFirmware do
 
   and reply with `:ok | {:error, term()}`
 
-  # VCR
-
-  This server can save all the input and output gcodes to a text file for
-  further external analysis or playback later.
-
-  ## Using VCR mode
-
-  The server can be started in VCR mode by doing:
-
-      FarmbotFirmware.start_link([transport: FarmbotFirmware.StubTransport, vcr_path: "/tmp/vcr.txt"], [])
-
-  or can be started at runtime:
-
-      FarmbotFirmware.enter_vcr_mode(firmware_server, "/tmp/vcr.txt")
-
-  in either case the VCR recording needs to be stopped:
-
-      FarmbotFirmware.exit_vcr_mode(firmware_server)
-
-  VCRs can later be played back:
-
-      FarmbotFirmware.VCR.playback!("/tmp/vcr.txt")
   """
   use GenServer
   require Logger
@@ -129,7 +107,6 @@ defmodule FarmbotFirmware do
     :command_queue,
     :caller_pid,
     :current,
-    :vcr_fd,
     :reset,
     :reset_pid
   ]
@@ -146,7 +123,6 @@ defmodule FarmbotFirmware do
           command_queue: [{pid(), GCODE.t()}],
           caller_pid: nil | pid,
           current: nil | GCODE.t(),
-          vcr_fd: nil | File.io_device(),
           reset: module(),
           reset_pid: nil | pid()
         }
@@ -209,22 +185,6 @@ defmodule FarmbotFirmware do
     GenServer.call(server, :reset)
   end
 
-  @doc """
-  Sets the Firmware server to record input and output GCODES
-  to a pair of text files.
-  """
-  def enter_vcr_mode(server \\ __MODULE__, tape_path) do
-    GenServer.call(server, {:enter_vcr_mode, tape_path})
-  end
-
-  @doc """
-  Sets the Firmware server to stop recording input and output
-  GCODES.
-  """
-  def exit_vcr_mode(server \\ __MODULE__) do
-    GenServer.cast(server, :exit_vcr_mode)
-  end
-
   @doc """
   Starting the Firmware server requires at least:
   * `:transport` - a module implementing the Transport GenServer behaviour.
@@ -252,18 +212,6 @@ defmodule FarmbotFirmware do
     # probably?
     reset = Keyword.get(args, :reset) || FarmbotFirmware.NullReset
 
-    vcr_fd =
-      case Keyword.get(args, :vcr_path) do
-        nil ->
-          nil
-
-        tape_path ->
-          {:ok, vcr_fd} =
-            File.open(tape_path, [:binary, :append, :exclusive, :write])
-
-          vcr_fd
-      end
-
     # Add an anon function that transport implementations should call.
     fw = self()
     fun = fn {_, _} = code -> GenServer.cast(fw, code) end
@@ -279,11 +227,10 @@ defmodule FarmbotFirmware do
       reset: reset,
       reset_pid: nil,
       command_queue: [],
-      configuration_queue: [],
+      configuration_queue: []
-      vcr_fd: vcr_fd
     }
 
-    send(self(), :timeout)
+    send_timeout_self()
     {:ok, state}
   end
 
@@ -347,7 +294,7 @@ defmodule FarmbotFirmware do
           ]
         } = state
       ) do
-    case GenServer.call(state.transport_pid, {tag, code}) do
+    case call_transport(state.transport_pid, {tag, code}, 297) do
       :ok ->
         new_state = %{
           state
@@ -358,7 +305,6 @@ defmodule FarmbotFirmware do
         }
 
         _ = side_effects(new_state, :handle_output_gcode, [{state.tag, code}])
-        _ = vcr_write(state, :out, {state.tag, code})
 
         {:noreply, new_state}
 
@@ -370,11 +316,10 @@ defmodule FarmbotFirmware do
   def handle_info(:timeout, %{configuration_queue: [code | rest]} = state) do
     # Logger.debug("Starting next configuration code: #{inspect(code)}")
 
-    case GenServer.call(state.transport_pid, {state.tag, code}) do
+    case call_transport(state.transport_pid, {state.tag, code}, 319) do
       :ok ->
         new_state = %{state | current: code, configuration_queue: rest}
         _ = side_effects(new_state, :handle_output_gcode, [{state.tag, code}])
-        _ = vcr_write(state, :out, {state.tag, code})
         {:noreply, new_state}
 
       error ->
@@ -389,7 +334,6 @@ defmodule FarmbotFirmware do
     for {pid, _code} <- state.command_queue,
         do: send(pid, {state.tag, {:report_busy, []}})
 
-    # Logger.debug "Got checkup message when current command still executing"
     {:noreply, state}
   end
 
@@ -400,7 +344,7 @@ defmodule FarmbotFirmware do
     for {pid, _code} <- state.command_queue,
         do: send(pid, {state.tag, {:report_busy, []}})
 
-    case GenServer.call(state.transport_pid, {tag, code}) do
+    case call_transport(state.transport_pid, {tag, code}, 348) do
       :ok ->
         new_state = %{
           state
@@ -411,7 +355,6 @@ defmodule FarmbotFirmware do
         }
 
         _ = side_effects(new_state, :handle_output_gcode, [{state.tag, code}])
-        _ = vcr_write(state, :out, {state.tag, code})
         for {pid, _code} <- rest, do: send(pid, {state.tag, {:report_busy, []}})
 
         {:noreply, new_state}
@@ -438,7 +381,12 @@ defmodule FarmbotFirmware do
       true = Process.demonitor(state.transport_ref)
     end
 
-    :ok = GenServer.stop(state.transport_pid, :normal)
+    if is_pid(state.transport_pid) do
+      Logger.debug("closing transport")
+      :ok = GenServer.stop(state.transport_pid, :normal)
+    else
+      Logger.debug("No tranport pid found. Nothing to close")
+    end
 
     next_state =
       goto(
@@ -473,7 +421,7 @@ defmodule FarmbotFirmware do
 
     next_state = %{state | transport: module, transport_args: transport_args}
 
-    send(self(), :timeout)
+    send_timeout_self()
     {:reply, :ok, next_state}
   end
 
@@ -485,16 +433,6 @@ defmodule FarmbotFirmware do
     {:reply, {:error, s}, state}
   end
 
-  def handle_call({:enter_vcr_mode, tape_path}, _from, state) do
-    with {:ok, vcr_fd} <-
-           File.open(tape_path, [:binary, :append, :exclusive, :write]) do
-      {:reply, :ok, %{state | vcr_fd: vcr_fd}}
-    else
-      error ->
-        {:reply, error, state}
-    end
-  end
-
   def handle_call({tag, {kind, args}}, from, state) do
     handle_command({tag, {kind, args}}, from, state)
   end
@@ -521,7 +459,7 @@ defmodule FarmbotFirmware do
     for {pid, _code} <- state.command_queue,
         do: send(pid, {state.tag, {:report_emergency_lock, []}})
 
-    send(self(), :timeout)
+    send_timeout_self()
 
     {:reply, {:ok, tag},
      %{state | command_queue: [{pid, code}], configuration_queue: []}}
@@ -533,7 +471,7 @@ defmodule FarmbotFirmware do
         {pid, _ref},
         state
       ) do
-    send(self(), :timeout)
+    send_timeout_self()
 
     {:reply, {:ok, tag},
      %{state | command_queue: [{pid, code}], configuration_queue: []}}
@@ -550,14 +488,14 @@ defmodule FarmbotFirmware do
 
     case {new_state.status, state.current} do
       {:idle, nil} ->
-        send(self(), :timeout)
+        send_timeout_self()
         {:reply, {:ok, tag}, new_state}
 
       # Don't do any flow control if state is emergency_lock.
       # This allows a transport to decide
       # if a command should be blocked or not.
       {:emergency_lock, _} ->
-        send(self(), :timeout)
+        send_timeout_self()
         {:reply, {:ok, tag}, new_state}
 
       _unknown ->
@@ -565,15 +503,9 @@ defmodule FarmbotFirmware do
     end
   end
 
-  def handle_cast(:exit_vcr_mode, state) do
-    state.vcr_fd && File.close(state.vcr_fd)
-    {:noreply, %{state | vcr_fd: nil}}
-  end
-
   # Extracts tag
   def handle_cast({tag, {_, _} = code}, state) do
     _ = side_effects(state, :handle_input_gcode, [{tag, code}])
-    _ = vcr_write(state, :in, {tag, code})
     handle_report(code, %{state | tag: tag})
   end
 
@@ -585,7 +517,7 @@ defmodule FarmbotFirmware do
     if state.caller_pid, do: send(state.caller_pid, {state.tag, code})
     for {pid, _code} <- state.command_queue, do: send(pid, code)
 
-    send(self(), :timeout)
+    send_timeout_self()
     {:noreply, goto(%{state | current: nil, caller_pid: nil}, :emergency_lock)}
   end
 
@@ -598,10 +530,7 @@ defmodule FarmbotFirmware do
     handle_report({:report_no_config, []}, state)
   end
 
-  def handle_report(
+  def handle_report({:report_idle, []}, %{status: :boot} = state) do
-        {:report_idle, []},
-        %{status: :boot} = state
-      ) do
     Logger.info("ARDUINO STARTUP COMPLETE (idle) transport=#{state.transport}")
     handle_report({:report_no_config, []}, state)
   end
@@ -648,7 +577,7 @@ defmodule FarmbotFirmware do
         do: to_process ++ [{:command_movement_find_home, [:x]}],
         else: to_process
 
-    send(self(), :timeout)
+    send_timeout_self()
 
     {:noreply,
      goto(%{state | tag: tag, configuration_queue: to_process}, :configuration)}
@@ -684,7 +613,7 @@ defmodule FarmbotFirmware do
 
     side_effects(state, :handle_busy, [false])
     side_effects(state, :handle_idle, [true])
-    send(self(), :timeout)
+    send_timeout_self()
     {:noreply, goto(%{state | caller_pid: nil, current: nil}, :idle)}
   end
 
@@ -705,7 +634,7 @@ defmodule FarmbotFirmware do
 
     new_state = %{state | current: nil, caller_pid: nil}
     side_effects(state, :handle_busy, [false])
-    send(self(), :timeout)
+    send_timeout_self()
     {:noreply, goto(new_state, :idle)}
   end
 
@@ -739,7 +668,7 @@ defmodule FarmbotFirmware do
         do: send(pid, {state.tag, {:report_busy, []}})
 
     side_effects(state, :handle_busy, [false])
-    send(self(), :timeout)
+    send_timeout_self()
     {:noreply, %{state | caller_pid: nil, current: nil}}
   end
 
@@ -761,7 +690,7 @@ defmodule FarmbotFirmware do
     for {pid, _code} <- state.command_queue,
         do: send(pid, {state.tag, {:report_busy, []}})
 
-    send(self(), :timeout)
+    send_timeout_self()
     {:noreply, %{state | caller_pid: nil, current: nil}}
   end
 
@@ -796,7 +725,7 @@ defmodule FarmbotFirmware do
     to_process = [{:parameter_write, param}]
     side_effects(state, :handle_parameter_value, [param])
     side_effects(state, :handle_parameter_calibration_value, [param])
-    send(self(), :timeout)
+    send_timeout_self()
 
     {:noreply,
      goto(
@@ -996,30 +925,28 @@ defmodule FarmbotFirmware do
   defp side_effects(%{side_effects: m}, function, args),
     do: apply(m, function, args)
 
-  @spec vcr_write(state, :in | :out, GCODE.t()) :: :ok
+  defp send_timeout_self do
-  defp vcr_write(%{vcr_fd: nil}, _direction, _code), do: :ok
+    send(self(), :timeout)
+  end
 
-  defp vcr_write(state, :in, code), do: vcr_write(state, "<", code)
+  defp call_transport(nil, args, where) do
+    msg =
+      "#{inspect(where)} Firmware not ready. A restart may be required if not already started (#{
+        inspect(args)
+      })"
 
-  defp vcr_write(state, :out, code), do: vcr_write(state, "\n>", code)
+    Logger.debug(msg)
+    {:error, msg}
+  end
 
-  defp vcr_write(state, direction, code) do
+  defp call_transport(transport_pid, args, where) do
-    data = GCODE.encode(code)
+    # Returns :ok
-    time = :os.system_time(:second)
+    response = GenServer.call(transport_pid, args)
 
-    current_data =
+    unless response == :ok do
-      if state.current do
+      Logger.debug("#{inspect(where)}: returned #{inspect(response)}")
-        GCODE.encode({state.tag, state.current})
+    end
-      else
-        "nil"
-      end
 
-    state_data =
+    response
-      "#{state.status} | #{current_data} | #{inspect(state.caller_pid)}"
-
-    IO.write(
-      state.vcr_fd,
-      direction <> " #{time} " <> data <> " state=" <> state_data <> "\n"
-    )
   end
 end

farmbot_os/config/config.exs

@@ -100,9 +100,7 @@ if Mix.target() == :host do
 else
   import_config("target/#{Mix.env()}.exs")
 
-  if File.exists?("config/target/#{Mix.target()}.exs") do
+  import_config("target/#{Mix.target()}.exs")
-    import_config("target/#{Mix.target()}.exs")
-  end
 end
 
 if Mix.env() == :test do

farmbot_os/config/target/rpi.exs

@@ -10,3 +10,7 @@ config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [
     FarmbotOS.Platform.Target.RTCWorker
   ]
+
+# :farmbot_firmware, FarmbotFirmware changes too much.
+# Needed one that would stay stable, so I duplicated it here:
+config :farmbot, FarmbotOS.SysCalls.FlashFirmware, gpio: Circuits.GPIO

farmbot_os/config/target/rpi0.exs

@@ -8,6 +8,10 @@ config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 50
 config :farmbot_firmware, FarmbotFirmware,
   reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
 
+# :farmbot_firmware, FarmbotFirmware changes too much.
+# Needed one that would stay stable, so I duplicated it here:
+config :farmbot, FarmbotOS.SysCalls.FlashFirmware, gpio: Circuits.GPIO
+
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [
     FarmbotOS.Platform.Target.RTCWorker

farmbot_os/lib/avrdude.ex

@@ -17,7 +17,6 @@ defmodule Avrdude do
 
     _ = File.stat!(hex_path)
 
-    # STEP 1: Is the UART in use?
     args = [
       "-patmega2560",
       "-cwiring",
@@ -25,16 +24,23 @@ defmodule Avrdude do
       "-b#{@uart_speed}",
       "-D",
       "-V",
+      "-v",
       "-Uflash:w:#{hex_path}:i"
     ]
 
-    # call the function for resetting the line before executing avrdude.
+    FarmbotCore.Logger.info(3, "Writing firmware to MCU...")
+
     call_reset_fun(reset_fun)
 
-    MuonTrap.cmd("avrdude", args,
+    result = MuonTrap.cmd("avrdude", args, stderr_to_stdout: true)
-      into: IO.stream(:stdio, :line),
+
-      stderr_to_stdout: true
+    if is_tuple(result) do
-    )
+      {a, exit_code} = result
+      FarmbotCore.Logger.info(3, inspect(a))
+      FarmbotCore.Logger.info(3, "Exit code #{exit_code}")
+    end
+
+    result
   end
 
   def call_reset_fun(reset_fun) do

farmbot_os/lib/farmbot_os/sys_calls/farmware.ex

@@ -4,7 +4,7 @@ defmodule FarmbotOS.SysCalls.Farmware do
   require FarmbotCore.Logger
   alias FarmbotCore.{Asset, AssetSupervisor, FarmwareRuntime}
   alias FarmbotExt.API.ImageUploader
-  @farmware_timeout 60_000
+  @farmware_timeout 1_200_000
 
   def update_farmware(farmware_name) do
     with {:ok, installation} <- lookup_installation(farmware_name) do
@@ -56,9 +56,9 @@ defmodule FarmbotOS.SysCalls.Farmware do
   end
 
   def farmware_timeout(farmware_runtime) do
-    time = @farmware_timeout / 1_000
+    time = @farmware_timeout / 1_000 / 60
     runtime = inspect(farmware_runtime)
-    msg = "Farmware did not exit after #{time} seconds. Terminating #{runtime}"
+    msg = "Farmware did not exit after #{time} minutes. Terminating #{runtime}"
 
     FarmbotCore.Logger.info(2, msg)
     FarmwareRuntime.stop(farmware_runtime)

farmbot_os/lib/farmbot_os/sys_calls/flash_firmware.ex

@@ -1,3 +1,5 @@
+Application.get_env(:farmbot, FarmbotOS.SysCalls.FlashFirmware, [])[:gpio]
+
 defmodule FarmbotOS.SysCalls.FlashFirmware do
   @moduledoc false
 
@@ -5,6 +7,23 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
   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]
 
@@ -29,9 +48,7 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
            ),
          :ok <- FarmbotFirmware.close_transport(),
          _ <- FarmbotCore.Logger.debug(3, "starting firmware flash"),
-         {_, 0} <- Avrdude.flash(hex_file, tty, fun) do
+         _ <- finish_flashing(Avrdude.flash(hex_file, tty, fun)) do
-      FarmbotCore.Logger.success(2, "Firmware flashed successfully!")
-
       %{firmware_path: tty}
       |> Asset.update_fbos_config!()
       |> Private.mark_dirty!(%{})
@@ -42,10 +59,18 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
         {:error, reason}
 
       error ->
-        {:error, "flash_firmware misc error: #{inspect(error)}"}
+        {:error, "flash_firmware returned #{inspect(error)}"}
     end
   end
 
+  def finish_flashing({_result, 0}) do
+    FarmbotCore.Logger.success(2, "Firmware flashed successfully!")
+  end
+
+  def finish_flashing(result) do
+    FarmbotCore.Logger.debug(2, "AVR flash returned #{inspect(result)}")
+  end
+
   defp find_tty() do
     case FirmwareTTYDetector.tty() do
       nil ->
@@ -60,18 +85,30 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
   end
 
   defp find_reset_fun("express_k10") do
-    FarmbotCore.Logger.debug(3, "Using special reset function for express")
+    FarmbotCore.Logger.debug(3, "Using special express reset function")
-    # "magic" workaround to avoid compiler warnings.
+    {:ok, fn -> express_reset_fun() end}
-    # We used to inject this via App config, but it was
-    # error prone.
-    mod = :"Elixir.FarmbotOS.Platform.Target.FirmwareReset.GPIO"
-    fun = &mod.reset/0
-    {:ok, fun}
   end
 
   defp find_reset_fun(_) do
     FarmbotCore.Logger.debug(3, "Using default reset function")
-    fun = &FarmbotFirmware.NullReset.reset/0
+    {:ok, &FarmbotFirmware.NullReset.reset/0}
-    {:ok, fun}
+  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/avrdude/avrdude_test.exs

@@ -31,17 +31,11 @@ defmodule FarmbotOs.AvrdudeTest do
                "-b115200",
                "-D",
                "-V",
+               "-v",
                "-Uflash:w:/tmp/wow:i"
              ]
 
-      assert opts == [
+      assert opts == [stderr_to_stdout: true]
-               into: %IO.Stream{
-                 device: :standard_io,
-                 line_or_bytes: :line,
-                 raw: false
-               },
-               stderr_to_stdout: true
-             ]
     end)
 
     Avrdude.flash("/tmp/wow", "null", fn ->
@@ -62,17 +56,11 @@ defmodule FarmbotOs.AvrdudeTest do
                "-b115200",
                "-D",
                "-V",
+               "-v",
                "-Uflash:w:/tmp/wow:i"
              ]
 
-      assert opts == [
+      assert opts == [stderr_to_stdout: true]
-               into: %IO.Stream{
-                 device: :standard_io,
-                 line_or_bytes: :line,
-                 raw: false
-               },
-               stderr_to_stdout: true
-             ]
     end)
 
     Avrdude.flash("/tmp/wow", "/dev/null", fn ->

farmbot_os/test/farmbot_os/syscalls/farmware_test.exs

@@ -9,7 +9,7 @@ defmodule FarmbotOS.SysCalls.FarmwareTest do
 
     expect(FarmbotCore.LogExecutor, :execute, fn log ->
       expected =
-        "Farmware did not exit after 60.0 seconds. Terminating :FAKE_PID"
+        "Farmware did not exit after 20.0 minutes. Terminating :FAKE_PID"
 
       assert log.message == expected
       :ok