Always provide metadata to encoding functions

lib/modboss.ex

@@ -363,24 +363,12 @@ defmodule ModBoss do
   defp get_encode_mfa(%Mapping{as: {module, as}} = mapping) do
     function = String.to_atom("encode_" <> "#{as}")
 
-    # In an effort to keep the API simple, when we call a user-defined encode function,
-    # we only pass the value to be encoded.
-    #
-    # However, when calling built-in encoding functions, we pass both the value to be encoded
-    # _and_ the mapping. We do this because in some cases we need to know how many objects
-    # we're encoding for in order to provide truly generic encoders. For example, when encoding
-    # a string to ASCII, we may need to add padding to fill out the mapped objects.
-    arguments =
-      case module do
-        ModBoss.Encoding -> [mapping.value, mapping]
-        _other -> [mapping.value]
-      end
-
-    if exists?(module, function, length(arguments)) do
-      {module, function, arguments}
+    if exists?(module, function, 2) do
+      metadata = ModBoss.Encoding.Metadata.from_mapping(mapping)
+      {module, function, [mapping.value, metadata]}
     else
       {:error,
-       "Modbus mapping #{inspect(mapping.name)} expected #{inspect(module)} to define #{inspect(function)}, but it did not."}
+       "Expected #{inspect(module)}.#{function}/2 to be defined for ModBoss mapping #{inspect(mapping.name)}."}
     end
   end
 
@@ -409,13 +397,12 @@ defmodule ModBoss do
 
   defp get_decode_mfa(%Mapping{as: {module, as}} = mapping) do
     function = String.to_atom("decode_" <> "#{as}")
-    arguments = [mapping.encoded_value]
 
-    if exists?(module, function, length(arguments)) do
-      {module, function, arguments}
+    if exists?(module, function, 1) do
+      {module, function, [mapping.encoded_value]}
     else
       {:error,
-       "Modbus mapping #{inspect(mapping.name)} expected #{inspect(module)} to define #{inspect(function)}, but it did not."}
+       "Expected #{inspect(module)}.#{function}/1 to be defined for ModBoss mapping #{inspect(mapping.name)}."}
     end
   end
 

lib/modboss/encoding.ex

@@ -17,9 +17,8 @@ defmodule ModBoss.Encoding do
   when providing your own `encode_*` functions, you'll only be passed the value to be encoded
   (and not the mapping).
   """
-  alias ModBoss.Mapping
-
   import Bitwise
+  alias ModBoss.Encoding.Metadata
 
   @doc false
   def encode_raw(value_or_values, _), do: {:ok, value_or_values}
@@ -38,9 +37,9 @@ defmodule ModBoss.Encoding do
       iex> encode_boolean(false, %{})
       {:ok, 0}
   """
-  @spec encode_boolean(boolean(), Mapping.t()) :: {:ok, integer()} | {:error, binary()}
-  def encode_boolean(true, _mapping), do: {:ok, 1}
-  def encode_boolean(false, _mapping), do: {:ok, 0}
+  @spec encode_boolean(boolean(), Metadata.t()) :: {:ok, integer()} | {:error, binary()}
+  def encode_boolean(true, _meta), do: {:ok, 1}
+  def encode_boolean(false, _meta), do: {:ok, 0}
 
   @doc """
   Interpret `1` as `true` and `0` as `false`
@@ -70,12 +69,12 @@ defmodule ModBoss.Encoding do
       iex> {:ok, 65_535} = encode_unsigned_int(65_535, %{})
       iex> {:error, _too_large} = encode_unsigned_int(65_536, %{})
   """
-  @spec encode_unsigned_int(integer(), Mapping.t()) :: {:ok, integer()} | {:error, binary()}
-  def encode_unsigned_int(value, _mapping) when value >= 0 and value <= 65_535 do
+  @spec encode_unsigned_int(integer(), Metadata.t()) :: {:ok, integer()} | {:error, binary()}
+  def encode_unsigned_int(value, _meta) when value >= 0 and value <= 65_535 do
     {:ok, value}
   end
 
-  def encode_unsigned_int(value, _mapping) do
+  def encode_unsigned_int(value, _meta) do
     {:error, "Value #{value} is outside the range of a 16-bit unsigned integer (0 to 65,535)"}
   end
 
@@ -106,13 +105,13 @@ defmodule ModBoss.Encoding do
       iex> {:ok, -32_768} = encode_signed_int(-32_768, %{})
       iex> {:error, _too_large} = encode_signed_int(32_768, %{})
   """
-  @spec encode_signed_int(integer(), Mapping.t()) :: {:ok, integer()} | {:error, binary()}
-  def encode_signed_int(value, _mapping)
+  @spec encode_signed_int(integer(), Metadata.t()) :: {:ok, integer()} | {:error, binary()}
+  def encode_signed_int(value, _meta)
       when is_integer(value) and value >= -32768 and value <= 32767 do
     {:ok, value}
   end
 
-  def encode_signed_int(value, _mapping) do
+  def encode_signed_int(value, _meta) do
     {:error, "Value #{value} is outside the range of a 16-bit signed integer (-32768 to 32767)"}
   end
 
@@ -143,49 +142,47 @@ defmodule ModBoss.Encoding do
 
   ## Examples
 
-      iex> encode_ascii("Hi!", %ModBoss.Mapping{address_count: 3})
+      iex> metadata = %ModBoss.Encoding.Metadata{type: :holding_register, address_count: 3}
+      iex> encode_ascii("Hi!", metadata)
       {:ok, [18537, 8448, 0]}
 
-      iex> {:error, _too_many_characters} = encode_ascii("Hi!", %ModBoss.Mapping{address_count: 1})
+      iex> metadata = %ModBoss.Encoding.Metadata{type: :holding_register, address_count: 1}
+      iex> {:error, reason} = encode_ascii("Hi!", metadata)
+      iex> String.match?(reason, ~r/Too many characters/)
+      true
   """
-  @spec encode_ascii(binary(), Mapping.t()) :: {:ok, list(integer())} | {:error, binary()}
-  def encode_ascii(text, %Mapping{} = mapping) do
-    with :ok <- verify_ascii(text),
+  @spec encode_ascii(binary(), Metadata.t()) :: {:ok, list(integer())} | {:error, binary()}
+  def encode_ascii(text, %Metadata{type: :holding_register} = meta) when is_binary(text) do
+    with :ok <- verify_ascii(text, meta),
          {:ok, chars} <- get_chars(text),
-         {:ok, padded_chars} <- pad(chars, mapping) do
-      do_encode_ascii(padded_chars)
+         {:ok, padded_chars} <- pad(chars, text, meta),
+         registers <- chars_to_registers(padded_chars) do
+      {:ok, registers}
     end
   end
 
-  defp do_encode_ascii(chars) when is_list(chars) do
-    chars
-    |> Enum.chunk_every(2, 2)
-    |> Enum.map(fn [upper_byte, lower_byte] -> (upper_byte <<< 8) + lower_byte end)
-    |> then(&{:ok, &1})
-  end
-
-  defp verify_ascii(text) do
+  defp verify_ascii(text, %Metadata{name: name}) do
     text
     |> String.to_charlist()
     |> Enum.all?(fn c -> c >= 0 and c < 127 end)
     |> case do
       true -> :ok
-      false -> {:error, "Text contains non-ASCII characters."}
+      false -> {:error, "Non-ASCII characters in #{text} for ModBoss mapping #{inspect(name)}."}
     end
   end
 
   defp get_chars(text) do
     {:ok, :binary.bin_to_list(text)}
   end
 
-  defp pad(chars, mapping) do
-    max_chars = mapping.address_count * 2
+  defp pad(chars, text, %Metadata{name: name, address_count: address_count}) do
+    max_chars = address_count * 2
     pad_count = max_chars - length(chars)
 
     if pad_count < 0 do
       message = """
-      Text for #{inspect(mapping.name)} contains too many characters. \
-      With #{mapping.address_count} registers, it can hold up to #{max_chars} ASCII characters.\
+      Too many characters in #{inspect(text)} for ModBoss mapping #{inspect(name)}. \
+      With #{address_count} registers, it can hold up to #{max_chars} ASCII characters.\
       """
 
       {:error, message}
@@ -194,6 +191,12 @@ defmodule ModBoss.Encoding do
     end
   end
 
+  defp chars_to_registers(chars) when is_list(chars) and rem(length(chars), 2) == 0 do
+    chars
+    |> Enum.chunk_every(2, 2)
+    |> Enum.map(fn [upper_byte, lower_byte] -> (upper_byte <<< 8) + lower_byte end)
+  end
+
   @doc """
   Decode integer `value`(s) representing ASCII characters to text
 

lib/modboss/encoding/metadata.ex

@@ -0,0 +1,25 @@
+defmodule ModBoss.Encoding.Metadata do
+  @moduledoc """
+  Metadata to support encoding
+
+  Sometimes it's essential to know how many registers you're encoding a value
+  to fit into. For example, when encoding ASCII text, you might choose to pad
+  unused bits with zeros. See the implementation of
+  `ModBoss.Encoding.encode_ascii/2` for an example.
+  """
+
+  alias ModBoss.Mapping
+
+  @type t() :: %__MODULE__{
+          name: Mapping.name(),
+          type: Mapping.type(),
+          address_count: pos_integer()
+        }
+
+  defstruct [:name, :type, :address_count]
+
+  @doc false
+  def from_mapping(%Mapping{} = mapping) do
+    struct!(__MODULE__, Map.take(mapping, [:name, :type, :address_count]))
+  end
+end

lib/modboss/mapping.ex

@@ -3,12 +3,15 @@ defmodule ModBoss.Mapping do
   Struct representing the Modbus mapping.
   """
 
+  @type name() :: atom()
+  @type type() :: :holding_register | :input_register | :coil | :discrete_input
+
   @type t() :: %__MODULE__{
-          name: atom(),
-          type: :holding_register | :input_register | :coil | :discrete_input,
+          name: name(),
+          type: type(),
           addresses: Range.t(),
-          starting_address: integer(),
-          address_count: integer(),
+          starting_address: non_neg_integer(),
+          address_count: pos_integer(),
           as: atom() | {module(), atom()},
           value: any(),
           encoded_value: integer() | [integer()],

test/modboss/encoding_test.exs

@@ -3,13 +3,13 @@ defmodule ModBoss.EncodingTest do
   doctest ModBoss.Encoding, import: true
 
   alias ModBoss.Encoding
-  alias ModBoss.Mapping
+  alias ModBoss.Encoding.Metadata
 
   describe "raw" do
     test "doesn't change the value when encoding" do
-      assert Encoding.encode_raw(123, %Mapping{}) == {:ok, 123}
-      assert Encoding.encode_raw(:abc, %Mapping{}) == {:ok, :abc}
-      assert Encoding.encode_raw([1, 2, 3], %Mapping{}) == {:ok, [1, 2, 3]}
+      assert Encoding.encode_raw(123, %Metadata{}) == {:ok, 123}
+      assert Encoding.encode_raw(:abc, %Metadata{}) == {:ok, :abc}
+      assert Encoding.encode_raw([1, 2, 3], %Metadata{}) == {:ok, [1, 2, 3]}
     end
 
     test "doesn't change the value when decoding" do
@@ -21,8 +21,8 @@ defmodule ModBoss.EncodingTest do
 
   describe "boolean" do
     test "encodes true and 1 and false as 0" do
-      assert Encoding.encode_boolean(true, %Mapping{}) == {:ok, 1}
-      assert Encoding.encode_boolean(false, %Mapping{}) == {:ok, 0}
+      assert Encoding.encode_boolean(true, %Metadata{}) == {:ok, 1}
+      assert Encoding.encode_boolean(false, %Metadata{}) == {:ok, 0}
     end
 
     test "decodes 1 as true and 0 as false" do
@@ -33,14 +33,14 @@ defmodule ModBoss.EncodingTest do
 
   describe "unsigned_int" do
     test "returns the value if within the range for 16 bits when encoding" do
-      assert Encoding.encode_unsigned_int(0, %Mapping{}) == {:ok, 0}
-      assert Encoding.encode_unsigned_int(65535, %Mapping{}) == {:ok, 65535}
+      assert Encoding.encode_unsigned_int(0, %Metadata{}) == {:ok, 0}
+      assert Encoding.encode_unsigned_int(65535, %Metadata{}) == {:ok, 65535}
     end
 
     test "returns an error for out-of-range values when encoding" do
-      assert {:error, _} = Encoding.encode_unsigned_int(-1, %Mapping{})
-      assert {:error, _} = Encoding.encode_unsigned_int(65536, %Mapping{})
-      assert {:error, _} = Encoding.encode_unsigned_int(:foo, %Mapping{})
+      assert {:error, _} = Encoding.encode_unsigned_int(-1, %Metadata{})
+      assert {:error, _} = Encoding.encode_unsigned_int(65536, %Metadata{})
+      assert {:error, _} = Encoding.encode_unsigned_int(:foo, %Metadata{})
     end
 
     test "returns the value when decoding" do
@@ -51,7 +51,7 @@ defmodule ModBoss.EncodingTest do
     test "roundtrips as expected" do
       [0, 65535]
       |> Enum.each(fn number ->
-        {:ok, encoded} = Encoding.encode_unsigned_int(number, %Mapping{})
+        {:ok, encoded} = Encoding.encode_unsigned_int(number, %Metadata{})
         {:ok, decoded} = Encoding.decode_unsigned_int(encoded)
         assert number == decoded
       end)
@@ -60,13 +60,13 @@ defmodule ModBoss.EncodingTest do
 
   describe "signed_int" do
     test "returns the value if within the range for 16 bits when encoding" do
-      assert {:ok, -32768} = Encoding.encode_signed_int(-32768, %Mapping{})
-      assert {:ok, 32767} = Encoding.encode_signed_int(32767, %Mapping{})
+      assert {:ok, -32768} = Encoding.encode_signed_int(-32768, %Metadata{})
+      assert {:ok, 32767} = Encoding.encode_signed_int(32767, %Metadata{})
     end
 
     test "returns error for out-of-range signed integers when encoding" do
-      assert {:error, _} = Encoding.encode_signed_int(-32769, %Mapping{})
-      assert {:error, _} = Encoding.encode_signed_int(32768, %Mapping{})
+      assert {:error, _} = Encoding.encode_signed_int(-32769, %Metadata{})
+      assert {:error, _} = Encoding.encode_signed_int(32768, %Metadata{})
     end
 
     test "interprets the value as a signed int when decoding" do
@@ -78,7 +78,7 @@ defmodule ModBoss.EncodingTest do
     test "roundtrips as expected" do
       [-32768, 0, 32767]
       |> Enum.each(fn initial ->
-        {:ok, encoded} = Encoding.encode_signed_int(initial, %Mapping{})
+        {:ok, encoded} = Encoding.encode_signed_int(initial, %Metadata{})
         {:ok, decoded} = Encoding.decode_signed_int(encoded)
         assert initial == decoded
       end)
@@ -87,20 +87,23 @@ defmodule ModBoss.EncodingTest do
 
   describe "ascii" do
     test "encodes ASCII text to the expected number of register values" do
-      assert {:ok, [0, 0, 0]} == Encoding.encode_ascii("", %Mapping{address_count: 3})
-      assert {:ok, [0x4100]} == Encoding.encode_ascii("A", %Mapping{address_count: 1})
-      assert {:ok, [0x4142]} == Encoding.encode_ascii("AB", %Mapping{address_count: 1})
-      assert {:ok, [0x4142, 0x4300]} == Encoding.encode_ascii("ABC", %Mapping{address_count: 2})
+      meta = %Metadata{name: :my_mapping, type: :holding_register}
+
+      assert {:ok, [0, 0, 0]} == Encoding.encode_ascii("", %{meta | address_count: 3})
+      assert {:ok, [0x4100]} == Encoding.encode_ascii("A", %{meta | address_count: 1})
+      assert {:ok, [0x4142]} == Encoding.encode_ascii("AB", %{meta | address_count: 1})
+      assert {:ok, [0x4142, 0x4300]} == Encoding.encode_ascii("ABC", %{meta | address_count: 2})
     end
 
     test "returns an error when attempting to encode more characters than what the mapped registers can hold" do
-      mapping = %Mapping{name: :my_mapping, address_count: 2}
-      assert {:error, message} = Encoding.encode_ascii("ABCDE", mapping)
-      assert String.match?(message, ~r/too many characters/)
+      meta = %Metadata{name: :some_mapping_name, type: :holding_register, address_count: 2}
+      assert {:error, message} = Encoding.encode_ascii("ABCDE", meta)
+      assert String.match?(message, ~r/Too many characters/)
     end
 
     test "returns an error when attempting to encode non-ASCII text" do
-      assert {:error, message} = Encoding.encode_ascii("José", %Mapping{address_count: 2})
+      meta = %Metadata{name: :my_mapping, type: :holding_register, address_count: 2}
+      assert {:error, message} = Encoding.encode_ascii("José", meta)
       assert String.match?(message, ~r/non-ASCII/i)
     end
 
@@ -117,7 +120,8 @@ defmodule ModBoss.EncodingTest do
     test "roundtrips as expected" do
       [{"", 1}, {"A", 1}, {"AB", 2}, {"ABC", 2}, {"ABCD", 2}, {"A", 4}]
       |> Enum.each(fn {text, count} ->
-        {:ok, encoded} = Encoding.encode_ascii(text, %Mapping{name: :foo, address_count: count})
+        meta = %Metadata{name: :my_mapping, type: :holding_register, address_count: count}
+        {:ok, encoded} = Encoding.encode_ascii(text, meta)
         {:ok, decoded} = Encoding.decode_ascii(encoded)
         assert text == decoded
       end)

test/modboss_test.exs

@@ -518,8 +518,8 @@ defmodule ModBossTest do
           holding_register 3..5, :text, as: {Encoding, :ascii}, mode: :w
         end
 
-        def encode_boolean(false), do: {:ok, 0}
-        def encode_boolean(true), do: {:ok, 1}
+        def encode_boolean(false, _), do: {:ok, 0}
+        def encode_boolean(true, _), do: {:ok, 1}
       end
       """)