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

@@ -1,5 +1,12 @@
 # Changelog
 
+# 10.0.0
+
+ * 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
 
  * Fix firmware locking error ("Can't perform X in Y state")
@@ -8,6 +15,9 @@
  * Unit test additions (+2.7% coverage :tada:)
  * Updates to build instructions for third party developers
  * Bug fix for criteria-based groups that have only one filter criteria.
+ * Bug fix for express bots involving timeout during remote firmware flash
+ * Remove VCR again (for now)
+ * Increase farmware timeout to 20 minutes (use at own risk)
 
 # 9.2.1
 

FEATURE_MIN_VERSIONS.json

@@ -5,6 +5,7 @@
   "assertion_block": "8.0.0",
   "backscheduled_regimens": "6.4.0",
   "change_ownership": "6.3.0",
+  "criteria_groups": "9.2.2",
   "diagnostic_dumps": "6.4.4",
   "endstop_invert": "6.4.1",
   "express_k10": "8.0.0",
@@ -20,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 @@
-9.2.2-rc8
+10.0.0

docs/target_development/releasing_target_firmware.md

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

farmbot_celery_script/fixture/corpus.json

@@ -22,13 +22,14 @@
     {
       "name": "ALLOWED_MESSAGE_TYPES",
       "allowed_values": [
-        "success",
+        "assertion",
         "busy",
-        "warn",
+        "debug",
         "error",
-        "info",
         "fun",
-        "debug"
+        "info",
+        "success",
+        "warn"
       ]
     },
     {
@@ -55,6 +56,15 @@
         1
       ]
     },
+    {
+      "name": "ALLOWED_ASSERTION_TYPES",
+      "allowed_values": [
+        "abort",
+        "recover",
+        "abort_recover",
+        "continue"
+      ]
+    },
     {
       "name": "AllowedPinTypes",
       "allowed_values": [
@@ -89,6 +99,7 @@
       "name": "LegalSequenceKind",
       "allowed_values": [
         "_if",
+        "assertion",
         "calibrate",
         "change_ownership",
         "check_updates",
@@ -109,7 +120,6 @@
         "read_status",
         "reboot",
         "remove_farmware",
-        "resource_update",
         "send_message",
         "set_servo_angle",
         "set_user_env",
@@ -117,6 +127,7 @@
         "take_photo",
         "toggle_pin",
         "update_farmware",
+        "update_resource",
         "wait",
         "write_pin",
         "zero"
@@ -206,7 +217,8 @@
         "planned",
         "planted",
         "harvested",
-        "sprouted"
+        "sprouted",
+        "removed"
       ]
     },
     {
@@ -214,24 +226,18 @@
       "allowed_values": [
         "GenericPointer",
         "ToolSlot",
-        "Plant"
+        "Plant",
+        "Weed"
       ]
     },
     {
       "name": "resource_type",
       "allowed_values": [
         "Device",
-        "FarmEvent",
-        "Image",
-        "Log",
-        "Peripheral",
-        "Plant",
         "Point",
-        "Regimen",
-        "Sequence",
-        "Tool",
+        "Plant",
         "ToolSlot",
-        "User",
+        "Weed",
         "GenericPointer"
       ]
     },
@@ -261,11 +267,13 @@
         "z",
         "pin_type",
         "pointer_id",
+        "point_group_id",
         "pointer_type",
         "pin_mode",
         "sequence_id",
         "lhs",
         "op",
+        "priority",
         "channel_name",
         "message_type",
         "tool_id",
@@ -273,19 +281,22 @@
         "axis",
         "message",
         "speed",
-        "resource_type"
+        "resource_type",
+        "assertion_type",
+        "lua",
+        "resource"
       ]
     },
     {
       "name": "LegalKindString",
       "allowed_values": [
+        "Assertion",
         "If",
         "Calibrate",
         "ChangeOwnership",
         "Channel",
         "CheckUpdates",
         "Coordinate",
-        "DumpInfo",
         "EmergencyLock",
         "EmergencyUnlock",
         "ExecuteScript",
@@ -331,7 +342,10 @@
         "MoveAbsolute",
         "WritePin",
         "ReadPin",
-        "ResourceUpdate"
+        "ResourceUpdate",
+        "Resource",
+        "UpdateResource",
+        "PointGroup"
       ]
     }
   ],
@@ -394,6 +408,10 @@
         {
           "tag": "identifier",
           "name": "identifier"
+        },
+        {
+          "tag": "point_group",
+          "name": "point_group"
         }
       ]
     },
@@ -625,6 +643,15 @@
         }
       ]
     },
+    {
+      "name": "point_group_id",
+      "allowed_values": [
+        {
+          "tag": "integer",
+          "name": "Integer"
+        }
+      ]
+    },
     {
       "name": "pointer_type",
       "allowed_values": [
@@ -674,6 +701,15 @@
         }
       ]
     },
+    {
+      "name": "priority",
+      "allowed_values": [
+        {
+          "tag": "integer",
+          "name": "Integer"
+        }
+      ]
+    },
     {
       "name": "channel_name",
       "allowed_values": [
@@ -745,9 +781,53 @@
           "name": "resource_type"
         }
       ]
+    },
+    {
+      "name": "assertion_type",
+      "allowed_values": [
+        {
+          "tag": "ALLOWED_ASSERTION_TYPES",
+          "name": "ALLOWED_ASSERTION_TYPES"
+        }
+      ]
+    },
+    {
+      "name": "lua",
+      "allowed_values": [
+        {
+          "tag": "string",
+          "name": "String"
+        }
+      ]
+    },
+    {
+      "name": "resource",
+      "allowed_values": [
+        {
+          "tag": "identifier",
+          "name": "identifier"
+        },
+        {
+          "tag": "resource",
+          "name": "resource"
+        }
+      ]
     }
   ],
   "nodes": [
+    {
+      "allowed_args": [
+        "assertion_type",
+        "_then",
+        "lua"
+      ],
+      "allowed_body_types": [],
+      "name": "assertion",
+      "tags": [
+        "*"
+      ],
+      "docs": ""
+    },
     {
       "allowed_args": [
         "lhs",
@@ -1151,9 +1231,11 @@
     },
     {
       "allowed_args": [
-        "label"
+        "label",
+        "priority"
       ],
       "allowed_body_types": [
+        "assertion",
         "calibrate",
         "change_ownership",
         "check_updates",
@@ -1175,7 +1257,7 @@
         "read_status",
         "reboot",
         "remove_farmware",
-        "resource_update",
+        "update_resource",
         "send_message",
         "set_servo_angle",
         "set_user_env",
@@ -1225,6 +1307,7 @@
         "locals"
       ],
       "allowed_body_types": [
+        "assertion",
         "calibrate",
         "change_ownership",
         "check_updates",
@@ -1246,7 +1329,7 @@
         "read_status",
         "reboot",
         "remove_farmware",
-        "resource_update",
+        "update_resource",
         "send_message",
         "set_servo_angle",
         "set_user_env",
@@ -1459,6 +1542,45 @@
         "network_user"
       ],
       "docs": ""
+    },
+    {
+      "allowed_args": [
+        "resource_type",
+        "resource_id"
+      ],
+      "allowed_body_types": [],
+      "name": "resource",
+      "tags": [
+        "network_user"
+      ],
+      "docs": ""
+    },
+    {
+      "allowed_args": [
+        "resource"
+      ],
+      "allowed_body_types": [
+        "pair"
+      ],
+      "name": "update_resource",
+      "tags": [
+        "function",
+        "api_writer",
+        "network_user"
+      ],
+      "docs": ""
+    },
+    {
+      "allowed_args": [
+        "point_group_id"
+      ],
+      "allowed_body_types": [],
+      "name": "point_group",
+      "tags": [
+        "data",
+        "list_like"
+      ],
+      "docs": ""
     }
   ]
-}
+}

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]
@@ -94,27 +87,28 @@ defmodule FarmbotCeleryScript.Compiler do
   defdelegate assertion(ast, env), to: Compiler.Assertion
   defdelegate calibrate(ast, env), to: Compiler.AxisControl
   defdelegate coordinate(ast, env), to: Compiler.DataControl
-  defdelegate execute(ast, env), to: Compiler.Execute
   defdelegate execute_script(ast, env), to: Compiler.Farmware
+  defdelegate execute(ast, env), to: Compiler.Execute
   defdelegate find_home(ast, env), to: Compiler.AxisControl
   defdelegate home(ast, env), to: Compiler.AxisControl
-  defdelegate unquote(:_if)(ast, env), to: Compiler.If
   defdelegate install_first_party_farmware(ast, env), to: Compiler.Farmware
   defdelegate move_absolute(ast, env), to: Compiler.AxisControl
   defdelegate move_relative(ast, env), to: Compiler.AxisControl
   defdelegate named_pin(ast, env), to: Compiler.DataControl
   defdelegate point(ast, env), to: Compiler.DataControl
   defdelegate read_pin(ast, env), to: Compiler.PinControl
-  defdelegate resource_update(ast, env), to: Compiler.DataControl
+  defdelegate resource(ast, env), to: Compiler.DataControl
   defdelegate rpc_request(ast, env), to: Compiler.RPCRequest
   defdelegate sequence(ast, env), to: Compiler.Sequence
   defdelegate set_pin_io_mode(ast, env), to: Compiler.PinControl
   defdelegate set_servo_angle(ast, env), to: Compiler.PinControl
   defdelegate set_user_env(ast, env), to: Compiler.Farmware
   defdelegate take_photo(ast, env), to: Compiler.Farmware
-  defdelegate tool(ast, env), to: Compiler.DataControl
   defdelegate toggle_pin(ast, env), to: Compiler.PinControl
+  defdelegate tool(ast, env), to: Compiler.DataControl
+  defdelegate unquote(:_if)(ast, env), to: Compiler.If
   defdelegate update_farmware(ast, env), to: Compiler.Farmware
+  defdelegate update_resource(ast, env), to: Compiler.UpdateResource
   defdelegate variable_declaration(ast, env), to: Compiler.VariableDeclaration
   defdelegate write_pin(ast, env), to: Compiler.PinControl
   defdelegate zero(ast, env), to: Compiler.AxisControl
@@ -280,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/data_control_compiler.ex

@@ -1,6 +1,19 @@
 defmodule FarmbotCeleryScript.Compiler.DataControl do
   alias FarmbotCeleryScript.Compiler
 
+  def resource(ast, _env) do
+    IO.puts("======")
+    IO.inspect(ast)
+    # %FarmbotCeleryScript.AST{
+    #   args: %{resource_id: 0, resource_type: "Device"},
+    #   body: [],
+    #   comment: nil,
+    #   kind: :resource,
+    #   meta: nil
+    # }
+    raise "TODO: Pull resource from DB?"
+  end
+
   # compiles coordinate
   # Coordinate should return a vec3
   def coordinate(%{args: %{x: x, y: y, z: z}}, env) do
@@ -40,35 +53,4 @@ defmodule FarmbotCeleryScript.Compiler.DataControl do
       )
     end
   end
-
-  def resource_update(
-        %{
-          args: %{
-            resource_type: kind,
-            resource_id: id,
-            label: label,
-            value: value
-          },
-          body: body
-        },
-        env
-      ) do
-    initial = %{label => value}
-    # Technically now body isn't supported by this node.
-    extra =
-      Map.new(body, fn %{args: %{label: label, data_value: value}} ->
-        {label, value}
-      end)
-
-    # Make sure the initial stuff higher most priority
-    params = Map.merge(extra, initial)
-
-    quote do
-      FarmbotCeleryScript.SysCalls.resource_update(
-        unquote(Compiler.compile_ast(kind, env)),
-        unquote(Compiler.compile_ast(id, env)),
-        unquote(Macro.escape(params))
-      )
-    end
-  end
 end

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

@@ -0,0 +1,61 @@
+defmodule FarmbotCeleryScript.Compiler.UpdateResource do
+  alias FarmbotCeleryScript.{AST, DotProps}
+
+  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(%{pointer_id: id, pointer_type: kind}, update_params) do
+    FarmbotCeleryScript.SysCalls.update_resource(kind, id, update_params)
+  end
+
+  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 (#{inspect(other)} / #{inspect(update)})
+          """)
+  end
+
+  defp unpair([pair | rest], acc) do
+    key = Map.fetch!(pair.args, :label)
+    val = Map.fetch!(pair.args, :value)
+    next_acc = Map.merge(acc, DotProps.create(key, val))
+    unpair(rest, next_acc)
+  end
+
+  defp unpair([], acc) do
+    acc
+  end
+end

farmbot_celery_script/lib/farmbot_celery_script/dot_props.ex

@@ -0,0 +1,22 @@
+defmodule FarmbotCeleryScript.DotProps do
+  @dot "."
+  @doc ~S"""
+  Takes a "dotted" key and val.
+  Returns deeply nested hash.
+
+  ## Examples
+
+      iex> create("foo.bar.baz", 321)
+      %{"foo" => %{"bar" => %{"baz" => 321}}}
+
+      iex> create("foo", "bar")
+      %{"foo" => "bar"}
+  """
+  def create(dotted, val) do
+    [key | list] = dotted |> String.split(@dot) |> Enum.reverse()
+
+    Enum.reduce(list, %{key => val}, fn next_key, acc ->
+      %{next_key => acc}
+    end)
+  end
+end

farmbot_celery_script/lib/farmbot_celery_script/sys_calls.ex

@@ -57,7 +57,6 @@ defmodule FarmbotCeleryScript.SysCalls do
               speed :: number()
             ) ::
               ok_or_error
-  # ?
   @callback named_pin(named_pin_type :: String.t(), resource_id) ::
               map() | integer | error()
   @callback nothing() :: any()
@@ -69,7 +68,6 @@ defmodule FarmbotCeleryScript.SysCalls do
   @callback toggle_pin(pin_num :: number()) :: ok_or_error
   @callback read_status() :: ok_or_error
   @callback reboot() :: ok_or_error
-  @callback resource_update(String.t(), resource_id, map()) :: ok_or_error
   @callback send_message(type :: String.t(), message :: String.t(), [atom]) ::
               ok_or_error
   @callback set_servo_angle(pin :: number(), value :: number()) :: ok_or_error
@@ -95,6 +93,8 @@ defmodule FarmbotCeleryScript.SysCalls do
   @callback find_points_via_group(String.t() | resource_id) :: %{
               required(:point_ids) => [resource_id]
             }
+  @callback update_resource(kind :: String.t(), resource_id, params :: map()) ::
+              ok_or_error
 
   def find_points_via_group(sys_calls \\ @sys_calls, point_group_id) do
     point_group_or_error(sys_calls, :find_points_via_group, [point_group_id])
@@ -297,10 +297,6 @@ defmodule FarmbotCeleryScript.SysCalls do
     ok_or_error(sys_calls, :reboot, [])
   end
 
-  def resource_update(sys_calls \\ @sys_calls, kind, id, params) do
-    ok_or_error(sys_calls, :resource_update, [kind, id, params])
-  end
-
   def send_message(sys_calls \\ @sys_calls, kind, msg, channels) do
     ok_or_error(sys_calls, :send_message, [kind, msg, channels])
   end
@@ -333,6 +329,10 @@ defmodule FarmbotCeleryScript.SysCalls do
     ok_or_error(sys_calls, :zero, [axis])
   end
 
+  def update_resource(sys_calls \\ @sys_calls, kind, id, params) do
+    ok_or_error(sys_calls, :update_resource, [kind, id, params])
+  end
+
   defp ok_or_error(sys_calls, fun, args) do
     case apply(sys_calls, fun, args) do
       :ok -> :ok

farmbot_celery_script/lib/farmbot_celery_script/sys_calls/stubs.ex

@@ -119,10 +119,6 @@ defmodule FarmbotCeleryScript.SysCalls.Stubs do
   @impl true
   def reboot(), do: error(:reboot, [])
 
-  @impl true
-  def resource_update(kind, resource_id, data),
-    do: error(:resource_update, [kind, resource_id, data])
-
   @impl true
   def send_message(type, message, channels),
     do: error(:send_message, [type, message, channels])
@@ -147,6 +143,10 @@ defmodule FarmbotCeleryScript.SysCalls.Stubs do
   def write_pin(pin_num, pin_mode, pin_value),
     do: error(:write_pin, [pin_num, pin_mode, pin_value])
 
+  @impl true
+  def update_resource(kind, id, params),
+    do: error(:update_resource, [kind, id, params])
+
   @impl true
   def zero(axis), do: error(:zero, [axis])
 

farmbot_celery_script/test/dot_props_test.exs

@@ -0,0 +1,4 @@
+defmodule FarmbotCeleryScript.DotPropsTest do
+  use ExUnit.Case
+  doctest FarmbotCeleryScript.DotProps, import: true
+end

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
              ]
              """)
@@ -355,6 +363,114 @@ defmodule FarmbotCeleryScript.CompilerTest do
              """)
   end
 
+  test "`update_resource`: " do
+    compiled =
+      "test/fixtures/mark_variable_removed.json"
+      |> File.read!()
+      |> Jason.decode!()
+      |> AST.decode()
+      |> compile()
+
+    assert compiled ==
+             strip_nl("""
+             [
+               fn params ->
+                 _ = inspect(params)
+
+                 unsafe_cGFyZW50 =
+                   Keyword.get(params, :unsafe_cGFyZW50, FarmbotCeleryScript.SysCalls.coordinate(1, 2, 3))
+
+                 better_params = %{}
+
+                 [
+                   fn ->
+                     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
+             ]
+             """)
+  end
+
+  test "`update_resource`: Multiple fields of `resource` type." do
+    compiled =
+      "test/fixtures/update_resource_multi.json"
+      |> File.read!()
+      |> Jason.decode!()
+      |> AST.decode()
+      |> compile()
+
+    assert compiled ==
+             strip_nl("""
+             [
+               fn params ->
+                 _ = inspect(params)
+                 better_params = %{}
+
+                 [
+                   fn ->
+                     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
+             ]
+             """)
+  end
+
   defp compile(ast) do
     ast
     |> Compiler.compile_ast([])

farmbot_celery_script/test/farmbot_celery_script/corpus/node_test.exs

@@ -4,7 +4,7 @@ defmodule FarmbotCeleryScript.Corpus.NodeTest do
 
   test "inspect" do
     a =
-      "Sequence(version, locals) [calibrate, change_ownership, check_updates, emergency_lock, emergency_unlock, execute, execute_script, factory_reset, find_home, flash_firmware, home, install_farmware, install_first_party_farmware, _if, move_absolute, move_relative, power_off, read_pin, read_status, reboot, remove_farmware, resource_update, send_message, set_servo_angle, set_user_env, sync, take_photo, toggle_pin, update_farmware, wait, write_pin, zero]"
+      "Sequence(version, locals) [assertion, calibrate, change_ownership, check_updates, emergency_lock, emergency_unlock, execute, execute_script, factory_reset, find_home, flash_firmware, home, install_farmware, install_first_party_farmware, _if, move_absolute, move_relative, power_off, read_pin, read_status, reboot, remove_farmware, update_resource, send_message, set_servo_angle, set_user_env, sync, take_photo, toggle_pin, update_farmware, wait, write_pin, zero]"
 
     b = inspect(Corpus.sequence())
     assert a == b

farmbot_celery_script/test/fixtures/mark_variable_meta.json

@@ -0,0 +1,48 @@
+{
+  "args": {
+    "locals": {
+      "args": {},
+      "body": [
+        {
+          "args": {
+            "default_value": {
+              "args": {
+                "x": 1,
+                "y": 2,
+                "z": 3
+              },
+              "kind": "coordinate"
+            },
+            "label": "parent"
+          },
+          "kind": "parameter_declaration"
+        }
+      ],
+      "kind": "scope_declaration"
+    },
+    "version": -999
+  },
+  "body": [
+    {
+      "args": {
+        "resource": {
+          "args": {
+            "label": "parent"
+          },
+          "kind": "identifier"
+        }
+      },
+      "body": [
+        {
+          "args": {
+            "label": "meta.my_prop",
+            "value": "whatever"
+          },
+          "kind": "pair"
+        }
+      ],
+      "kind": "update_resource"
+    }
+  ],
+  "kind": "sequence"
+}

farmbot_celery_script/test/fixtures/mark_variable_removed.json

@@ -0,0 +1,48 @@
+{
+  "args": {
+    "locals": {
+      "args": {},
+      "body": [
+        {
+          "args": {
+            "default_value": {
+              "args": {
+                "x": 1,
+                "y": 2,
+                "z": 3
+              },
+              "kind": "coordinate"
+            },
+            "label": "parent"
+          },
+          "kind": "parameter_declaration"
+        }
+      ],
+      "kind": "scope_declaration"
+    },
+    "version": -999
+  },
+  "body": [
+    {
+      "args": {
+        "resource": {
+          "args": {
+            "label": "parent"
+          },
+          "kind": "identifier"
+        }
+      },
+      "body": [
+        {
+          "args": {
+            "label": "plant_stage",
+            "value": "removed"
+          },
+          "kind": "pair"
+        }
+      ],
+      "kind": "update_resource"
+    }
+  ],
+  "kind": "sequence"
+}

farmbot_celery_script/test/fixtures/set_mounted_tool_id.json

@@ -0,0 +1,34 @@
+{
+  "args": {
+    "locals": {
+      "args": {},
+      "body": [],
+      "kind": "scope_declaration"
+    },
+    "version": -999
+  },
+  "body": [
+    {
+      "args": {
+        "resource": {
+          "args": {
+            "resource_id": 0,
+            "resource_type": "Device"
+          },
+          "kind": "resource"
+        }
+      },
+      "body": [
+        {
+          "args": {
+            "label": "mounted_tool_id",
+            "value": 12161
+          },
+          "kind": "pair"
+        }
+      ],
+      "kind": "update_resource"
+    }
+  ],
+  "kind": "sequence"
+}

farmbot_celery_script/test/fixtures/update_resource_multi.json

@@ -0,0 +1,41 @@
+{
+  "args": {
+    "locals": {
+      "args": {},
+      "body": [],
+      "kind": "scope_declaration"
+    },
+    "version": -999
+  },
+  "body": [
+    {
+      "args": {
+        "resource": {
+          "args": {
+            "resource_id": 23,
+            "resource_type": "Plant"
+          },
+          "kind": "resource"
+        }
+      },
+      "body": [
+        {
+          "args": {
+            "label": "plant_stage",
+            "value": "planted"
+          },
+          "kind": "pair"
+        },
+        {
+          "args": {
+            "label": "r",
+            "value": 23
+          },
+          "kind": "pair"
+        }
+      ],
+      "kind": "update_resource"
+    }
+  ],
+  "kind": "sequence"
+}

farmbot_core/config/config.exs

@@ -46,14 +46,7 @@ config :farmbot_core, FarmbotCore.EctoMigrator,
       "beta"
     )
 
-config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: []
-
-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,10 +27,13 @@ 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__) || []
-Keyword.get(config, :children, default)
+    config = (Application.get_env(:farmbot_ext, __MODULE__) || [])
+    Keyword.get(config, :children, default)
   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
 
@@ -323,8 +339,7 @@ defmodule FarmbotCore.Asset do
         # the DB / API.
         sorted = CriteriaRetriever.run(point_group)
           |> sort_points(sort_by || "xy_ascending")
-          |> Enum.map(&Map.fetch!(&1, :id))
-
+          |> Enum.map(fn point -> point.id end)
         %{ point_group | point_ids: sorted }
       other ->
         # Swallow all other errors

farmbot_core/lib/farmbot_core/asset/criteria_retriever.ex

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

farmbot_core/lib/farmbot_core/asset/point.ex

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

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

farmbot_core/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

@@ -3,17 +3,14 @@ defmodule FarmbotCore.FirmwareTTYDetector do
   require Logger
   alias Circuits.UART
 
-  @expected_names Application.get_env(:farmbot_core, __MODULE__)[:expected_names]
-  @expected_names ||
-    Mix.raise("""
-    Please configure `expected_names` for TTYDetector.
-
-        config :farmbot_core, FarmbotCore.FirmwareTTYDetector,
-          expected_names: ["ttyS0", "ttyNotReal"]
-    """)
-
   @error_retry_ms 5_000
 
+  if System.get_env("FARMBOT_TTY") do
+    @expected_names ["ttyUSB0", "ttyAMA0", "ttyACM0", System.get_env("FARMBOT_TTY")]
+  else
+    @expected_names ["ttyUSB0", "ttyAMA0", "ttyACM0"]
+  end
+
   @doc "Gets the detected TTY"
   def tty(server \\ __MODULE__) do
     GenServer.call(server, :tty)
@@ -53,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_core/priv/asset/migrations/20200411204400_add_pullout_direction_to_point.exs

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

farmbot_core/test/asset/criteria_retriever_test.exs

@@ -13,7 +13,7 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
 
   @fake_point_group %PointGroup{
     criteria: %{
-      "day" => %{"op" => "<", "days_ago" => 4},
+      "day" => %{"op" => ">", "days_ago" => 4},
       "string_eq" => %{
         "openfarm_slug" => ["five", "nine"],
         "meta.created_by" => ["plant-detection"]
@@ -124,8 +124,6 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
     expect(Timex, :now, fn -> @now end)
     pg = point_group_with_fake_points()
 
-    # This one is _almost_ a perfect match,
-    # but the meta field is a miss.
     point!(%{
       id: 888,
       created_at: @five_days_ago,
@@ -482,7 +480,6 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
         "string_eq" => %{}
       },
       id: 201,
-      local_id: "30856f5e-1f97-4e18-b5e0-84dc7cd9bbf0",
       name: "Test (Broke?)",
       point_ids: whitelist,
       sort_type: "xy_ascending",
@@ -494,4 +491,101 @@ defmodule FarmbotCore.Asset.CriteriaRetrieverTest do
     assert Enum.count(whitelist) == Enum.count(results)
     Enum.map(whitelist, fn id -> assert Enum.member?(results, id) end)
   end
+
+  test "edge case: Filter by crop type" do
+    Repo.delete_all(PointGroup)
+    Repo.delete_all(Point)
+    ok = point!(%{id: 1, pointer_type: "Plant", openfarm_slug: "spinach"})
+    point!(%{id: 2, pointer_type: "Plant", openfarm_slug: "beetroot"})
+    point!(%{id: 3, pointer_type: "Weed", openfarm_slug: "thistle"})
+    point!(%{id: 4, pointer_type: "Weed", openfarm_slug: "spinach"})
+
+    pg = %PointGroup{
+      :id => 241,
+      :point_ids => [],
+      :criteria => %{
+        "day" => %{
+          "op" => "<",
+          "days_ago" => 0
+        },
+        "string_eq" => %{
+          "pointer_type" => ["Plant"],
+          "openfarm_slug" => ["spinach"]
+        },
+        "number_eq" => %{},
+        "number_lt" => %{},
+        "number_gt" => %{}
+      }
+    }
+
+    ids = CriteriaRetriever.run(pg) |> Enum.map(fn p -> p.id end)
+    assert Enum.member?(ids, ok.id)
+    assert Enum.count(ids) == 1
+  end
+
+  test "edge case: Retrieves by `day` criteria only" do
+    Repo.delete_all(PointGroup)
+    Repo.delete_all(Point)
+    days_ago4 = Timex.shift(@now, days: -4)
+    days_ago2 = Timex.shift(@now, days: -2)
+    expect(Timex, :now, fn -> @now end)
+
+    point!(%{id: 1, pointer_type: "Plant", created_at: days_ago4})
+    p2 = point!(%{id: 2, pointer_type: "Plant", created_at: days_ago2})
+
+    pg1 = %PointGroup{
+      id: 212,
+      created_at: Timex.shift(@now, hours: -1),
+      updated_at: Timex.shift(@now, hours: -1),
+      name: "Less than 2 days ago",
+      point_ids: [],
+      sort_type: "yx_descending",
+      criteria: %{
+        day: %{"op" => "<", "days_ago" => 3},
+        string_eq: %{},
+        number_eq: %{},
+        number_lt: %{},
+        number_gt: %{}
+      }
+    }
+
+    ids = CriteriaRetriever.run(pg1) |> Enum.map(fn p -> p.id end)
+    assert Enum.count(ids) == 1
+    assert Enum.member?(ids, p2.id)
+  end
+
+  test "edge case: Filter by slot direction" do
+    Repo.delete_all(PointGroup)
+    Repo.delete_all(Point)
+
+    ok = point!(%{id: 1, pointer_type: "ToolSlot", pullout_direction: 3})
+    point!(%{id: 2, pointer_type: "Weed", pullout_direction: 3})
+    point!(%{id: 3, pointer_type: "ToolSlot", pullout_direction: 4})
+    point!(%{id: 4, pointer_type: "GenericPointer", pullout_direction: 2})
+
+    pg = %PointGroup{
+      :id => 242,
+      :name => "Filter by slot direction",
+      :point_ids => [],
+      :sort_type => "xy_ascending",
+      :criteria => %{
+        "day" => %{
+          "op" => "<",
+          "days_ago" => 0
+        },
+        "string_eq" => %{
+          "pointer_type" => ["ToolSlot"]
+        },
+        "number_eq" => %{
+          "pullout_direction" => [3]
+        },
+        "number_lt" => %{},
+        "number_gt" => %{}
+      }
+    }
+
+    ids = CriteriaRetriever.run(pg) |> Enum.map(fn p -> p.id end)
+    assert Enum.member?(ids, ok.id)
+    assert Enum.count(ids) == 1
+  end
 end

farmbot_ext/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/config/farmbot_core.exs

@@ -39,8 +39,6 @@ config :farmbot_core, FarmbotCore.EctoMigrator,
   default_currently_on_beta:
     String.contains?(to_string(:os.cmd('git rev-parse --abbrev-ref HEAD')), "beta")
 
-config :farmbot_core, FarmbotCore.FirmwareTTYDetector, expected_names: []
-
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 0
 
 config :farmbot_core, FarmbotCore.AssetWorker.FarmbotCore.Asset.FbosConfig,

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/farmbot_ext/amqp/auto_sync_channel_test.exs

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

farmbot_ext/test/test_helper.exs

@@ -21,13 +21,13 @@ 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 60
+  @wait_time 180
   # Base case: We have a pid
   def wait_for(pid) when is_pid(pid), do: check_on_mbox(pid)
   # Failure case: We failed to find a pid for a module.

farmbot_firmware/lib/farmbot_firmware.ex

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

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

farmbot_os/config/host/dev.exs

@@ -36,15 +36,7 @@ config :farmbot,
     FarmbotCore.Asset.Repo
   ]
 
-config :farmbot_core, FarmbotCore.FirmwareTTYDetector,
-  expected_names: [
-    System.get_env("FARMBOT_TTY")
-  ]
-
 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,11 +1,5 @@
 use Mix.Config
 
-config :farmbot_core, FarmbotCore.FirmwareTTYDetector,
-  expected_names: ["ttyUSB0", "ttyAMA0"]
-
-config :farmbot_firmware, FarmbotFirmware,
-  reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
-
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [
     FarmbotOS.Platform.Target.RTCWorker

farmbot_os/config/target/rpi0.exs

@@ -1,13 +1,7 @@
 use Mix.Config
 
-config :farmbot_core, FarmbotCore.FirmwareTTYDetector,
-  expected_names: ["ttyUSB0", "ttyAMA0"]
-
 config :farmbot_core, FarmbotCore.FirmwareOpenTask, attempt_threshold: 50
 
-config :farmbot_firmware, FarmbotFirmware,
-  reset: FarmbotOS.Platform.Target.FirmwareReset.GPIO
-
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [
     FarmbotOS.Platform.Target.RTCWorker

farmbot_os/config/target/rpi3.exs

@@ -1,9 +1,6 @@
 use Mix.Config
 
-config :farmbot_core, FarmbotCore.FirmwareTTYDetector,
-  expected_names: ["ttyUSB0", "ttyACM0"]
-
-config :farmbot_firmware, FarmbotFirmware, reset: FarmbotFirmware.NullReset
+config :farmbot_firmware, FarmbotFirmware, reset: FarmbotCore.FirmwareResetter
 
 config :farmbot, FarmbotOS.Init.Supervisor,
   init_children: [

farmbot_os/lib/avrdude.ex

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

farmbot_os/lib/farmbot_os/sys_calls.ex

@@ -84,7 +84,7 @@ defmodule FarmbotOS.SysCalls do
   defdelegate set_servo_angle(pin, angle), to: PinControl
 
   @impl true
-  defdelegate resource_update(kind, id, params), to: ResourceUpdate
+  defdelegate update_resource(kind, id, params), to: ResourceUpdate
 
   @impl true
   defdelegate get_current_x(), to: Movement
@@ -184,6 +184,7 @@ defmodule FarmbotOS.SysCalls do
 
   @impl true
   def firmware_reboot do
+    FarmbotCore.Logger.info(1, "Restarting firmware...")
     GenServer.stop(FarmbotFirmware, :reboot)
   end
 
@@ -204,12 +205,14 @@ defmodule FarmbotOS.SysCalls do
   @impl true
   def emergency_lock do
     _ = FarmbotFirmware.command({:command_emergency_lock, []})
+    FarmbotCore.Logger.error(1, "E-stopped")
     :ok
   end
 
   @impl true
   def emergency_unlock do
     _ = FarmbotFirmware.command({:command_emergency_unlock, []})
+    FarmbotCore.Logger.busy(1, "Unlocked")
     :ok
   end
 

farmbot_os/lib/farmbot_os/sys_calls/farmware.ex

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

farmbot_os/lib/farmbot_os/sys_calls/flash_firmware.ex

@@ -1,7 +1,7 @@
 defmodule FarmbotOS.SysCalls.FlashFirmware do
   @moduledoc false
 
-  alias FarmbotCore.{Asset, Asset.Private}
+  alias FarmbotCore.{Asset, Asset.Private, FirmwareResetter}
   alias FarmbotFirmware
   alias FarmbotCore.FirmwareTTYDetector
 
@@ -21,17 +21,15 @@ 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"),
-         {_, 0} <- Avrdude.flash(hex_file, tty, fun) do
-      FarmbotCore.Logger.success(2, "Firmware flashed successfully!")
-
+         _ <- finish_flashing(Avrdude.flash(hex_file, tty, fun)) do
       %{firmware_path: tty}
       |> Asset.update_fbos_config!()
       |> Private.mark_dirty!(%{})
@@ -42,11 +40,22 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
         {:error, reason}
 
       error ->
-        {:error, "flash_firmware misc error: #{inspect(error)}"}
+        {:error, "flash_firmware returned #{inspect(error)}"}
     end
   end
 
-  defp find_tty() do
+  def finish_flashing({_result, 0}) do
+    FarmbotCore.Logger.success(
+      1,
+      "Firmware flashed successfully. Unlock FarmBot to finish initialization."
+    )
+  end
+
+  def finish_flashing(result) do
+    FarmbotCore.Logger.debug(2, "AVR flash returned #{inspect(result)}")
+  end
+
+  def find_tty() do
     case FirmwareTTYDetector.tty() do
       nil ->
         {:error,
@@ -58,20 +67,4 @@ defmodule FarmbotOS.SysCalls.FlashFirmware do
         {:ok, tty}
     end
   end
-
-  defp find_reset_fun("express_k10") do
-    FarmbotCore.Logger.debug(3, "Using special reset function for express")
-    # "magic" workaround to avoid compiler warnings.
-    # We used to inject this via App config, but it was
-    # error prone.
-    mod = :"Elixir.FarmbotOS.Platform.Target.FirmwareReset.GPIO"
-    fun = &mod.reset/0
-    {:ok, fun}
-  end
-
-  defp find_reset_fun(_) do
-    FarmbotCore.Logger.debug(3, "Using default reset function")
-    fun = &FarmbotFirmware.NullReset.reset/0
-    {:ok, fun}
-  end
 end

farmbot_os/lib/farmbot_os/sys_calls/movement.ex

@@ -98,6 +98,12 @@ defmodule FarmbotOS.SysCalls.Movement do
   # TODO(Rick): Figure out source of Error: {:ok, "ok"} logs.
   def handle_movement_error({:ok, _}), do: :ok
 
+  def handle_movement_error(:emergency_lock) do
+    msg = "Cannot execute commands while E-stopped"
+    FarmbotCore.Logger.busy(1, msg)
+    {:error, msg}
+  end
+
   def handle_movement_error(reason) do
     msg = "Movement failed. #{inspect(reason)}"
     FarmbotCore.Logger.error(1, msg)

farmbot_os/lib/farmbot_os/sys_calls/point_lookup.ex

@@ -8,8 +8,21 @@ defmodule FarmbotOS.SysCalls.PointLookup do
 
   def point(kind, id) do
     case Asset.get_point(id: id) do
-      nil -> {:error, "#{kind} not found"}
-      %{name: name, x: x, y: y, z: z} -> %{name: name, x: x, y: y, z: z}
+      nil ->
+        {:error, "#{kind || "point"} #{id} not found"}
+
+      %{name: name, x: x, y: y, z: z, pointer_type: type} ->
+        %{
+          name: name,
+          resource_type: type,
+          resource_id: id,
+          x: x,
+          y: y,
+          z: z
+        }
+
+      other ->
+        Logger.debug("Point error: Please notify support #{inspect(other)}")
     end
   end
 

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,
@@ -10,9 +11,39 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
 
   alias FarmbotOS.SysCalls.SendMessage
 
-  @point_kinds ~w(Plant GenericPointer)
+  @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 resource_update("Device", 0, params) do
     params
     |> do_handlebars()
     |> Asset.update_device!()
@@ -21,12 +52,13 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
     :ok
   end
 
-  def resource_update(kind, id, params) when kind in @point_kinds do
+  def update_resource(kind, id, params) when kind in @point_kinds do
+    notify_user_of_updates(kind, params, id)
     params = do_handlebars(params)
-    point_resource_update(kind, id, params)
+    point_update_resource(kind, id, params)
   end
 
-  def resource_update(kind, id, _params) do
+  def update_resource(kind, id, _params) do
     {:error,
      """
      Unknown resource: #{kind}.#{id}
@@ -34,18 +66,26 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
   end
 
   @doc false
-  def point_resource_update(type, id, params) do
-    with %{} = point <- Asset.get_point(pointer_type: type, id: id),
+  def point_update_resource(type, id, params) do
+    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
 
@@ -59,7 +99,7 @@ defmodule FarmbotOS.SysCalls.ResourceUpdate do
 
           _ ->
             Logger.warn(
-              "failed to render #{key} => #{value} for resource_update"
+              "failed to render #{key} => #{value} for update_resource"
             )
 
             {key, value}

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/avrdude/avrdude_test.exs

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

farmbot_os/test/farmbot_os/syscalls/farmware_test.exs

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

farmbot_os/test/farmbot_os/syscalls/point_lookup_test.exs

@@ -17,7 +17,7 @@ defmodule FarmbotOS.SysCalls.PointLookupTest do
 
   test "failure cases" do
     err1 = PointLookup.point("GenericPointer", 24)
-    assert {:error, "GenericPointer not found"} == err1
+    assert {:error, "GenericPointer 24 not found"} == err1
 
     err2 = PointLookup.get_toolslot_for_tool(24)
     assert {:error, "Could not find point for tool by id: 24"} == err2
@@ -33,7 +33,9 @@ defmodule FarmbotOS.SysCalls.PointLookupTest do
       name: "test suite III",
       x: 1.2,
       y: 3.4,
-      z: 5.6
+      z: 5.6,
+      resource_id: 555,
+      resource_type: "GenericPointer"
     }
 
     p = point(expected)

farmbot_os/test/farmbot_os/syscalls/resource_update_test.exs

@@ -14,14 +14,14 @@ defmodule FarmbotOS.SysCalls.ResourceUpdateTest do
     end)
   end
 
-  test "resource_update/3 - Device" do
+  test "update_resource/3 - Device" do
     fake_coords!()
     params = %{name: "X is {{ x }}"}
-    assert :ok == ResourceUpdate.resource_update("Device", 0, params)
+    assert :ok == ResourceUpdate.update_resource("Device", 0, params)
     assert "X is 1.2" == FarmbotCore.Asset.device().name
   end
 
-  test "resource_update/3 - Point" do
+  test "update_resource/3 - Point" do
     Repo.delete_all(Point)
 
     %Point{id: 555, pointer_type: "Plant"}
@@ -29,17 +29,17 @@ defmodule FarmbotOS.SysCalls.ResourceUpdateTest do
     |> Repo.insert!()
 
     params = %{name: "Updated to {{ x }}"}
-    assert :ok == ResourceUpdate.resource_update("Plant", 555, params)
+    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.resource_update("Plant", 0, params)
-    error = "Plant.0 is not currently synced, so it could not be updated"
+    bad_result1 = ResourceUpdate.update_resource("Plant", 0, params)
+    error = "Plant.0 is not currently synced. Please re-sync."
     assert {:error, error} == bad_result1
   end
 
-  test "resource_update/3 - unknown" do
-    {:error, error} = ResourceUpdate.resource_update("Foo", 0, nil)
+  test "update_resource/3 - unknown" do
+    {:error, error} = ResourceUpdate.update_resource("Foo", 0, nil)
     assert error == "Unknown resource: Foo.0\n"
   end
 end