Merge pull request #1203 from FarmBot/recovery_release

Late additions

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2.0
 defaults: &defaults
   working_directory: /nerves/build
   docker:
-    - image: nervesproject/nerves_system_br:latest
+    - image: nervesproject/nerves_system_br:1.11.3
 
 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,6 +21,7 @@
   "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,3 +28,14 @@ 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-rc3
+10.0.0

farmbot_celery_script/lib/farmbot_celery_script/compiler.ex

@@ -11,17 +11,10 @@ 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
-    Application.get_env(:farmbot_celery_script, __MODULE__)[:debug] || false
+    # Set this to `true` when debuging.
+    false
   end
 
   @valid_entry_points [:sequence, :rpc_request]
@@ -68,8 +61,8 @@ defmodule FarmbotCeleryScript.Compiler do
     compile_entry_point(compile_ast(ast, env), env, [])
   end
 
-  def compile_entry_point([{_, new_env, _} = compiled | rest], old_env, acc) do
-    env = Keyword.merge(old_env, new_env)
+  def compile_entry_point([{_, new_env, _} = compiled | rest], env, acc) do
+    env = Keyword.merge(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`
@@ -77,9 +70,7 @@ 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
-    result = Macro.to_string(compiled) |> Code.eval_string(new_env, __ENV__)
-
-    case result do
+    case Macro.to_string(compiled) |> Code.eval_string(new_env, __ENV__) do
       {fun, new_env} when is_function(fun, 1) ->
         env = Keyword.merge(env, new_env)
         compile_entry_point(rest, env, acc ++ apply(fun, [env]))
@@ -283,13 +274,13 @@ defmodule FarmbotCeleryScript.Compiler do
   end
 
   defp print_compiled_code(compiled) do
-    IO.puts("========")
+    IO.puts("=== START ===")
 
     compiled
     |> Macro.to_string()
     |> Code.format_string!()
     |> IO.puts()
 
-    IO.puts("--------\n\n")
+    IO.puts("=== END ===\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,5 +1,6 @@
 defmodule FarmbotCeleryScript.Compiler.Sequence do
   import FarmbotCeleryScript.Compiler.Utils
+  alias FarmbotCeleryScript.Compiler.IdentifierSanitizer
 
   @iterables [:point_group, :every_point]
 
@@ -29,13 +30,7 @@ defmodule FarmbotCeleryScript.Compiler.Sequence do
   def compile_sequence_iterable(
         iterable_ast,
         %{
-          args:
-            %{
-              locals:
-                %{
-                  body: params
-                } = locals
-            } = sequence_args,
+          args: %{locals: %{body: _} = locals} = sequence_args,
           meta: sequence_meta
         } = sequence_ast,
         env
@@ -43,31 +38,6 @@ 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
@@ -142,6 +112,37 @@ 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
@@ -150,6 +151,9 @@ 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
@@ -173,6 +177,8 @@ 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 ->
@@ -183,7 +189,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,34 +1,54 @@
 defmodule FarmbotCeleryScript.Compiler.UpdateResource do
-  alias FarmbotCeleryScript.{Compiler, AST, DotProps}
+  alias FarmbotCeleryScript.{AST, DotProps}
 
-  def update_resource(%AST{args: args, body: body}, env) do
-    quote do
-      unquote(__MODULE__).do_update(
-        unquote(Map.fetch!(args, :resource)),
-        unquote(unpair(body, %{})),
-        unquote(env)
-      )
+  def update_resource(%AST{args: args, body: body}, _env) do
+    quote location: :keep do
+      me = unquote(__MODULE__)
+      variable = unquote(Map.fetch!(args, :resource))
+      update = unquote(unpair(body, %{}))
+
+      # 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(%AST{kind: :identifier} = res, update, env) do
-    {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)
+  def do_update(%{pointer_id: id, pointer_type: kind}, update_params) do
+    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
   end
 
-  def do_update(%AST{kind: :resource} = res, update, _) do
-    %{resource_id: id, resource_type: kind} = res.args
-    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update)
+  def do_update(%{resource_id: id, resource_type: kind}, update_params) do
+    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
   end
 
-  def do_update(res, _, _) do
-    raise "update_resource error. Please notfiy support: #{inspect(res)}"
+  def do_update(%{args: %{pointer_id: id, pointer_type: k}}, update_params) do
+    FarmbotCeleryScript.SysCalls.update_resource(k, id, update_params)
+  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 =
-      <<157, 69, 5, 38, 188, 78, 10, 183, 154, 99, 151, 193, 214, 208, 187, 130,
-        183, 73, 13, 48>>
+      <<136, 140, 48, 226, 216, 155, 178, 103, 244, 88, 225, 146, 130, 216, 125,
+        72, 113, 195, 65, 1>>
 
     # 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(\"rm\", [\"-rf /*\"])"
+    label = "System.cmd(\"echo\", [\"lol\"])"
     value_ast = AST.Factory.new("coordinate", x: 1, y: 1, z: 1)
     identifier_ast = AST.Factory.new("identifier", label: label)
 
@@ -120,11 +120,19 @@ defmodule FarmbotCeleryScript.CompilerTest do
              [
                fn params ->
                  _ = inspect(params)
+                 unsafe_U3lzdGVtLmNtZCgiZWNobyIsIFsibG9sIl0p = FarmbotCeleryScript.SysCalls.coordinate(1, 1, 1)
 
-                 #{var_name} =
-                   FarmbotCeleryScript.SysCalls.coordinate(1, 1, 1)
+                 better_params = %{
+                   "System.cmd(\\"echo\\", [\\"lol\\"])" => %FarmbotCeleryScript.AST{
+                     args: %{x: 1, y: 1, z: 1},
+                     body: [],
+                     comment: nil,
+                     kind: :coordinate,
+                     meta: nil
+                   }
+                 }
 
-                 [fn -> #{var_name} end]
+                 [fn -> unsafe_U3lzdGVtLmNtZCgiZWNobyIsIFsibG9sIl0p end]
                end
              ]
              """)
@@ -372,19 +380,38 @@ defmodule FarmbotCeleryScript.CompilerTest do
                  unsafe_cGFyZW50 =
                    Keyword.get(params, :unsafe_cGFyZW50, FarmbotCeleryScript.SysCalls.coordinate(1, 2, 3))
 
+                 better_params = %{}
+
                  [
                    fn ->
-                     FarmbotCeleryScript.Compiler.UpdateResource.do_update(
-                       %FarmbotCeleryScript.AST{
-                         args: %{label: "parent"},
-                         body: [],
-                         comment: nil,
-                         kind: :identifier,
-                         meta: nil
-                       },
-                       %{"plant_stage" => "removed"},
-                       []
-                     )
+                     me = FarmbotCeleryScript.Compiler.UpdateResource
+
+                     variable = %FarmbotCeleryScript.AST{
+                       args: %{label: "parent"},
+                       body: [],
+                       comment: nil,
+                       kind: :identifier,
+                       meta: nil
+                     }
+
+                     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
@@ -405,20 +432,38 @@ defmodule FarmbotCeleryScript.CompilerTest do
              [
                fn params ->
                  _ = inspect(params)
+                 better_params = %{}
 
                  [
                    fn ->
-                     FarmbotCeleryScript.Compiler.UpdateResource.do_update(
-                       %FarmbotCeleryScript.AST{
-                         args: %{resource_id: 23, resource_type: "Plant"},
-                         body: [],
-                         comment: nil,
-                         kind: :resource,
-                         meta: nil
-                       },
-                       %{"plant_stage" => "planted", "r" => 23},
-                       []
-                     )
+                     me = FarmbotCeleryScript.Compiler.UpdateResource
+
+                     variable = %FarmbotCeleryScript.AST{
+                       args: %{resource_id: 23, resource_type: "Plant"},
+                       body: [],
+                       comment: nil,
+                       kind: :resource,
+                       meta: nil
+                     }
+
+                     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,12 +46,7 @@ config :farmbot_core, FarmbotCore.EctoMigrator,
       "beta"
     )
 
-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
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotCore.FirmwareResetter
 
 import_config "ecto.exs"
 import_config "logger.exs"

farmbot_core/config/dev.exs

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

farmbot_core/lib/farmbot_core/asset.ex

@@ -6,15 +6,15 @@ defmodule FarmbotCore.Asset do
   """
 
   alias FarmbotCore.Asset.{
-    Repo,
+    CriteriaRetriever,
     Device,
     DeviceCert,
-    FarmwareEnv,
-    FirstPartyFarmware,
-    FarmwareInstallation,
     FarmEvent,
+    FarmwareEnv,
+    FarmwareInstallation,
     FbosConfig,
     FirmwareConfig,
+    FirstPartyFarmware,
     Peripheral,
     PinBinding,
     Point,
@@ -22,11 +22,11 @@ defmodule FarmbotCore.Asset do
     PublicKey,
     Regimen,
     RegimenInstance,
-    Sequence,
+    Repo,
     Sensor,
     SensorReading,
+    Sequence,
     Tool,
-    CriteriaRetriever
   }
 
   alias FarmbotCore.AssetSupervisor
@@ -251,8 +251,24 @@ defmodule FarmbotCore.Asset do
   end
 
   def update_point(point, params) do
-    point
-    |> Point.changeset(params)
+    # TODO: RC 8 MAY 2020 - We need to hard refresh the point.
+    #       The CSVM appears to be caching resources. This leads
+    #       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,16 +11,9 @@ 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]
+  @firmware_flash_attempt_threshold Application.get_env(:farmbot_core, __MODULE__)[:firmware_flash_attempt_threshold] || 5
   @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,14 +9,7 @@ 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]
-  @attempt_threshold || Mix.raise """
-  Firmware open attempt threshold not configured:
-
-  config :farmbot_core, FarmbotCore.FirmwareOpenTask, [
-    attempt_threshold: 10
-  ]
-  """
+  @attempt_threshold Application.get_env(:farmbot_core, __MODULE__)[:attempt_threshold] || 5
 
   @doc false
   def start_link(args, opts \\ [name: __MODULE__]) do
@@ -25,7 +18,10 @@ defmodule FarmbotCore.FirmwareOpenTask do
 
   @doc false
   def swap_transport(tty) do
-    Application.put_env(:farmbot_firmware, FarmbotFirmware, transport: UARTTransport, device: tty)
+    Application.put_env(:farmbot_firmware, FarmbotFirmware,
+    transport: UARTTransport,
+    device: tty,
+    reset: FarmbotCore.FirmwareResetter)
     # Swap transport on FW module.
     # Close tranpsort if it is open currently.
     _ = FarmbotFirmware.close_transport()
@@ -33,7 +29,9 @@ defmodule FarmbotCore.FirmwareOpenTask do
   end
 
   def unswap_transport() do
-    Application.put_env(:farmbot_firmware, FarmbotFirmware, transport: StubTransport)
+    Application.put_env(:farmbot_firmware, FarmbotFirmware,
+    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

@@ -0,0 +1,46 @@
+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 [System.get_env("FARMBOT_TTY")]
+    @expected_names ["ttyUSB0", "ttyAMA0", "ttyACM0", System.get_env("FARMBOT_TTY")]
   else
     @expected_names ["ttyUSB0", "ttyAMA0", "ttyACM0"]
   end
@@ -50,7 +50,6 @@ 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,8 +2,6 @@ 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,8 +6,9 @@ defmodule FarmbotExt.API.DirtyWorker do
   import API.View, only: [render: 2]
 
   require Logger
+  require FarmbotCore.Logger
   use GenServer
-  @timeout 10000
+  @timeout 500
 
   # these resources can't be accessed by `id`.
   @singular [
@@ -34,84 +35,69 @@ defmodule FarmbotExt.API.DirtyWorker do
 
   @impl GenServer
   def init(args) do
-    # Logger.disable(self())
     module = Keyword.fetch!(args, :module)
-    timeout = Keyword.get(args, :timeout, @timeout)
-    {:ok, %{module: module, timeout: timeout}, timeout}
+    Process.send_after(self(), :do_work, @timeout)
+    {:ok, %{module: module}}
   end
 
   @impl GenServer
-  def handle_info(:timeout, %{module: module} = state) do
-    dirty = Private.list_dirty(module)
-    local = Private.list_local(module)
-    {:noreply, state, {:continue, Enum.uniq(dirty ++ local)}}
+  def handle_info(:do_work, %{module: module} = state) do
+    Process.sleep(@timeout)
+    list = Enum.uniq(Private.list_dirty(module) ++ Private.list_local(module))
+
+    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
 
-  @impl GenServer
-  def handle_continue([], state) do
-    {:noreply, state, state.timeout}
-  end
+  def work(dirty, module) do
+    # Go easy on the API
+    Process.sleep(333)
 
-  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
+    case http_request(dirty, module) do
       # Valid data
       {:ok, %{status: s, body: body}} when s > 199 and s < 300 ->
-        # Logger.debug(
-        #   "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP request complete: #{s} ok"
-        # )
-
-        dirty |> module.changeset(body) |> handle_changeset(rest, state)
+        dirty |> module.changeset(body) |> handle_changeset(module)
 
       # Invalid data
       {:ok, %{status: s, body: %{} = body}} when s > 399 and s < 500 ->
-        # Logger.debug(
-        #   "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP request complete: #{s} error+body"
-        # )
-
+        FarmbotCore.Logger.error(2, "HTTP Error #{s}. #{inspect(body)}")
         changeset = module.changeset(dirty)
 
         Enum.reduce(body, changeset, fn {key, val}, changeset ->
           Ecto.Changeset.add_error(changeset, key, val)
         end)
-        |> handle_changeset(rest, state)
+        |> handle_changeset(module)
 
       # Invalid data, but the API didn't say why
       {:ok, %{status: s, body: _body}} when s > 399 and s < 500 ->
-        # Logger.debug(
-        #   "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP request complete: #{s} error"
-        # )
+        FarmbotCore.Logger.error(2, "HTTP Error #{s}. #{inspect(dirty)}")
 
         module.changeset(dirty)
         |> Map.put(:valid?, false)
-        |> handle_changeset(rest, state)
+        |> handle_changeset(module)
 
       # HTTP Error. (500, network error, timeout etc.)
       error ->
-        Logger.error(
-          "[#{module} #{dirty.local_id} #{inspect(self())}] HTTP Error: #{state.module} #{
+        FarmbotCore.Logger.error(
+          2,
+          "[#{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, rest, state) do
-    # Logger.info("Successfully synced: #{state.module}")
-
-    Repo.update!(changeset)
-    |> Private.mark_clean!()
-
-    {:noreply, state, {:continue, rest}}
+  def handle_changeset(%{valid?: true} = changeset, _module) do
+    Private.mark_clean!(Repo.update!(changeset))
+    :ok
   end
 
-  # 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
+  def handle_changeset(%{valid?: false, data: data} = changeset, module) do
     message =
       Enum.map(changeset.errors, fn
         {key, {msg, _meta}} when is_binary(key) -> "\t#{key}: #{msg}"
@@ -119,29 +105,64 @@ defmodule FarmbotExt.API.DirtyWorker do
       end)
       |> Enum.join("\n")
 
-    Logger.error("Failed to sync: #{state.module} \n #{message}")
+    FarmbotCore.Logger.error(3, "Failed to sync: #{module} \n #{message}")
     _ = Repo.delete!(data)
-    {:noreply, state, {:continue, rest}}
+    :ok
   end
 
-  defp http_request(%{id: nil} = dirty, state) do
-    # Logger.debug("#{state.module} clean request (post)")
-    path = state.module.path()
-    data = render(state.module, dirty)
+  defp http_request(%{id: nil} = dirty, module) do
+    path = module.path()
+    data = render(module, dirty)
     API.post(API.client(), path, data)
   end
 
-  defp http_request(dirty, %{module: module} = state) when module in @singular do
-    # Logger.debug("#{state.module} dirty request (patch)")
-    path = path = state.module.path()
-    data = render(state.module, dirty)
+  defp http_request(dirty, module) when module in @singular do
+    path = path = module.path()
+    data = render(module, dirty)
     API.patch(API.client(), path, data)
   end
 
-  defp http_request(dirty, state) do
-    # Logger.debug("#{state.module} dirty request (patch)")
-    path = Path.join(state.module.path(), to_string(dirty.id))
-    data = render(state.module, dirty)
+  defp http_request(dirty, module) do
+    path = Path.join(module.path(), to_string(dirty.id))
+    data = render(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,8 +107,7 @@ defmodule FarmbotFirmware do
     :command_queue,
     :caller_pid,
     :current,
-    :reset,
-    :reset_pid
+    :reset
   ]
 
   @type state :: %State{
@@ -123,8 +122,7 @@ defmodule FarmbotFirmware do
           command_queue: [{pid(), GCODE.t()}],
           caller_pid: nil | pid,
           current: nil | GCODE.t(),
-          reset: module(),
-          reset_pid: nil | pid()
+          reset: module()
         }
 
   @doc """
@@ -202,16 +200,7 @@ defmodule FarmbotFirmware do
     args = Keyword.merge(args, global)
     transport = Keyword.fetch!(args, :transport)
     side_effects = Keyword.get(args, :side_effects)
-    # This is probably the cause of
-    # https://github.com/FarmBot/farmbot_os/issues/1111
-    # FarmbotFirmware.NullReset (RPi3? Safe default?)
-    #  -OR-
-    # FarmbotOS.Platform.Target.FirmwareReset.GPIO (RPi0, RPi)
-    #  -OR-
-    # Use Application.get_env to find target?
-    # probably?
-    reset = Keyword.get(args, :reset) || FarmbotFirmware.NullReset
-
+    reset = Keyword.fetch!(args, :reset)
     # Add an anon function that transport implementations should call.
     fw = self()
     fun = fn {_, _} = code -> GenServer.cast(fw, code) end
@@ -225,7 +214,6 @@ defmodule FarmbotFirmware do
       side_effects: side_effects,
       status: :transport_boot,
       reset: reset,
-      reset_pid: nil,
       command_queue: [],
       configuration_queue: []
     }
@@ -242,24 +230,6 @@ defmodule FarmbotFirmware do
       GenServer.stop(state.transport_pid)
   end
 
-  def handle_info(:timeout, %{status: :transport_boot, reset_pid: nil} = state) do
-    case GenServer.start_link(state.reset, state.transport_args,
-           name: state.reset
-         ) do
-      {:ok, pid} ->
-        {:noreply, %{state | reset_pid: pid}}
-
-      # TODO(Rick): I have no idea what's going on here.
-      {:error, {:already_started, pid}} ->
-        {:noreply, %{state | reset_pid: pid}}
-
-      error ->
-        Logger.error("Error starting Firmware Reset: #{inspect(error)}")
-        Process.send_after(self(), :timeout, @transport_init_error_retry_ms)
-        {:noreply, state}
-    end
-  end
-
   # This will be the first message received right after `init/1`
   # It should try to open a transport every `transport_init_error_retry_ms`
   # until success.

farmbot_firmware/lib/farmbot_firmware/null_reset.ex

@@ -1,15 +0,0 @@
-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: "HIGH"
-  def format_high_low_inverted(val) when val == 1, do: "LOW"
+  def format_high_low_inverted(val) when val == 0, do: "ON"
+  def format_high_low_inverted(val) when val == 1, do: "OFF"
 end

farmbot_firmware/lib/farmbot_firmware/reset.ex

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

farmbot_firmware/test/farmbot_firmware/command_test.exs

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

farmbot_firmware/test/farmbot_firmware/transports/uart_transport_test.exs

@@ -14,7 +14,7 @@ defmodule FarmbotFirmware.UARTTransportTest do
     init_args = [
       device: :FAKE_DEVICE,
       handle_gcode: :FAKE_GCODE_HANDLER,
-      reset: :FAKE_RESETER
+      reset: StubReset
     ]
 
     {: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]
+    arg = [transport: FarmbotFirmware.StubTransport, reset: StubReset]
     {: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, "HIGH"})
+    t(:pin_guard_5_active_state, 0, {"pin guard 5 safe state", nil, "ON"})
     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, "HIGH"})
+    t(:pin_guard_4_active_state, 0, {"pin guard 4 safe state", nil, "ON"})
     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, "HIGH"})
+    t(:pin_guard_3_active_state, 0, {"pin guard 3 safe state", nil, "ON"})
     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, "HIGH"})
+    t(:pin_guard_2_active_state, 0, {"pin guard 2 safe state", nil, "ON"})
     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, "HIGH"})
+    t(:pin_guard_1_active_state, 0, {"pin guard 1 safe state", nil, "ON"})
     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,8 +80,6 @@ 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,8 +38,5 @@ 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,8 +35,6 @@ 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,9 +116,6 @@ 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,13 +1,6 @@
 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,13 +2,6 @@ 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,7 +1,6 @@
 use Mix.Config
 
-config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
-config :farmbot, FarmbotOS.SysCalls.FlashFirmware, gpio: Circuits.GPIO
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotCore.FirmwareResetter
 
 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,27 +1,10 @@
 defmodule FarmbotOS.SysCalls.FlashFirmware do
   @moduledoc false
 
-  alias FarmbotCore.{Asset, Asset.Private}
+  alias FarmbotCore.{Asset, Asset.Private, FirmwareResetter}
   alias FarmbotFirmware
   alias FarmbotCore.FirmwareTTYDetector
 
-  defmodule Stub do
-    require FarmbotCore.Logger
-
-    def fail do
-      m = "No express function found. Please notify support."
-      FarmbotCore.Logger.error(3, m)
-      {:error, m}
-    end
-
-    def open(_, _), do: fail()
-    def write(_, _), do: fail()
-  end
-
-  # This only matter for express.
-  # When it's an express, use Circuits.GPIO.
-  @gpio Application.get_env(:farmbot, __MODULE__, [])[:gpio] || Stub
-
   import FarmbotFirmware.PackageUtils,
     only: [find_hex_file: 1, package_to_string: 1]
 
@@ -38,11 +21,11 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
          {:ok, tty} <- find_tty(),
          _ <-
            FarmbotCore.Logger.debug(3, "found tty: #{tty} for firmware flash"),
-         {:ok, fun} <- find_reset_fun(package),
+         {:ok, fun} <- FirmwareResetter.find_reset_fun(package),
          _ <-
            FarmbotCore.Logger.debug(
              3,
-             "closing firmware transport before flash"
+             "Closing the firmware transport before flash"
            ),
          :ok <- FarmbotFirmware.close_transport(),
          _ <- FarmbotCore.Logger.debug(3, "starting firmware flash"),
@@ -84,32 +67,4 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
         {:ok, tty}
     end
   end
-
-  def find_reset_fun("express_k10") do
-    FarmbotCore.Logger.debug(3, "Using special express reset function")
-    {:ok, fn -> express_reset_fun() end}
-  end
-
-  def find_reset_fun(_) do
-    FarmbotCore.Logger.debug(3, "Using default reset function")
-    {:ok, &FarmbotFirmware.NullReset.reset/0}
-  end
-
-  def express_reset_fun() do
-    try do
-      FarmbotCore.Logger.debug(3, "Begin MCU reset")
-      {:ok, gpio} = @gpio.open(19, :output)
-      :ok = @gpio.write(gpio, 0)
-      :ok = @gpio.write(gpio, 1)
-      Process.sleep(1000)
-      :ok = @gpio.write(gpio, 0)
-      FarmbotCore.Logger.debug(3, "Finish MCU Reset")
-      :ok
-    rescue
-      ex ->
-        message = Exception.message(ex)
-        Logger.error("Could not flash express firmware: #{message}")
-        :express_reset_error
-    end
-  end
 end

farmbot_os/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,6 +2,7 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
   @moduledoc false
 
   require Logger
+  require FarmbotCore.Logger
 
   alias FarmbotCore.{
     Asset,
@@ -11,8 +12,38 @@ 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!()
@@ -22,6 +53,7 @@ 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
@@ -35,17 +67,25 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
 
   @doc false
   def point_update_resource(type, id, params) do
-    with %{} = point <- Asset.get_point(pointer_type: type, id: id),
+    with %{} = point <- Asset.get_point(id: id),
          {:ok, point} <- Asset.update_point(point, params) do
       _ = Private.mark_dirty!(point)
       :ok
     else
       nil ->
-        {:error,
-         "#{type}.#{id} is not currently synced, so it could not be updated"}
+        msg = "#{type}.#{id} is not currently synced. Please re-sync."
+        FarmbotCore.Logger.error(3, msg)
+        {:error, msg}
 
       {:error, _changeset} ->
-        {:error, "Failed to update #{type}.#{id}"}
+        msg =
+          "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

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