(qa/v10.0.0-rc4) Experiment: Remove `state.status` guard clause from `close_transport` call?

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2.0
 defaults: &defaults
   working_directory: /nerves/build
   docker:
-    - image: nervesproject/nerves_system_br:1.11.3
+    - image: nervesproject/nerves_system_br:latest
 
 install_elixir: &install_elixir
   run:

CHANGELOG.md

@@ -4,8 +4,8 @@
 
  * Deprecate `resource_update` RPC
  * Introduce `update_resource` RPC, which allows users to modify variables from the sequence editor.
+ * Allow Express users to upgrade from RPi0 => RPi3 (requires 40 pin ribbon cable and USB adapter).
  * Genesis v1.5 and Express v1.0 firmware updates.
- * Fix a bug where FBOS would not honor an "AUTO UPDATE" value of "false".
 
 # 9.2.2
 

FEATURE_MIN_VERSIONS.json

@@ -21,7 +21,6 @@
   "ota_update_hour": "8.2.3",
   "rpi_led_control": "6.4.4",
   "sensors": "6.3.0",
-  "update_resource": "10.0.0",
   "use_update_channel": "6.4.12",
   "variables": "8.0.0"
 }

RELEASE_NOTES.md

@@ -28,14 +28,3 @@ This release uses an improved Farmware API:
 # v9
 
 FarmBot OS v8+ uses an improved Farmware API. See the [Farmware developer documentation](https://developer.farm.bot/docs/farmware) for more information.
-
-# v10
-
-FarmBot OS v10 features an improved *Mark As* step. If you have previously added *Mark As* steps to sequences, you will need to update them before they can be executed by FarmBot:
-* Open any sequences with a caution icon next to the name.
-* Click the `CONVERT` button in each old *Mark As* step.
-* Save the sequence.
-* If you have auto-sync disabled, press `SYNC NOW` once all sequences have been updated.
-* Verify that any events using the updated sequences are running as expected.
-
-FarmBot OS auto-update was disabled prior to this release.

VERSION

@@ -1 +1 @@
-10.0.0
+10.0.0-rc4

farmbot_celery_script/lib/farmbot_celery_script/compiler.ex

@@ -11,10 +11,17 @@ defmodule FarmbotCeleryScript.Compiler do
     Compiler.IdentifierSanitizer
   }
 
+  @doc "Sets debug mode for the compiler"
+  def debug_mode(bool \\ true) do
+    old = Application.get_env(:farmbot_celery_script, __MODULE__, [])
+    new = Keyword.put(old, :debug, bool)
+    Application.put_env(:farmbot_celery_script, __MODULE__, new)
+    bool
+  end
+
   @doc "Returns current debug mode value"
   def debug_mode?() do
-    # Set this to `true` when debuging.
+    Application.get_env(:farmbot_celery_script, __MODULE__)[:debug] || false
-    false
   end
 
   @valid_entry_points [:sequence, :rpc_request]
@@ -61,8 +68,8 @@ defmodule FarmbotCeleryScript.Compiler do
     compile_entry_point(compile_ast(ast, env), env, [])
   end
 
-  def compile_entry_point([{_, new_env, _} = compiled | rest], env, acc) do
+  def compile_entry_point([{_, new_env, _} = compiled | rest], old_env, acc) do
-    env = Keyword.merge(env, new_env)
+    env = Keyword.merge(old_env, new_env)
     debug_mode?() && print_compiled_code(compiled)
     # entry points must be evaluated once more with the calling `env`
     # to return a list of compiled `steps`
@@ -70,7 +77,9 @@ defmodule FarmbotCeleryScript.Compiler do
     # TODO: investigate why i have to turn this to a string
     # before eval ing it?
     # case Code.eval_quoted(compiled, [], __ENV__) do
-    case Macro.to_string(compiled) |> Code.eval_string(new_env, __ENV__) do
+    result = Macro.to_string(compiled) |> Code.eval_string(new_env, __ENV__)
+
+    case result do
       {fun, new_env} when is_function(fun, 1) ->
         env = Keyword.merge(env, new_env)
         compile_entry_point(rest, env, acc ++ apply(fun, [env]))
@@ -274,13 +283,13 @@ defmodule FarmbotCeleryScript.Compiler do
   end
 
   defp print_compiled_code(compiled) do
-    IO.puts("=== START ===")
+    IO.puts("========")
 
     compiled
     |> Macro.to_string()
     |> Code.format_string!()
     |> IO.puts()
 
-    IO.puts("=== END ===\n\n")
+    IO.puts("--------\n\n")
   end
 end

farmbot_celery_script/lib/farmbot_celery_script/compiler/identifier_sanitizer.ex

@@ -2,7 +2,7 @@ defmodule FarmbotCeleryScript.Compiler.IdentifierSanitizer do
   @moduledoc """
   Responsible for ensuring variable names in Sequences are clean.
   This is done because identifiers are `unquote`d and the user controls
-  the data inside them. To prevent things like
+  the data inside them. To prevent things like 
   `"System.cmd("rm -rf /*/**")"` being evaluated, all identifiers
   are sanitized by prepending a token and hashing the value.
   """

farmbot_celery_script/lib/farmbot_celery_script/compilers/sequence_compiler.ex

@@ -1,6 +1,5 @@
 defmodule FarmbotCeleryScript.Compiler.Sequence do
   import FarmbotCeleryScript.Compiler.Utils
-  alias FarmbotCeleryScript.Compiler.IdentifierSanitizer
 
   @iterables [:point_group, :every_point]
 
@@ -30,7 +29,13 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
   def compile_sequence_iterable(
         iterable_ast,
         %{
-          args: %{locals: %{body: _} = locals} = sequence_args,
+          args:
+            %{
+              locals:
+                %{
+                  body: params
+                } = locals
+            } = sequence_args,
           meta: sequence_meta
         } = sequence_ast,
         env
@@ -38,6 +43,31 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
     sequence_name =
       sequence_meta[:sequence_name] || sequence_args[:sequence_name]
 
+    # remove the iterable from the parameter applications,
+    # since it will be injected after this.
+    _params =
+      Enum.reduce(params, [], fn
+        # Remove point_group from parameter appls
+        %{
+          kind: :parameter_application,
+          args: %{data_value: %{kind: :point_group}}
+        },
+        acc ->
+          acc
+
+        # Remove every_point from parameter appls
+        %{
+          kind: :parameter_application,
+          args: %{data_value: %{kind: :every_point}}
+        },
+        acc ->
+          acc
+
+        # Everything else gets added back
+        ast, acc ->
+          acc ++ [ast]
+      end)
+
     # will be a point_group or every_point node
     group_ast = iterable_ast.args.data_value
     # check if it's a point_group first, then fall back to every_point
@@ -112,37 +142,6 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
     end
   end
 
-  def create_better_params(body, env) do
-    parameter_declarations =
-      Enum.reduce(env, %{}, fn
-        {key, value}, map ->
-          encoded_label = "#{key}"
-
-          if String.starts_with?(encoded_label, "unsafe_") do
-            Map.put(map, IdentifierSanitizer.to_string(encoded_label), value)
-          else
-            map
-          end
-      end)
-
-    Enum.reduce(body, parameter_declarations, fn ast, map ->
-      case ast do
-        %{kind: :parameter_application} ->
-          args = Map.fetch!(ast, :args)
-          label = Map.fetch!(args, :label)
-          Map.put(map, label, Map.fetch!(args, :data_value))
-
-        %{kind: :variable_declaration} ->
-          args = Map.fetch!(ast, :args)
-          label = Map.fetch!(args, :label)
-          Map.put(map, label, Map.fetch!(args, :data_value))
-
-        %{kind: :parameter_declaration} ->
-          map
-      end
-    end)
-  end
-
   def compile_sequence(
         %{args: %{locals: %{body: params}} = args, body: block, meta: meta},
         env
@@ -151,9 +150,6 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
     # The `params` side gets turned into
     # a keyword list. These `params` are passed in from a previous sequence.
     # The `body` side declares variables in _this_ scope.
-    # === DON'T USE THIS IN NEW CODE.
-    #     SCHEDULED FOR DEPRECATION.
-    #     USE `better_params` INSTEAD.
     {params_fetch, body} =
       Enum.reduce(params, {[], []}, fn ast, {params, body} = _acc ->
         case ast do
@@ -177,8 +173,6 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
 
     steps = add_sequence_init_and_complete_logs(steps, sequence_name)
 
-    better_params = create_better_params(params, env)
-
     [
       quote location: :keep do
         fn params ->
@@ -189,7 +183,7 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
           # parent = Keyword.fetch!(params, :parent)
           unquote_splicing(params_fetch)
           unquote_splicing(assignments)
-          better_params = unquote(better_params)
+
           # Unquote the remaining sequence steps.
           unquote(steps)
         end

farmbot_celery_script/lib/farmbot_celery_script/compilers/update_resource_compiler.ex

@@ -1,54 +1,34 @@
 defmodule FarmbotCeleryScript.Compiler.UpdateResource do
-  alias FarmbotCeleryScript.{AST, DotProps}
+  alias FarmbotCeleryScript.{Compiler, AST, DotProps}
 
-  def update_resource(%AST{args: args, body: body}, _env) do
+  def update_resource(%AST{args: args, body: body}, env) do
-    quote location: :keep do
+    quote do
-      me = unquote(__MODULE__)
+      unquote(__MODULE__).do_update(
-      variable = unquote(Map.fetch!(args, :resource))
+        unquote(Map.fetch!(args, :resource)),
-      update = unquote(unpair(body, %{}))
+        unquote(unpair(body, %{})),
-
+        unquote(env)
-      # Go easy on the API...
+      )
-      case variable do
-        %AST{kind: :identifier} ->
-          args = Map.fetch!(variable, :args)
-          label = Map.fetch!(args, :label)
-          resource = Map.fetch!(better_params, label)
-          me.do_update(resource, update)
-
-        %AST{kind: :point} ->
-          me.do_update(variable.args, update)
-
-        %AST{kind: :resource} ->
-          me.do_update(variable.args, update)
-
-        res ->
-          raise "Resource error. Please notfiy support: #{inspect(res)}"
-      end
     end
   end
 
-  def do_update(%{pointer_id: id, pointer_type: kind}, update_params) do
+  def do_update(%AST{kind: :identifier} = res, update, env) do
-    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
+    {name, environ, nil} = Compiler.compile_ast(res, env)
+    value = Keyword.fetch!(environ, name)
+    %{resource_id: id, resource_type: kind} = value
+    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update)
   end
 
-  def do_update(%{resource_id: id, resource_type: kind}, update_params) do
+  def do_update(%AST{kind: :resource} = res, update, _) do
-    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
+    %{resource_id: id, resource_type: kind} = res.args
+    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update)
   end
 
-  def do_update(%{args: %{pointer_id: id, pointer_type: k}}, update_params) do
+  def do_update(res, _, _) do
-    FarmbotCeleryScript.SysCalls.update_resource(k, id, update_params)
+    raise "update_resource error. Please notfiy support: #{inspect(res)}"
-  end
-
-  def do_update(other, update) do
-    raise String.trim("""
-          MARK AS can only be used to mark resources like plants and devices.
-          It cannot be used on things like coordinates.
-          Ensure that your sequences and farm events us MARK AS on plants and not
-          coordinates (#{inspect(other)} / #{inspect(update)})
-          """)
   end
 
   defp unpair([pair | rest], acc) do
+    IO.puts("TODO: Need to apply handlebars to `value`s.")
     key = Map.fetch!(pair.args, :label)
     val = Map.fetch!(pair.args, :value)
     next_acc = Map.merge(acc, DotProps.create(key, val))

farmbot_celery_script/test/farmbot_celery_script/compiler_groups_test.exs

@@ -47,8 +47,8 @@ defmodule FarmbotCeleryScript.CompilerGroupsTest do
     canary_actual = :crypto.hash(:sha, Macro.to_string(result))
 
     canary_expected =
-      <<136, 140, 48, 226, 216, 155, 178, 103, 244, 88, 225, 146, 130, 216, 125,
+      <<157, 69, 5, 38, 188, 78, 10, 183, 154, 99, 151, 193, 214, 208, 187, 130,
-        72, 113, 195, 65, 1>>
+        183, 73, 13, 48>>
 
     # READ THE NOTE ABOVE IF THIS TEST FAILS!!!
     assert canary_expected == canary_actual

farmbot_celery_script/test/farmbot_celery_script/compiler_test.exs

@@ -79,7 +79,7 @@ defmodule FarmbotCeleryScript.CompilerTest do
   end
 
   test "identifier sanitization" do
-    label = "System.cmd(\"echo\", [\"lol\"])"
+    label = "System.cmd(\"rm\", [\"-rf /*\"])"
     value_ast = AST.Factory.new("coordinate", x: 1, y: 1, z: 1)
     identifier_ast = AST.Factory.new("identifier", label: label)
 
@@ -120,19 +120,11 @@ defmodule FarmbotCeleryScript.CompilerTest do
              [
                fn params ->
                  _ = inspect(params)
-                 unsafe_U3lzdGVtLmNtZCgiZWNobyIsIFsibG9sIl0p = FarmbotCeleryScript.SysCalls.coordinate(1, 1, 1)
 
-                 better_params = %{
+                 #{var_name} =
-                   "System.cmd(\\"echo\\", [\\"lol\\"])" => %FarmbotCeleryScript.AST{
+                   FarmbotCeleryScript.SysCalls.coordinate(1, 1, 1)
-                     args: %{x: 1, y: 1, z: 1},
-                     body: [],
-                     comment: nil,
-                     kind: :coordinate,
-                     meta: nil
-                   }
-                 }
 
-                 [fn -> unsafe_U3lzdGVtLmNtZCgiZWNobyIsIFsibG9sIl0p end]
+                 [fn -> #{var_name} end]
                end
              ]
              """)
@@ -380,38 +372,19 @@ defmodule FarmbotCeleryScript.CompilerTest do
                  unsafe_cGFyZW50 =
                    Keyword.get(params, :unsafe_cGFyZW50, FarmbotCeleryScript.SysCalls.coordinate(1, 2, 3))
 
-                 better_params = %{}
-
                  [
                    fn ->
-                     me = FarmbotCeleryScript.Compiler.UpdateResource
+                     FarmbotCeleryScript.Compiler.UpdateResource.do_update(
-
+                       %FarmbotCeleryScript.AST{
-                     variable = %FarmbotCeleryScript.AST{
+                         args: %{label: "parent"},
-                       args: %{label: "parent"},
+                         body: [],
-                       body: [],
+                         comment: nil,
-                       comment: nil,
+                         kind: :identifier,
-                       kind: :identifier,
+                         meta: nil
-                       meta: nil
+                       },
-                     }
+                       %{"plant_stage" => "removed"},
-
+                       []
-                     update = %{"plant_stage" => "removed"}
+                     )
-
-                     case(variable) do
-                       %AST{kind: :identifier} ->
-                         args = Map.fetch!(variable, :args)
-                         label = Map.fetch!(args, :label)
-                         resource = Map.fetch!(better_params, label)
-                         me.do_update(resource, update)
-
-                       %AST{kind: :point} ->
-                         me.do_update(variable.args(), update)
-
-                       %AST{kind: :resource} ->
-                         me.do_update(variable.args(), update)
-
-                       res ->
-                         raise("Resource error. Please notfiy support: \#{inspect(res)}")
-                     end
                    end
                  ]
                end
@@ -432,38 +405,20 @@ defmodule FarmbotCeleryScript.CompilerTest do
              [
                fn params ->
                  _ = inspect(params)
-                 better_params = %{}
 
                  [
                    fn ->
-                     me = FarmbotCeleryScript.Compiler.UpdateResource
+                     FarmbotCeleryScript.Compiler.UpdateResource.do_update(
-
+                       %FarmbotCeleryScript.AST{
-                     variable = %FarmbotCeleryScript.AST{
+                         args: %{resource_id: 23, resource_type: "Plant"},
-                       args: %{resource_id: 23, resource_type: "Plant"},
+                         body: [],
-                       body: [],
+                         comment: nil,
-                       comment: nil,
+                         kind: :resource,
-                       kind: :resource,
+                         meta: nil
-                       meta: nil
+                       },
-                     }
+                       %{"plant_stage" => "planted", "r" => 23},
-
+                       []
-                     update = %{"plant_stage" => "planted", "r" => 23}
+                     )
-
-                     case(variable) do
-                       %AST{kind: :identifier} ->
-                         args = Map.fetch!(variable, :args)
-                         label = Map.fetch!(args, :label)
-                         resource = Map.fetch!(better_params, label)
-                         me.do_update(resource, update)
-
-                       %AST{kind: :point} ->
-                         me.do_update(variable.args(), update)
-
-                       %AST{kind: :resource} ->
-                         me.do_update(variable.args(), update)
-
-                       res ->
-                         raise("Resource error. Please notfiy support: \#{inspect(res)}")
-                     end
                    end
                  ]
                end

farmbot_core/config/config.exs

@@ -46,7 +46,12 @@ config :farmbot_core, FarmbotCore.EctoMigrator,
       "beta"
     )
 
-config :farmbot_firmware, FarmbotFirmware, reset: FarmbotCore.FirmwareResetter
+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
 
 import_config "ecto.exs"
 import_config "logger.exs"

farmbot_core/config/dev.exs

@@ -2,3 +2,8 @@ use Mix.Config
 
 config :farmbot_celery_script, FarmbotCeleryScript.SysCalls,
   sys_calls: FarmbotCeleryScript.SysCalls.Stubs
+
+config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 5
+
+config :farmbot_core, FarmbotCore.AssetWorker.FarmbotCore.Asset.FbosConfig,
+  firmware_flash_attempt_threshold: 5

farmbot_core/lib/farmbot_core.ex

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

farmbot_core/lib/farmbot_core/asset.ex

@@ -6,15 +6,15 @@ defmodule FarmbotCore.Asset do
   """
 
   alias FarmbotCore.Asset.{
-    CriteriaRetriever,
+    Repo,
     Device,
     DeviceCert,
-    FarmEvent,
     FarmwareEnv,
+    FirstPartyFarmware,
     FarmwareInstallation,
+    FarmEvent,
     FbosConfig,
     FirmwareConfig,
-    FirstPartyFarmware,
     Peripheral,
     PinBinding,
     Point,
@@ -22,11 +22,11 @@ defmodule FarmbotCore.Asset do
     PublicKey,
     Regimen,
     RegimenInstance,
-    Repo,
+    Sequence,
     Sensor,
     SensorReading,
-    Sequence,
     Tool,
+    CriteriaRetriever
   }
 
   alias FarmbotCore.AssetSupervisor
@@ -251,24 +251,8 @@ defmodule FarmbotCore.Asset do
   end
 
   def update_point(point, params) do
-    # TODO: RC 8 MAY 2020 - We need to hard refresh the point.
+    point
-    #       The CSVM appears to be caching resources. This leads
+    |> Point.changeset(params)
-    #       to problems when a user runs a sequence that has two
-    #       MARK AS steps.
-    # NOTE: Updating the `meta` attribute is a _replace_ action
-    #       by default, not a merge action.
-    # MORE NOTES: Mixed keys (symbol vs. string) will crash this FN.
-    #             Let's just stringify everything...
-    new_meta     = params[:meta] || params["meta"] || %{}
-    old_meta     = point.meta || %{}
-    updated_meta = Map.merge(old_meta, new_meta)
-    clean_params = params
-    |> Map.merge(%{meta: updated_meta})
-    |> Enum.map(fn {k, v} -> {"#{k}", v} end)
-    |> Map.new()
-
-    Repo.get_by(Point, id: point.id)
-    |> Point.changeset(clean_params)
     |> Repo.update()
   end
 

farmbot_core/lib/farmbot_core/asset/private.ex

@@ -40,17 +40,17 @@ defmodule FarmbotCore.Asset.Private do
     # Because sqlite can't test unique constraints before a transaction, if this function gets called for
     # the same asset more than once asyncronously, the asset can be marked dirty twice at the same time
     # causing the `unique constraint` error to happen in either `ecto` OR `sqlite`. I've
-    # caught both errors here as they are both essentially the same thing, and can be safely
+    # caught both errors here as they are both essentially the same thing, and can be safely 
     # discarded. Doing an `insert_or_update/1` (without the bang) can still result in the sqlite
-    # error being thrown.
+    # error being thrown.  
     changeset = LocalMeta.changeset(local_meta, Map.merge(params, %{table: table, status: "dirty"}))
     try do
       Repo.insert_or_update!(changeset)
     catch
       :error,  %Sqlite.DbConnection.Error{
-        message: "UNIQUE constraint failed: local_metas.table, local_metas.asset_local_id",
+        message: "UNIQUE constraint failed: local_metas.table, local_metas.asset_local_id", 
         sqlite: %{code: :constraint}
-      } ->
+      } -> 
         Logger.warn """
         Caught race condition marking data as dirty (sqlite)
         table: #{inspect(table)}
@@ -59,10 +59,10 @@ defmodule FarmbotCore.Asset.Private do
         Ecto.Changeset.apply_changes(changeset)
       :error, %Ecto.InvalidChangesetError{
         changeset: %{
-          action: :insert,
+          action: :insert, 
           errors: [
             table: {"LocalMeta already exists.", [
-              validation: :unsafe_unique,
+              validation: :unsafe_unique, 
               fields: [:table, :asset_local_id]
             ]}
           ]}
@@ -73,7 +73,7 @@ defmodule FarmbotCore.Asset.Private do
         id: #{inspect(asset.local_id)}
         """
         Ecto.Changeset.apply_changes(changeset)
-      type, reason ->
+      type, reason -> 
         FarmbotCore.Logger.error 1, """
         Caught unexpected error marking data as dirty
         table: #{inspect(table)}

farmbot_core/lib/farmbot_core/asset_workers/fbos_config_worker.ex

@@ -11,9 +11,16 @@ defimpl FarmbotCore.AssetWorker, for: FarmbotCore.Asset.FbosConfig do
   alias FarmbotCore.{Asset.FbosConfig, BotState, Config}
   import FarmbotFirmware.PackageUtils, only: [package_to_string: 1]
 
-  @firmware_flash_attempt_threshold Application.get_env(:farmbot_core, __MODULE__)[:firmware_flash_attempt_threshold] || 5
+  @firmware_flash_attempt_threshold Application.get_env(:farmbot_core, __MODULE__)[:firmware_flash_attempt_threshold]
   @firmware_flash_timeout Application.get_env(:farmbot_core, __MODULE__)[:firmware_flash_timeout] || 5000
   @disable_firmware_io_logs_timeout Application.get_env(:farmbot_core, __MODULE__)[:disable_firmware_io_logs_timeout] || 300000
+  @firmware_flash_attempt_threshold || Mix.raise """
+  Firmware open attempt threshold not configured:
+
+  config :farmbot_core, #{__MODULE__}, [
+    firmware_flash_attempt_threshold: :infinity
+  ]
+  """
 
   @impl FarmbotCore.AssetWorker
   def preload(%FbosConfig{}), do: []

farmbot_core/lib/farmbot_core/firmware_open_task.ex

@@ -9,7 +9,14 @@ defmodule FarmbotCore.FirmwareOpenTask do
   require FarmbotCore.Logger
   alias FarmbotFirmware.{UARTTransport, StubTransport}
   alias FarmbotCore.{Asset, Config}
-  @attempt_threshold Application.get_env(:farmbot_core, __MODULE__)[:attempt_threshold] || 5
+  @attempt_threshold Application.get_env(:farmbot_core, __MODULE__)[:attempt_threshold]
+  @attempt_threshold || Mix.raise """
+  Firmware open attempt threshold not configured:
+
+  config :farmbot_core, FarmbotCore.FirmwareOpenTask, [
+    attempt_threshold: 10
+  ]
+  """
 
   @doc false
   def start_link(args, opts \\ [name: __MODULE__]) do
@@ -18,10 +25,7 @@ defmodule FarmbotCore.FirmwareOpenTask do
 
   @doc false
   def swap_transport(tty) do
-    Application.put_env(:farmbot_firmware, FarmbotFirmware,
+    Application.put_env(:farmbot_firmware, FarmbotFirmware, transport: UARTTransport, device: tty)
-    transport: UARTTransport,
-    device: tty,
-    reset: FarmbotCore.FirmwareResetter)
     # Swap transport on FW module.
     # Close tranpsort if it is open currently.
     _ = FarmbotFirmware.close_transport()
@@ -29,9 +33,7 @@ defmodule FarmbotCore.FirmwareOpenTask do
   end
 
   def unswap_transport() do
-    Application.put_env(:farmbot_firmware, FarmbotFirmware,
+    Application.put_env(:farmbot_firmware, FarmbotFirmware, transport: StubTransport)
-    transport: StubTransport,
-    reset: FarmbotCore.FirmwareResetter)
     # Swap transport on FW module.
     # Close tranpsort if it is open currently.
     _ = FarmbotFirmware.close_transport()

farmbot_core/lib/farmbot_core/firmware_resetter.ex

@@ -1,46 +0,0 @@
-defmodule FarmbotCore.FirmwareResetter do
-  if Code.ensure_compiled?(Circuits.GPIO) do
-    @gpio Circuits.GPIO
-  else
-    @gpio nil
-  end
-  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
-      gpio_module = @gpio
-      FarmbotCore.Logger.debug(3, "Begin MCU reset")
-      {:ok, gpio} = gpio_module.open(19, :output)
-      :ok         = gpio_module.write(gpio, 0)
-      :ok         = gpio_module.write(gpio, 1)
-      Process.sleep(1000)
-      :ok = gpio_module.write(gpio, 0)
-      FarmbotCore.Logger.debug(3, "Finish MCU Reset")
-      :ok
-    rescue
-      ex ->
-        message = Exception.message(ex)
-        msg = "Express reset failed #{message}"
-        FarmbotCore.Logger.error(3, msg)
-        {:error, msg}
-    end
-  end
-end

farmbot_core/lib/farmbot_core/firmware_tty_detector.ex

@@ -6,7 +6,7 @@ defmodule FarmbotCore.FirmwareTTYDetector do
   @error_retry_ms 5_000
 
   if System.get_env("FARMBOT_TTY") do
-    @expected_names ["ttyUSB0", "ttyAMA0", "ttyACM0", System.get_env("FARMBOT_TTY")]
+    @expected_names [System.get_env("FARMBOT_TTY")]
   else
     @expected_names ["ttyUSB0", "ttyAMA0", "ttyACM0"]
   end
@@ -50,6 +50,7 @@ defmodule FarmbotCore.FirmwareTTYDetector do
     if farmbot_tty?(name) do
       {:noreply, name}
     else
+      # Logger.warn("#{name} is not an expected Farmbot Firmware TTY")
       {:noreply, state, {:continue, rest}}
     end
   end

farmbot_ext/config/config.exs

@@ -2,6 +2,8 @@ use Mix.Config
 
 config :logger, handle_otp_reports: true, handle_sasl_reports: true
 
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+
 # TODO(Rick) We probably don't need to use this anymore now that Mox is a thing.
 config :farmbot_celery_script, FarmbotCeleryScript.SysCalls,
   sys_calls: FarmbotCeleryScript.SysCalls.Stubs

farmbot_ext/lib/farmbot_ext/api/dirty_worker.ex

@@ -6,9 +6,8 @@ defmodule FarmbotExt.API.DirtyWorker do
   import API.View, only: [render: 2]
 
   require Logger
-  require FarmbotCore.Logger
   use GenServer
-  @timeout 500
+  @timeout 10000
 
   # these resources can't be accessed by `id`.
   @singular [
@@ -35,69 +34,84 @@ defmodule FarmbotExt.API.DirtyWorker do
 
   @impl GenServer
   def init(args) do
+    # Logger.disable(self())
     module = Keyword.fetch!(args, :module)
-    Process.send_after(self(), :do_work, @timeout)
+    timeout = Keyword.get(args, :timeout, @timeout)
-    {:ok, %{module: module}}
+    {:ok, %{module: module, timeout: timeout}, timeout}
   end
 
   @impl GenServer
-  def handle_info(:do_work, %{module: module} = state) do
+  def handle_info(:timeout, %{module: module} = state) do
-    Process.sleep(@timeout)
+    dirty = Private.list_dirty(module)
-    list = Enum.uniq(Private.list_dirty(module) ++ Private.list_local(module))
+    local = Private.list_local(module)
-
+    {:noreply, state, {:continue, Enum.uniq(dirty ++ local)}}
-    unless has_race_condition?(module, list) do
-      Enum.map(list, fn dirty -> work(dirty, module) end)
-    end
-
-    Process.send_after(self(), :do_work, @timeout)
-    {:noreply, state}
   end
 
-  def work(dirty, module) do
+  @impl GenServer
-    # Go easy on the API
+  def handle_continue([], state) do
-    Process.sleep(333)
+    {:noreply, state, state.timeout}
+  end
 
-    case http_request(dirty, module) do
+  def handle_continue([dirty | rest], %{module: module} = state) do
+    # Logger.info("[#{module} #{dirty.local_id} #{inspect(self())}] Handling dirty data")
+
+    case http_request(dirty, state) do
       # Valid data
       {:ok, %{status: s, body: body}} when s > 199 and s < 300 ->
-        dirty |> module.changeset(body) |> handle_changeset(module)
+        # Logger.debug(
+        #   "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP request complete: #{s} ok"
+        # )
+
+        dirty |> module.changeset(body) |> handle_changeset(rest, state)
 
       # Invalid data
       {:ok, %{status: s, body: %{} = body}} when s > 399 and s < 500 ->
-        FarmbotCore.Logger.error(2, "HTTP Error #{s}. #{inspect(body)}")
+        # Logger.debug(
+        #   "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP request complete: #{s} error+body"
+        # )
+
         changeset = module.changeset(dirty)
 
         Enum.reduce(body, changeset, fn {key, val}, changeset ->
           Ecto.Changeset.add_error(changeset, key, val)
         end)
-        |> handle_changeset(module)
+        |> handle_changeset(rest, state)
 
       # Invalid data, but the API didn't say why
       {:ok, %{status: s, body: _body}} when s > 399 and s < 500 ->
-        FarmbotCore.Logger.error(2, "HTTP Error #{s}. #{inspect(dirty)}")
+        # Logger.debug(
+        #   "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP request complete: #{s} error"
+        # )
 
         module.changeset(dirty)
         |> Map.put(:valid?, false)
-        |> handle_changeset(module)
+        |> handle_changeset(rest, state)
 
       # HTTP Error. (500, network error, timeout etc.)
       error ->
-        FarmbotCore.Logger.error(
+        Logger.error(
-          2,
+          "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP Error: #{state.module} #{
-          "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP Error: #{module} #{
             inspect(error)
           }"
         )
+
+        {:noreply, state, @timeout}
     end
   end
 
   # If the changeset was valid, update the record.
-  def handle_changeset(%{valid?: true} = changeset, _module) do
+  def handle_changeset(%{valid?: true} = changeset, rest, state) do
-    Private.mark_clean!(Repo.update!(changeset))
+    # Logger.info("Successfully synced: #{state.module}")
-    :ok
+
+    Repo.update!(changeset)
+    |> Private.mark_clean!()
+
+    {:noreply, state, {:continue, rest}}
   end
 
-  def handle_changeset(%{valid?: false, data: data} = changeset, module) do
+  # If the changeset was invalid, delete the record.
+  # TODO(Connor) - Update the dirty field here, upload to rollbar?
+  def handle_changeset(%{valid?: false, data: data} = changeset, rest, state) do
     message =
       Enum.map(changeset.errors, fn
         {key, {msg, _meta}} when is_binary(key) -> "\t#{key}: #{msg}"
@@ -105,64 +119,29 @@ defmodule FarmbotExt.API.DirtyWorker do
       end)
       |> Enum.join("\n")
 
-    FarmbotCore.Logger.error(3, "Failed to sync: #{module} \n #{message}")
+    Logger.error("Failed to sync: #{state.module} \n #{message}")
     _ = Repo.delete!(data)
-    :ok
+    {:noreply, state, {:continue, rest}}
   end
 
-  defp http_request(%{id: nil} = dirty, module) do
+  defp http_request(%{id: nil} = dirty, state) do
-    path = module.path()
+    # Logger.debug("#{state.module} clean request (post)")
-    data = render(module, dirty)
+    path = state.module.path()
+    data = render(state.module, dirty)
     API.post(API.client(), path, data)
   end
 
-  defp http_request(dirty, module) when module in @singular do
+  defp http_request(dirty, %{module: module} = state) when module in @singular do
-    path = path = module.path()
+    # Logger.debug("#{state.module} dirty request (patch)")
-    data = render(module, dirty)
+    path = path = state.module.path()
+    data = render(state.module, dirty)
     API.patch(API.client(), path, data)
   end
 
-  defp http_request(dirty, module) do
+  defp http_request(dirty, state) do
-    path = Path.join(module.path(), to_string(dirty.id))
+    # Logger.debug("#{state.module} dirty request (patch)")
-    data = render(module, dirty)
+    path = Path.join(state.module.path(), to_string(dirty.id))
+    data = render(state.module, dirty)
     API.patch(API.client(), path, data)
   end
-
-  # This is a fix for a race condtion. The root cause is unknown
-  # as of 18 May 2020. The problem is that records are marked
-  # diry _before_ the dirty data is saved. That means that FBOS
-  # knows a record has changed, but for a brief moment, it only
-  # has the old copy of the record (not the changes).
-  # Because of this race condtion,
-  # The race condition:
-  #
-  # * Is nondeterministic
-  # * Happens frequently when running many MARK AS steps in one go.
-  # * Happens frequently when Erlang VM only has one thread
-  #    * Ie: `iex --erl '+S 1 +A 1' -S mix`
-  # * Happens frequently when @timeout is decreased to `1`.
-  #
-  # This function PREVENTS CORRUPTION OF API DATA. It can be
-  # removed once the root cause of the data race is determined.
-  #   - RC 18 May 2020
-  def has_race_condition?(module, list) do
-    Enum.find(list, fn item ->
-      if item.id do
-        if item == Repo.get_by(module, id: item.id) do
-          # This item is OK - no race condition.
-          false
-        else
-          # There was a race condtion. We probably can't trust
-          # any of the data in this list. We need to wait and
-          # try again later.
-          Process.sleep(@timeout * 3)
-          true
-        end
-      else
-        # This item only exists on the FBOS side.
-        # It will never be affected by the data race condtion.
-        false
-      end
-    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,7 +107,8 @@ defmodule FarmbotFirmware do
     :command_queue,
     :caller_pid,
     :current,
-    :reset
+    :reset,
+    :reset_pid
   ]
 
   @type state :: %State{
@@ -122,7 +123,8 @@ defmodule FarmbotFirmware do
           command_queue: [{pid(), GCODE.t()}],
           caller_pid: nil | pid,
           current: nil | GCODE.t(),
-          reset: module()
+          reset: module(),
+          reset_pid: nil | pid()
         }
 
   @doc """
@@ -200,7 +202,16 @@ 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)
+    # 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
+
     # Add an anon function that transport implementations should call.
     fw = self()
     fun = fn {_, _} = code -> GenServer.cast(fw, code) end
@@ -214,6 +225,7 @@ defmodule FarmbotFirmware do
       side_effects: side_effects,
       status: :transport_boot,
       reset: reset,
+      reset_pid: nil,
       command_queue: [],
       configuration_queue: []
     }
@@ -230,6 +242,24 @@ 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.
@@ -345,8 +375,7 @@ defmodule FarmbotFirmware do
 
   # 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)
+  def handle_call(:close_transport, _from, state) do
-      when s != :transport_boot do
     if is_reference(state.transport_ref) do
       true = Process.demonitor(state.transport_ref)
     end
@@ -374,9 +403,11 @@ defmodule FarmbotFirmware do
     {:reply, :ok, next_state}
   end
 
-  def handle_call(:close_transport, _, %{status: s} = state) do
+  # WE MAY WANT TO REVERT THIS AND ADD GUARD CLAUSE BACK TO
-    {:reply, {:error, s}, state}
+  # `handle_call` ABOVE THIS. RC 1 MAY 2020
-  end
+  # def handle_call(:close_transport, _, %{status: s} = state) do
+  #   {:reply, {:error, s}, state}
+  # end
 
   def handle_call({:open_transport, module, args}, _from, %{status: s} = state)
       when s == :transport_boot do

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/param.ex

@@ -553,6 +553,6 @@ defmodule FarmbotFirmware.Param do
   def format_bool(val) when val == 1, do: true
   def format_bool(val) when val == 0, do: false
 
-  def format_high_low_inverted(val) when val == 0, do: "ON"
+  def format_high_low_inverted(val) when val == 0, do: "HIGH"
-  def format_high_low_inverted(val) when val == 1, do: "OFF"
+  def format_high_low_inverted(val) when val == 1, do: "LOW"
 end

farmbot_firmware/lib/farmbot_firmware/reset.ex

@@ -0,0 +1,8 @@
+defmodule FarmbotFirmware.Reset do
+  @moduledoc """
+  Behaviour to reset the UART connection into 
+  bootloader mode for firmware upgrades.
+  """
+
+  @callback reset :: :ok | {:error, Stirng.t()}
+end

farmbot_firmware/lib/farmbot_firmware/transports/uart_transport.ex

@@ -13,7 +13,7 @@ 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)
+    reset = Keyword.get(args, :reset)
     {:ok, uart} = UartDefaultAdapter.start_link()
 
     {:ok,

farmbot_firmware/test/farmbot_firmware/command_test.exs

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

farmbot_firmware/test/farmbot_firmware/transports/uart_transport_test.exs

@@ -14,7 +14,7 @@ defmodule FarmbotFirmware.UARTTransportTest do
     init_args = [
       device: :FAKE_DEVICE,
       handle_gcode: :FAKE_GCODE_HANDLER,
-      reset: StubReset
+      reset: :FAKE_RESETER
     ]
 
     {:ok, state, 0} = UARTTransport.init(init_args)

farmbot_firmware/test/farmbot_firmware_test.exs

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

farmbot_firmware/test/param_test.exs

@@ -16,19 +16,19 @@ defmodule FarmbotFirmware.ParamTest do
 
     t(:pin_guard_5_time_out, 12, {"pin guard 5 timeout", "(seconds)", "12"})
     t(:pin_guard_5_pin_nr, 12, {"pin guard 5 pin number", nil, "12"})
-    t(:pin_guard_5_active_state, 0, {"pin guard 5 safe state", nil, "ON"})
+    t(:pin_guard_5_active_state, 0, {"pin guard 5 safe state", nil, "HIGH"})
     t(:pin_guard_4_time_out, 12, {"pin guard 4 timeout", "(seconds)", "12"})
     t(:pin_guard_4_pin_nr, 12, {"pin guard 4 pin number", nil, "12"})
-    t(:pin_guard_4_active_state, 0, {"pin guard 4 safe state", nil, "ON"})
+    t(:pin_guard_4_active_state, 0, {"pin guard 4 safe state", nil, "HIGH"})
     t(:pin_guard_3_time_out, 1.0, {"pin guard 3 timeout", "(seconds)", "1"})
     t(:pin_guard_3_pin_nr, 1.0, {"pin guard 3 pin number", nil, "1"})
-    t(:pin_guard_3_active_state, 0, {"pin guard 3 safe state", nil, "ON"})
+    t(:pin_guard_3_active_state, 0, {"pin guard 3 safe state", nil, "HIGH"})
     t(:pin_guard_2_time_out, 1.0, {"pin guard 2 timeout", "(seconds)", "1"})
     t(:pin_guard_2_pin_nr, 1.0, {"pin guard 2 pin number", nil, "1"})
-    t(:pin_guard_2_active_state, 0, {"pin guard 2 safe state", nil, "ON"})
+    t(:pin_guard_2_active_state, 0, {"pin guard 2 safe state", nil, "HIGH"})
     t(:pin_guard_1_time_out, 1.0, {"pin guard 1 timeout", "(seconds)", "1"})
     t(:pin_guard_1_pin_nr, 1.0, {"pin guard 1 pin number", nil, "1"})
-    t(:pin_guard_1_active_state, 0, {"pin guard 1 safe state", nil, "ON"})
+    t(:pin_guard_1_active_state, 0, {"pin guard 1 safe state", nil, "HIGH"})
     t(:param_use_eeprom, 1, {"use eeprom", nil, true})
     t(:param_test, 1, {"param_test", nil, true})
     t(:param_mov_nr_retry, 1.0, {"max retries", nil, "1"})

farmbot_os/config/config.exs

@@ -80,6 +80,8 @@ config :farmbot, FarmbotOS.Platform.Supervisor,
     FarmbotOS.Platform.Host.Configurator
   ]
 
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+
 config :logger,
   handle_sasl_reports: false,
   handle_otp_reports: false,

farmbot_os/config/host/dev.exs

@@ -38,5 +38,8 @@ config :farmbot,
 
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 5
 
+config :farmbot_core, FarmbotCore.AssetWorker.FarmbotCore.Asset.FbosConfig,
+  firmware_flash_attempt_threshold: 5
+
 config :logger,
   backends: [:console]

farmbot_os/config/host/test.exs

@@ -35,6 +35,8 @@ config :farmbot, FarmbotOS.Configurator,
   data_layer: FarmbotOS.Configurator.ConfigDataLayer,
   network_layer: FarmbotOS.Configurator.FakeNetworkLayer
 
+config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: []
+
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 0
 
 config :farmbot_core, FarmbotCore.AssetWorker.FarmbotCore.Asset.FbosConfig,

farmbot_os/config/target/dev.exs

@@ -116,6 +116,9 @@ config :farmbot, FarmbotOS.System,
 
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 5
 
+config :farmbot_core, FarmbotCore.AssetWorker.FarmbotCore.Asset.FbosConfig,
+  firmware_flash_attempt_threshold: 5
+
 config :logger, backends: [RingLogger]
 
 config :logger, RingLogger,

farmbot_os/config/target/rpi.exs

@@ -1,6 +1,13 @@
 use Mix.Config
 
+config :farmbot_firmware, FarmbotFirmware,
+  reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
+
 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

@@ -2,6 +2,13 @@ use Mix.Config
 
 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/config/target/rpi3.exs

@@ -1,6 +1,7 @@
 use Mix.Config
 
-config :farmbot_firmware, FarmbotFirmware, reset: FarmbotCore.FirmwareResetter
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+config :farmbot, FarmbotOS.SysCalls.FlashFirmware, gpio: Circuits.GPIO
 
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [

farmbot_os/lib/farmbot_os/sys_calls.ex

@@ -212,7 +212,7 @@ defmodule FarmbotOS.SysCalls do
   @impl true
   def emergency_unlock do
     _ = FarmbotFirmware.command({:command_emergency_unlock, []})
-    FarmbotCore.Logger.busy(1, "Unlocked")
+    FarmbotCore.Logger.busy(1, "Unlocked.")
     :ok
   end
 

farmbot_os/lib/farmbot_os/sys_calls/flash_firmware.ex

@@ -1,10 +1,27 @@
 defmodule FarmbotOS.SysCalls.FlashFirmware do
   @moduledoc false
 
-  alias FarmbotCore.{Asset, Asset.Private, FirmwareResetter}
+  alias FarmbotCore.{Asset, Asset.Private}
   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]
 
@@ -21,11 +38,11 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
          {:ok, tty} <- find_tty(),
          _ <-
            FarmbotCore.Logger.debug(3, "found tty: #{tty} for firmware flash"),
-         {:ok, fun} <- FirmwareResetter.find_reset_fun(package),
+         {:ok, fun} <- find_reset_fun(package),
          _ <-
            FarmbotCore.Logger.debug(
              3,
-             "Closing the firmware transport before flash"
+             "closing firmware transport before flash"
            ),
          :ok <- FarmbotFirmware.close_transport(),
          _ <- FarmbotCore.Logger.debug(3, "starting firmware flash"),
@@ -67,4 +84,32 @@ 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/lib/farmbot_os/sys_calls/point_lookup.ex

@@ -9,7 +9,7 @@ defmodule FarmbotOS.SysCalls.PointLookup do
   def point(kind, id) do
     case Asset.get_point(id: id) do
       nil ->
-        {:error, "#{kind || "point"} #{id} not found"}
+        {:error, "#{kind || "point?"} #{id} not found"}
 
       %{name: name, x: x, y: y, z: z, pointer_type: type} ->
         %{

farmbot_os/lib/farmbot_os/sys_calls/resource_update.ex

@@ -2,7 +2,6 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
   @moduledoc false
 
   require Logger
-  require FarmbotCore.Logger
 
   alias FarmbotCore.{
     Asset,
@@ -12,38 +11,8 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
   alias FarmbotOS.SysCalls.SendMessage
 
   @point_kinds ~w(Plant GenericPointer ToolSlot Weed)
-  @friendly_names %{
-    "gantry_mounted" => "`gantry mounted` property",
-    "mounted_tool_id" => "mounted tool ID",
-    "openfarm_slug" => "Openfarm slug",
-    "ota_hour" => "OTA hour",
-    "plant_stage" => "plant stage",
-    "planted_at" => "planted at time",
-    "pullout_direction" => "pullout direction",
-    "tool_id" => "tool ID",
-    "tz_offset_hrs" => "timezone offset hours",
-    "x" => "X axis",
-    "y" => "Y axis",
-    "z" => "Z axis",
-    "Device" => "device",
-    "Plant" => "plant",
-    "GenericPointer" => "map point",
-    "ToolSlot" => "tool slot",
-    "Weed" => "weed"
-  }
-
-  def notify_user_of_updates(kind, params, id \\ nil) do
-    Enum.map(params, fn {k, v} ->
-      name = @friendly_names[kind] || kind
-      property = @friendly_names["#{k}"] || k
-      msg = "Setting #{name} #{id} #{property} to #{inspect(v)}"
-      FarmbotCore.Logger.info(3, msg)
-    end)
-  end
-
-  def update_resource("Device" = kind, _, params) do
-    notify_user_of_updates(kind, params)
 
+  def update_resource("Device", 0, params) do
     params
     |> do_handlebars()
     |> Asset.update_device!()
@@ -53,7 +22,6 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
   end
 
   def update_resource(kind, id, params) when kind in @point_kinds do
-    notify_user_of_updates(kind, params, id)
     params = do_handlebars(params)
     point_update_resource(kind, id, params)
   end
@@ -67,25 +35,17 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
 
   @doc false
   def point_update_resource(type, id, params) do
-    with %{} = point <- Asset.get_point(id: id),
+    with %{} = point <- Asset.get_point(pointer_type: type, id: id),
          {:ok, point} <- Asset.update_point(point, params) do
       _ = Private.mark_dirty!(point)
       :ok
     else
       nil ->
-        msg = "#{type}.#{id} is not currently synced. Please re-sync."
+        {:error,
-        FarmbotCore.Logger.error(3, msg)
+         "#{type}.#{id} is not currently synced, so it could not be updated"}
-        {:error, msg}
 
       {:error, _changeset} ->
-        msg =
+        {:error, "Failed to update #{type}.#{id}"}
-          "Failed update (#{type}.#{id}): Ensure the data is properly formatted"
-
-        FarmbotCore.Logger.error(3, msg)
-        {:error, msg}
-
-      err ->
-        {:error, "Unknown error. Please notify support. #{inspect(err)}"}
     end
   end
 

farmbot_os/platform/target/firmware_reset/gpio.ex

@@ -0,0 +1,34 @@
+defmodule FarmbotOS.Platform.Target.FirmwareReset.GPIO do
+  @moduledoc """
+  Uses GPIO pin 19 to reset the firmware.
+  """
+  @behaviour FarmbotFirmware.Reset
+
+  use GenServer
+  require Logger
+
+  @impl FarmbotFirmware.Reset
+  def reset(server \\ __MODULE__) do
+    Logger.debug("calling gpio reset/0")
+    GenServer.call(server, :reset)
+  end
+
+  @impl GenServer
+  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}
+    else
+      error -> {:reply, error, state}
+    end
+  end
+end

farmbot_os/platform/target/nerves_hub_client.ex

@@ -542,7 +542,7 @@ defmodule FarmbotOS.Platform.Target.NervesHubClient do
           "ota_hour = #{ota_hour || "null"} timezone = #{timezone || "null"}"
         )
 
-        !!auto_update
+        true
       end
 
     result && !currently_downloading?()

farmbot_os/test/farmbot_os/syscalls/resource_update_test.exs

@@ -31,10 +31,10 @@ 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 String.contains?(next_plant.name, "Updated to ")
+    assert "Updated to " == next_plant.name
 
     bad_result1 = ResourceUpdate.update_resource("Plant", 0, params)
-    error = "Plant.0 is not currently synced. Please re-sync."
+    error = "Plant.0 is not currently synced, so it could not be updated"
     assert {:error, error} == bad_result1
   end