diff --git a/lib/modboss.ex b/lib/modboss.ex index 44db071..8e68617 100644 --- a/lib/modboss.ex +++ b/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 diff --git a/test/modboss_test.exs b/test/modboss_test.exs index 52b274c..73c82be 100644 --- a/test/modboss_test.exs +++ b/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()