Skip decoding if decode false

lib/modboss.ex

@@ -67,10 +67,13 @@ defmodule ModBoss do
         names when is_list(names) -> {names, :plural}
       end
 
+    should_decode = Keyword.get(opts, :decode, true)
+    field_to_return = if should_decode, do: :value, else: :encoded_value
+
     with {:ok, mappings} <- get_mappings(:readable, module, names),
          {:ok, mappings} <- read_mappings(module, mappings, read_func),
-         {:ok, mappings} <- decode(mappings) do
-      collect_results(mappings, plurality, opts)
+         {:ok, mappings} <- maybe_decode(mappings, should_decode) do
+      collect_results(mappings, plurality, field_to_return)
     end
   end
 
@@ -80,9 +83,7 @@ defmodule ModBoss do
     |> Enum.map(fn {name, _mapping} -> name end)
   end
 
-  defp collect_results(mappings, plurality, opts) do
-    field_to_return = if Keyword.get(opts, :decode, true), do: :value, else: :encoded_value
-
+  defp collect_results(mappings, plurality, field_to_return) do
     mappings
     |> Enum.map(&{&1.name, Map.get(&1, field_to_return)})
     |> then(fn results ->
@@ -106,7 +107,7 @@ defmodule ModBoss do
       iex> ModBoss.encode(MyDevice.Schema, foo: "Yay")
       {:ok, %{{:holding_register, 15} => 22881, {:holding_register, 16} => 30976}}
   """
-  @spec encode(module(), values()) :: {:ok, map()} | {:error, any()}
+  @spec encode(module(), values()) :: {:ok, map()} | {:error, String.t()}
   def encode(module, values) when is_atom(module) do
     with {:ok, mappings} <- get_mappings(:any, module, get_keys(values)),
          mappings <- put_values(mappings, values),
@@ -383,13 +384,19 @@ defmodule ModBoss do
     end
   end
 
-  @spec decode([Mapping.t()]) :: {:ok, [Mapping.t()]}
-  defp decode(mappings) do
+  @spec maybe_decode([Mapping.t()], boolean()) :: {:ok, [Mapping.t()]} | {:error, String.t()}
+  defp maybe_decode(mappings, false), do: {:ok, mappings}
+
+  defp maybe_decode(mappings, true) do
     Enum.reduce_while(mappings, {:ok, []}, fn mapping, {:ok, acc} ->
       case decode_value(mapping) do
         {:ok, decoded_value} ->
           updated_mapping = %{mapping | value: decoded_value}
           {:cont, {:ok, [updated_mapping | acc]}}
+
+        {:error, error} ->
+          message = "Failed to decode #{inspect(mapping.name)}. #{error}"
+          {:halt, {:error, message}}
       end
     end)
   end

test/modboss_test.exs

@@ -330,6 +330,37 @@ defmodule ModBossTest do
                ModBoss.read(schema, read_func(device), [:yep, :nope, :text])
     end
 
+    test "returns an error if decoding fails" do
+      schema = unique_module()
+
+      Code.compile_string("""
+      defmodule #{schema} do
+        use ModBoss.Schema
+
+        modbus_schema do
+          # Assumes the function lives in the current module…
+          holding_register 1, :yep, as: :boolean
+          holding_register 2, :nope, as: :boolean
+        end
+
+        def decode_boolean(0), do: {:ok, false}
+        def decode_boolean(1), do: {:ok, true}
+        def decode_boolean(value), do: {:error, "Not sure what to do with \#{value}."}
+      end
+      """)
+
+      device = start_supervised!({Agent, fn -> @initial_state end})
+
+      # only 1 and 2 are values expected to be decoded, so this will break…
+      set_objects(device, %{
+        1 => 1,
+        2 => 33
+      })
+
+      message = "Failed to decode :nope. Not sure what to do with 33."
+      assert {:error, ^message} = ModBoss.read(schema, read_func(device), [:yep, :nope])
+    end
+
     test "allows reading of 'raw' values" do
       schema = unique_module()
 
@@ -365,6 +396,34 @@ defmodule ModBossTest do
                ModBoss.read(schema, read_func(device), [:yep, :nope, :text], decode: false)
     end
 
+    test "bypasses (potentially-buggy) decode logic when asked to return raw values" do
+      schema = unique_module()
+
+      Code.compile_string("""
+      defmodule #{schema} do
+        use ModBoss.Schema
+
+        modbus_schema do
+          holding_register 1, :yep, as: :boolean
+          holding_register 2, :nope, as: :boolean
+        end
+
+        def decode_boolean(0), do: raise "bam!"
+        def decode_boolean(1), do: raise "kapow!"
+      end
+      """)
+
+      device = start_supervised!({Agent, fn -> @initial_state end})
+
+      set_objects(device, %{
+        1 => 1,
+        2 => 0
+      })
+
+      assert {:ok, %{yep: 1, nope: 0}} =
+               ModBoss.read(schema, read_func(device), [:yep, :nope], decode: false)
+    end
+
     test "fetches all readable mappings if told to read `:all`" do
       schema = unique_module()