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

@@ -5,6 +5,7 @@
  * Deprecate `resource_update` RPC
  * Introduce `update_resource` RPC, which allows users to modify variables from the sequence editor.
  * 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-rc17
+10.0.0

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,45 +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_resource(
-        unquote(Map.fetch!(args, :resource)),
-        unquote(unpair(body, %{})),
-        params
-      )
+    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_resource(%AST{kind: :identifier} = variable, update, env) do
-    {name, environ, nil} = Compiler.compile_ast(variable, env)
-    value = Keyword.fetch!(environ, name)
-    run_update_syscall(value, update)
-  end
-
-  def do_update_resource(%AST{kind: :resource} = res, update, _) do
-    run_update_syscall(res.args, update)
-  end
-
-  def do_update_resource(res, _, _) do
-    raise "update_resource error. Please notfiy support: #{inspect(res)}"
-  end
-
-  defp run_update_syscall(%{resource_id: id, resource_type: kind}, update_params) do
+  def do_update(%{pointer_id: id, pointer_type: kind}, update_params) do
     FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
   end
 
-  defp run_update_syscall(other, update) do
+  def do_update(%{resource_id: id, resource_type: kind}, update_params) do
+    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
+  end
+
+  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. Tried updating #{inspect(other)} to #{inspect(update)}
-    """)
+          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/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_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_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/test/farmbot_firmware/command_test.exs

@@ -16,6 +16,7 @@ defmodule FarmbotFirmware.CommandTest do
   @tag :capture_log
   test "command() runs RPCs" do
     pid = fake_pid()
+
     assert {:error, :emergency_lock} ==
              FarmbotFirmware.command(pid, {: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/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", _, 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/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

@@ -34,7 +34,7 @@ defmodule FarmbotOS.SysCalls.ResourceUpdateTest do
     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