From e2cd45041b474f02a36b3d5b19400e2ad1ab99a2 Mon Sep 17 00:00:00 2001 From: alde103 Date: Fri, 3 Jul 2026 18:51:26 -0600 Subject: [PATCH 1/6] Modbus TCP Client: handle multiple responses in a single tcp packet (#14) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01V9dLEMSgBybY4Xe9szthoQ --- CHANGELOG.md | 4 +++ lib/tcp/client.ex | 51 +++++++++++++++++++---------- test/tcp/modbus_tcp_client_test.exs | 28 +++++++++++++++- 3 files changed, 65 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba253c4..e3df922 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.3.14 + +* Modbus TCP Client (active mode): handles multiple responses received in a single tcp packet. + ## 0.3.13 * Modbus TCP: the protocol identifier other than Modbus (0x00, 0x00) has been relaxed and logged. diff --git a/lib/tcp/client.ex b/lib/tcp/client.ex index 977479f..e57ce08 100644 --- a/lib/tcp/client.ex +++ b/lib/tcp/client.ex @@ -316,37 +316,54 @@ defmodule Modbux.Tcp.Client do Logger.debug("(#{__MODULE__}, :message_active) response: #{inspect(response)}") Logger.debug("(#{__MODULE__}, :message_active) state: #{inspect(state)}") - h = :binary.at(response, 0) - l = :binary.at(response, 1) - transid = h * 256 + l + new_state = process_responses(response, state) + {:noreply, new_state} + end + + def handle_info({:tcp_closed, _port}, state) do + Logger.info("(#{__MODULE__}, :tcp_close) Server close the port") + new_state = close_socket(state) + {:noreply, new_state} + end + + def handle_info(msg, state) do + Logger.error("(#{__MODULE__}, :random_msg) msg: #{inspect(msg)}") + {:noreply, state} + end + + # A tcp packet may carry several responses, each one framed by the length field of its MBAP header. + defp process_responses(<<>>, state), do: state + + defp process_responses(<>, state) + when byte_size(tail) >= len do + <> = tail + response = <> + new_state = process_response(transid, response, state) + process_responses(rest, new_state) + end + + defp process_responses(response, state) do + Logger.error("(#{__MODULE__}, :message_active) incomplete response: #{inspect(response, base: :hex)}") + state + end + + defp process_response(transid, response, state) do Logger.debug("(#{__MODULE__}, :message_active) transid: #{inspect(transid)}") case Map.fetch(state.pending_msg, transid) do :error -> Logger.error("(#{__MODULE__}, :message_active) unknown transaction id") - {:noreply, state} + state {:ok, cmd} -> values = Tcp.parse_res(cmd, response, transid) msg = {:modbus_tcp, cmd, values} send(state.d_pid, msg) new_pending_msg = Map.delete(state.pending_msg, transid) - new_state = %Client{state | cmd: nil, msg_len: 0, pending_msg: new_pending_msg} - {:noreply, new_state} + %Client{state | cmd: nil, msg_len: 0, pending_msg: new_pending_msg} end end - def handle_info({:tcp_closed, _port}, state) do - Logger.info("(#{__MODULE__}, :tcp_close) Server close the port") - new_state = close_socket(state) - {:noreply, new_state} - end - - def handle_info(msg, state) do - Logger.error("(#{__MODULE__}, :random_msg) msg: #{inspect(msg)}") - {:noreply, state} - end - defp close_socket(state) do :ok = :gen_tcp.close(state.socket) new_state = %Client{state | socket: nil, status: :closed} diff --git a/test/tcp/modbus_tcp_client_test.exs b/test/tcp/modbus_tcp_client_test.exs index fba0ed7..d6f08e4 100644 --- a/test/tcp/modbus_tcp_client_test.exs +++ b/test/tcp/modbus_tcp_client_test.exs @@ -1,5 +1,6 @@ defmodule ModbuxTcpClientTest do use ExUnit.Case + alias Modbux.Tcp alias Modbux.Tcp.{Client, Server} test "test Client (connection, stop, configuration)" do @@ -15,8 +16,33 @@ defmodule ModbuxTcpClientTest do end test "test Client errors" do - {:ok, cpid} = Client.start_link(ip: {127, 0, 0, 1}, tcp_port: 5000) + {:ok, cpid} = Client.start_link(ip: {127, 0, 0, 1}, tcp_port: 50123) assert {:error, :econnrefused} == Client.connect(cpid) assert {:error, :closed} == Client.close(cpid) end + + # https://github.com/valiot/modbux/issues/14 + test "client handles two responses in a single tcp packet" do + {:ok, listen_socket} = :gen_tcp.listen(0, [:binary, packet: :raw, active: false]) + {:ok, port} = :inet.port(listen_socket) + + {:ok, cpid} = Client.start_link(ip: {127, 0, 0, 1}, tcp_port: port, active: true) + assert :ok == Client.connect(cpid) + {:ok, server_socket} = :gen_tcp.accept(listen_socket) + + coils = [1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1] + assert :ok == Client.request(cpid, {:ri, 0, 0, 16}) + assert :ok == Client.request(cpid, {:fc, 0, 16, 1}) + + # Both responses (transid 0 and 1) arrive in the same tcp packet. + response = Tcp.pack_res({:ri, 0, 0, 16}, coils, 0) <> Tcp.pack_res({:fc, 0, 16, 1}, nil, 1) + :ok = :gen_tcp.send(server_socket, response) + + assert_receive {:modbus_tcp, {:ri, 0, 0, 16}, ^coils}, 1000 + assert_receive {:modbus_tcp, {:fc, 0, 16, 1}, nil}, 1000 + + assert :ok == Client.stop(cpid) + :gen_tcp.close(server_socket) + :gen_tcp.close(listen_socket) + end end From 239ade50fdd99e5e8b719bbd4301b777cbc6368b Mon Sep 17 00:00:00 2001 From: alde103 Date: Fri, 3 Jul 2026 18:54:40 -0600 Subject: [PATCH 2/6] Modbus TCP Server: handle multiple requests in a single tcp packet (#14) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01V9dLEMSgBybY4Xe9szthoQ --- lib/tcp/client.ex | 27 +++++++-------------- lib/tcp/server/server_handler.ex | 37 +++++++++++++++++++---------- lib/tcp/tcp.ex | 16 +++++++++++++ test/tcp/modbus_tcp_server_test.exs | 21 ++++++++++++++++ 4 files changed, 70 insertions(+), 31 deletions(-) diff --git a/lib/tcp/client.ex b/lib/tcp/client.ex index e57ce08..3ae6872 100644 --- a/lib/tcp/client.ex +++ b/lib/tcp/client.ex @@ -316,7 +316,14 @@ defmodule Modbux.Tcp.Client do Logger.debug("(#{__MODULE__}, :message_active) response: #{inspect(response)}") Logger.debug("(#{__MODULE__}, :message_active) state: #{inspect(state)}") - new_state = process_responses(response, state) + # A tcp packet may carry several responses. + {responses, rest} = Tcp.split_adus(response) + + if rest != <<>> do + Logger.error("(#{__MODULE__}, :message_active) incomplete response: #{inspect(rest, base: :hex)}") + end + + new_state = Enum.reduce(responses, state, &process_response/2) {:noreply, new_state} end @@ -331,23 +338,7 @@ defmodule Modbux.Tcp.Client do {:noreply, state} end - # A tcp packet may carry several responses, each one framed by the length field of its MBAP header. - defp process_responses(<<>>, state), do: state - - defp process_responses(<>, state) - when byte_size(tail) >= len do - <> = tail - response = <> - new_state = process_response(transid, response, state) - process_responses(rest, new_state) - end - - defp process_responses(response, state) do - Logger.error("(#{__MODULE__}, :message_active) incomplete response: #{inspect(response, base: :hex)}") - state - end - - defp process_response(transid, response, state) do + defp process_response(<> = response, state) do Logger.debug("(#{__MODULE__}, :message_active) transid: #{inspect(transid)}") case Map.fetch(state.pending_msg, transid) do diff --git a/lib/tcp/server/server_handler.ex b/lib/tcp/server/server_handler.ex index 98038d2..7966cb6 100644 --- a/lib/tcp/server/server_handler.ex +++ b/lib/tcp/server/server_handler.ex @@ -23,23 +23,15 @@ defmodule Modbux.Tcp.Server.Handler do def handle_info({:tcp, socket, data}, state) do Logger.debug("(#{__MODULE__}) Received: #{inspect(data, base: :hex)} ") - {cmd, transid} = Tcp.parse_req(data) - Logger.debug("(#{__MODULE__}) Received Modbux request: #{inspect({cmd, transid})}") - case Shared.apply(state.model_pid, cmd) do - {:ok, values} -> - Logger.debug("(#{__MODULE__}) msg send") - resp = Tcp.pack_res(cmd, values, transid) - if !is_nil(state.parent_pid), do: notify(state.parent_pid, cmd) - :ok = :gen_tcp.send(socket, resp) + # A tcp packet may carry several requests. + {requests, rest} = Tcp.split_adus(data) - {:error, reason} -> - Logger.debug("(#{__MODULE__}) An error has occur: #{reason}") - - nil -> - Logger.debug("(#{__MODULE__}) Message for another slave") + if rest != <<>> do + Logger.error("(#{__MODULE__}) incomplete request: #{inspect(rest, base: :hex)}") end + Enum.each(requests, fn request -> attend_request(socket, request, state) end) {:noreply, state} end @@ -67,6 +59,25 @@ defmodule Modbux.Tcp.Server.Handler do :gen_tcp.close(state.socket) end + defp attend_request(socket, request, state) do + {cmd, transid} = Tcp.parse_req(request) + Logger.debug("(#{__MODULE__}) Received Modbux request: #{inspect({cmd, transid})}") + + case Shared.apply(state.model_pid, cmd) do + {:ok, values} -> + Logger.debug("(#{__MODULE__}) msg send") + resp = Tcp.pack_res(cmd, values, transid) + if !is_nil(state.parent_pid), do: notify(state.parent_pid, cmd) + :ok = :gen_tcp.send(socket, resp) + + {:error, reason} -> + Logger.debug("(#{__MODULE__}) An error has occur: #{reason}") + + nil -> + Logger.debug("(#{__MODULE__}) Message for another slave") + end + end + defp notify(pid, cmd) do send(pid, {:modbus_tcp, {:server_request, cmd}}) end diff --git a/lib/tcp/tcp.ex b/lib/tcp/tcp.ex index 47af26d..2ccedb1 100644 --- a/lib/tcp/tcp.ex +++ b/lib/tcp/tcp.ex @@ -45,6 +45,22 @@ defmodule Modbux.Tcp do Request.length(cmd) + 6 end + @doc """ + Splits a tcp stream into complete Modbus TCP messages (ADUs), each one framed by the + length field of its MBAP header. Returns the complete messages and the remaining bytes. + """ + @spec split_adus(binary) :: {[binary], binary} + def split_adus(data), do: split_adus(data, []) + + defp split_adus(<>, adus) + when byte_size(tail) >= len do + <> = tail + adu = <> + split_adus(rest, [adu | adus]) + end + + defp split_adus(rest, adus), do: {Enum.reverse(adus), rest} + @spec wrap(binary, integer) :: <<_::48, _::_*8>> def wrap(payload, transid) do size = :erlang.byte_size(payload) diff --git a/test/tcp/modbus_tcp_server_test.exs b/test/tcp/modbus_tcp_server_test.exs index 09d6a06..fcbb7ff 100644 --- a/test/tcp/modbus_tcp_server_test.exs +++ b/test/tcp/modbus_tcp_server_test.exs @@ -1,5 +1,6 @@ defmodule ModbuxTcpServerTest do use ExUnit.Case + alias Modbux.Tcp alias Modbux.Tcp.Server alias Modbux.Tcp.Client @@ -47,6 +48,26 @@ defmodule ModbuxTcpServerTest do assert_received {:modbus_tcp, {:server_request, {:rc, 80, 20818, 1}}} end + # https://github.com/valiot/modbux/issues/14 + test "server handles two requests in a single tcp packet" do + model = %{0x50 => %{{:c, 20818} => 1, {:hr, 20817} => 7}} + {:ok, _spid} = Server.start_link(model: model, port: 2010) + + {:ok, socket} = :gen_tcp.connect({127, 0, 0, 1}, 2010, [:binary, packet: :raw, active: false]) + + # Both requests (transid 0 and 1) are sent in the same tcp packet. + request = Tcp.pack_req({:rc, 0x50, 20818, 1}, 0) <> Tcp.pack_req({:rhr, 0x50, 20817, 1}, 1) + :ok = :gen_tcp.send(socket, request) + + {:ok, resp1} = :gen_tcp.recv(socket, Tcp.res_len({:rc, 0x50, 20818, 1}), 1000) + assert Tcp.parse_res({:rc, 0x50, 20818, 1}, resp1, 0) == [1] + + {:ok, resp2} = :gen_tcp.recv(socket, Tcp.res_len({:rhr, 0x50, 20817, 1}), 1000) + assert Tcp.parse_res({:rhr, 0x50, 20817, 1}, resp2, 1) == [7] + + :gen_tcp.close(socket) + end + test "DB updates from Elixir" do model = %{0x50 => %{{:c, 0x5152} => 0}} {:ok, s_pid} = Server.start_link(model: model, port: 2002, active: true) From a59b88237ec9c14a65eb34d8798829aa092dafb7 Mon Sep 17 00:00:00 2001 From: alde103 Date: Fri, 3 Jul 2026 18:56:40 -0600 Subject: [PATCH 3/6] Modbus TCP: reassemble requests/responses split across tcp packets (#14) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01V9dLEMSgBybY4Xe9szthoQ --- CHANGELOG.md | 2 ++ lib/tcp/client.ex | 17 +++++++---------- lib/tcp/server/server_handler.ex | 15 ++++++--------- lib/tcp/tcp.ex | 6 +++++- test/tcp/modbus_tcp_client_test.exs | 25 +++++++++++++++++++++++++ test/tcp/modbus_tcp_server_test.exs | 20 ++++++++++++++++++++ 6 files changed, 65 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3df922..78abb79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## 0.3.14 * Modbus TCP Client (active mode): handles multiple responses received in a single tcp packet. +* Modbus TCP Server: handles multiple requests received in a single tcp packet. +* Modbus TCP Client & Server: reassemble requests/responses split across tcp packets. ## 0.3.13 diff --git a/lib/tcp/client.ex b/lib/tcp/client.ex index 3ae6872..1af7199 100644 --- a/lib/tcp/client.ex +++ b/lib/tcp/client.ex @@ -23,7 +23,8 @@ defmodule Modbux.Tcp.Client do d_pid: nil, msg_len: 0, pending_msg: %{}, - cmd: nil + cmd: nil, + buffer: <<>> @type client_option :: {:ip, {byte(), byte(), byte(), byte()}} @@ -316,14 +317,10 @@ defmodule Modbux.Tcp.Client do Logger.debug("(#{__MODULE__}, :message_active) response: #{inspect(response)}") Logger.debug("(#{__MODULE__}, :message_active) state: #{inspect(state)}") - # A tcp packet may carry several responses. - {responses, rest} = Tcp.split_adus(response) - - if rest != <<>> do - Logger.error("(#{__MODULE__}, :message_active) incomplete response: #{inspect(rest, base: :hex)}") - end - - new_state = Enum.reduce(responses, state, &process_response/2) + # A tcp packet may carry several responses or just a fragment of one, + # incomplete bytes are kept in the buffer until the next packet. + {responses, buffer} = Tcp.split_adus(state.buffer <> response) + new_state = Enum.reduce(responses, %Client{state | buffer: buffer}, &process_response/2) {:noreply, new_state} end @@ -357,7 +354,7 @@ defmodule Modbux.Tcp.Client do defp close_socket(state) do :ok = :gen_tcp.close(state.socket) - new_state = %Client{state | socket: nil, status: :closed} + new_state = %Client{state | socket: nil, status: :closed, buffer: <<>>} new_state end end diff --git a/lib/tcp/server/server_handler.ex b/lib/tcp/server/server_handler.ex index 7966cb6..b36214c 100644 --- a/lib/tcp/server/server_handler.ex +++ b/lib/tcp/server/server_handler.ex @@ -10,7 +10,8 @@ defmodule Modbux.Tcp.Server.Handler do defstruct model_pid: nil, parent_pid: nil, - socket: nil + socket: nil, + buffer: <<>> @spec start_link([...]) :: :ignore | {:error, any} | {:ok, pid} def start_link([socket, model_pid, parent_pid]) do @@ -24,15 +25,11 @@ defmodule Modbux.Tcp.Server.Handler do def handle_info({:tcp, socket, data}, state) do Logger.debug("(#{__MODULE__}) Received: #{inspect(data, base: :hex)} ") - # A tcp packet may carry several requests. - {requests, rest} = Tcp.split_adus(data) - - if rest != <<>> do - Logger.error("(#{__MODULE__}) incomplete request: #{inspect(rest, base: :hex)}") - end - + # A tcp packet may carry several requests or just a fragment of one, + # incomplete bytes are kept in the buffer until the next packet. + {requests, buffer} = Tcp.split_adus(state.buffer <> data) Enum.each(requests, fn request -> attend_request(socket, request, state) end) - {:noreply, state} + {:noreply, %Handler{state | buffer: buffer}} end def handle_info({:tcp_closed, _socket}, state) do diff --git a/lib/tcp/tcp.ex b/lib/tcp/tcp.ex index 2ccedb1..e32c278 100644 --- a/lib/tcp/tcp.ex +++ b/lib/tcp/tcp.ex @@ -101,6 +101,10 @@ defmodule Modbux.Tcp do # Protocol identifier -> MODBUS protocol = 0x00, 0x00 defp check_protocol_identifier(0, 0), do: :ok + defp check_protocol_identifier(protocol_id_h, protocol_id_l), - do: Logger.warning("(#{__MODULE__}) Protocol Identifier: #{inspect(<>, base: :hex)}") + do: + Logger.warning( + "(#{__MODULE__}) Protocol Identifier: #{inspect(<>, base: :hex)}" + ) end diff --git a/test/tcp/modbus_tcp_client_test.exs b/test/tcp/modbus_tcp_client_test.exs index d6f08e4..a1a3b7a 100644 --- a/test/tcp/modbus_tcp_client_test.exs +++ b/test/tcp/modbus_tcp_client_test.exs @@ -45,4 +45,29 @@ defmodule ModbuxTcpClientTest do :gen_tcp.close(server_socket) :gen_tcp.close(listen_socket) end + + # https://github.com/valiot/modbux/issues/14 + test "client reassembles a response split across tcp packets" do + {:ok, listen_socket} = :gen_tcp.listen(0, [:binary, packet: :raw, active: false]) + {:ok, port} = :inet.port(listen_socket) + + {:ok, cpid} = Client.start_link(ip: {127, 0, 0, 1}, tcp_port: port, active: true) + assert :ok == Client.connect(cpid) + {:ok, server_socket} = :gen_tcp.accept(listen_socket) + + assert :ok == Client.request(cpid, {:rhr, 0x50, 20817, 2}) + + # The response (transid 0) arrives split in two tcp packets. + response = Tcp.pack_res({:rhr, 0x50, 20817, 2}, [7, 9], 0) + <> = response + :ok = :gen_tcp.send(server_socket, first) + Process.sleep(50) + :ok = :gen_tcp.send(server_socket, second) + + assert_receive {:modbus_tcp, {:rhr, 0x50, 20817, 2}, [7, 9]}, 1000 + + assert :ok == Client.stop(cpid) + :gen_tcp.close(server_socket) + :gen_tcp.close(listen_socket) + end end diff --git a/test/tcp/modbus_tcp_server_test.exs b/test/tcp/modbus_tcp_server_test.exs index fcbb7ff..d1f1b07 100644 --- a/test/tcp/modbus_tcp_server_test.exs +++ b/test/tcp/modbus_tcp_server_test.exs @@ -68,6 +68,26 @@ defmodule ModbuxTcpServerTest do :gen_tcp.close(socket) end + # https://github.com/valiot/modbux/issues/14 + test "server reassembles a request split across tcp packets" do + model = %{0x50 => %{{:hr, 20817} => 7}} + {:ok, _spid} = Server.start_link(model: model, port: 2011) + + {:ok, socket} = :gen_tcp.connect({127, 0, 0, 1}, 2011, [:binary, packet: :raw, active: false]) + + # The request (transid 0) is sent split in two tcp packets. + request = Tcp.pack_req({:rhr, 0x50, 20817, 1}, 0) + <> = request + :ok = :gen_tcp.send(socket, first) + Process.sleep(50) + :ok = :gen_tcp.send(socket, second) + + {:ok, resp} = :gen_tcp.recv(socket, Tcp.res_len({:rhr, 0x50, 20817, 1}), 1000) + assert Tcp.parse_res({:rhr, 0x50, 20817, 1}, resp, 0) == [7] + + :gen_tcp.close(socket) + end + test "DB updates from Elixir" do model = %{0x50 => %{{:c, 0x5152} => 0}} {:ok, s_pid} = Server.start_link(model: model, port: 2002, active: true) From f32561e62c36f0108397c3ca80a96ba7fb68fac0 Mon Sep 17 00:00:00 2001 From: alde103 Date: Fri, 3 Jul 2026 19:49:13 -0600 Subject: [PATCH 4/6] Modbus TCP Client: discard stale pending requests on close && nodelay in fragmentation tests Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01V9dLEMSgBybY4Xe9szthoQ --- ...ttings.local.json.tmp.3761534.353b31b2950e | 18 +++++++++++++++ CHANGELOG.md | 1 + lib/tcp/client.ex | 12 +++++++++- test/tcp/modbus_tcp_client_test.exs | 22 ++++++++++++++++++- test/tcp/modbus_tcp_server_test.exs | 4 +++- 5 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 .claude/settings.local.json.tmp.3761534.353b31b2950e diff --git a/.claude/settings.local.json.tmp.3761534.353b31b2950e b/.claude/settings.local.json.tmp.3761534.353b31b2950e new file mode 100644 index 0000000..a58bda1 --- /dev/null +++ b/.claude/settings.local.json.tmp.3761534.353b31b2950e @@ -0,0 +1,18 @@ +{ + "permissions": { + "allow": [ + "Bash(gh issue *)", + "Bash(mix test *)", + "Bash(git checkout *)", + "Bash(mix format *)", + "Bash(git add *)", + "Bash(git commit -m 'Modbus TCP Client: handle multiple responses in a single tcp packet \\(#14\\) *)", + "Bash(git commit -q -m 'Modbus TCP Server: handle multiple requests in a single tcp packet \\(#14\\) *)", + "Bash(git commit -q -m 'Modbus TCP: reassemble requests/responses split across tcp packets \\(#14\\) *)", + "Bash(git push *)", + "Bash(gh pr *)", + "Bash(gh api *)", + "Bash(git commit -q -m 'Modbus TCP Client: discard stale pending requests on close && nodelay in fragmentation tests *)" + ] + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 78abb79..01948c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Modbus TCP Client (active mode): handles multiple responses received in a single tcp packet. * Modbus TCP Server: handles multiple requests received in a single tcp packet. * Modbus TCP Client & Server: reassemble requests/responses split across tcp packets. +* Modbus TCP Client: pending requests are discarded when the connection is closed. ## 0.3.13 diff --git a/lib/tcp/client.ex b/lib/tcp/client.ex index 1af7199..7d2421c 100644 --- a/lib/tcp/client.ex +++ b/lib/tcp/client.ex @@ -354,7 +354,17 @@ defmodule Modbux.Tcp.Client do defp close_socket(state) do :ok = :gen_tcp.close(state.socket) - new_state = %Client{state | socket: nil, status: :closed, buffer: <<>>} + + new_state = %Client{ + state + | socket: nil, + status: :closed, + buffer: <<>>, + pending_msg: %{}, + cmd: nil, + msg_len: 0 + } + new_state end end diff --git a/test/tcp/modbus_tcp_client_test.exs b/test/tcp/modbus_tcp_client_test.exs index a1a3b7a..be95c3e 100644 --- a/test/tcp/modbus_tcp_client_test.exs +++ b/test/tcp/modbus_tcp_client_test.exs @@ -46,9 +46,29 @@ defmodule ModbuxTcpClientTest do :gen_tcp.close(listen_socket) end + test "client discards pending requests when the socket is closed" do + {:ok, listen_socket} = :gen_tcp.listen(0, [:binary, packet: :raw, active: false]) + {:ok, port} = :inet.port(listen_socket) + + {:ok, cpid} = Client.start_link(ip: {127, 0, 0, 1}, tcp_port: port, active: true) + assert :ok == Client.connect(cpid) + {:ok, server_socket} = :gen_tcp.accept(listen_socket) + + assert :ok == Client.request(cpid, {:rhr, 0x50, 20817, 1}) + assert :ok == Client.close(cpid) + + # Requests from a closed connection can never be answered. + assert Client.state(cpid).pending_msg == %{} + + assert :ok == Client.stop(cpid) + :gen_tcp.close(server_socket) + :gen_tcp.close(listen_socket) + end + # https://github.com/valiot/modbux/issues/14 test "client reassembles a response split across tcp packets" do - {:ok, listen_socket} = :gen_tcp.listen(0, [:binary, packet: :raw, active: false]) + # nodelay avoids Nagle coalescing the two fragments into a single tcp packet. + {:ok, listen_socket} = :gen_tcp.listen(0, [:binary, packet: :raw, active: false, nodelay: true]) {:ok, port} = :inet.port(listen_socket) {:ok, cpid} = Client.start_link(ip: {127, 0, 0, 1}, tcp_port: port, active: true) diff --git a/test/tcp/modbus_tcp_server_test.exs b/test/tcp/modbus_tcp_server_test.exs index d1f1b07..a8b7c66 100644 --- a/test/tcp/modbus_tcp_server_test.exs +++ b/test/tcp/modbus_tcp_server_test.exs @@ -73,7 +73,9 @@ defmodule ModbuxTcpServerTest do model = %{0x50 => %{{:hr, 20817} => 7}} {:ok, _spid} = Server.start_link(model: model, port: 2011) - {:ok, socket} = :gen_tcp.connect({127, 0, 0, 1}, 2011, [:binary, packet: :raw, active: false]) + # nodelay avoids Nagle coalescing the two fragments into a single tcp packet. + {:ok, socket} = + :gen_tcp.connect({127, 0, 0, 1}, 2011, [:binary, packet: :raw, active: false, nodelay: true]) # The request (transid 0) is sent split in two tcp packets. request = Tcp.pack_req({:rhr, 0x50, 20817, 1}, 0) From 618345c2f3beb8162c327fd84f95ed112e40f3ab Mon Sep 17 00:00:00 2001 From: alde103 Date: Sat, 4 Jul 2026 07:55:25 -0600 Subject: [PATCH 5/6] Remove accidentally committed Claude Code temp file && ignore .claude/ Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01V9dLEMSgBybY4Xe9szthoQ --- ...ettings.local.json.tmp.3761534.353b31b2950e | 18 ------------------ .gitignore | 2 +- 2 files changed, 1 insertion(+), 19 deletions(-) delete mode 100644 .claude/settings.local.json.tmp.3761534.353b31b2950e diff --git a/.claude/settings.local.json.tmp.3761534.353b31b2950e b/.claude/settings.local.json.tmp.3761534.353b31b2950e deleted file mode 100644 index a58bda1..0000000 --- a/.claude/settings.local.json.tmp.3761534.353b31b2950e +++ /dev/null @@ -1,18 +0,0 @@ -{ - "permissions": { - "allow": [ - "Bash(gh issue *)", - "Bash(mix test *)", - "Bash(git checkout *)", - "Bash(mix format *)", - "Bash(git add *)", - "Bash(git commit -m 'Modbus TCP Client: handle multiple responses in a single tcp packet \\(#14\\) *)", - "Bash(git commit -q -m 'Modbus TCP Server: handle multiple requests in a single tcp packet \\(#14\\) *)", - "Bash(git commit -q -m 'Modbus TCP: reassemble requests/responses split across tcp packets \\(#14\\) *)", - "Bash(git push *)", - "Bash(gh pr *)", - "Bash(gh api *)", - "Bash(git commit -q -m 'Modbus TCP Client: discard stale pending requests on close && nodelay in fragmentation tests *)" - ] - } -} diff --git a/.gitignore b/.gitignore index a1764b8..8daed11 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,4 @@ /priv erl_crash.dump *.ez -.elixir_ls \ No newline at end of file +.elixir_ls.claude/ From 83c81abeb7e98bcd4fd56e8abc4445dcf724eaf8 Mon Sep 17 00:00:00 2001 From: alde103 Date: Sun, 5 Jul 2026 10:24:11 -0600 Subject: [PATCH 6/6] Fix .gitignore concatenation && log message typos Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01V9dLEMSgBybY4Xe9szthoQ --- .gitignore | 3 ++- lib/tcp/client.ex | 2 +- lib/tcp/server/server_handler.ex | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 8daed11..d744e63 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,5 @@ /priv erl_crash.dump *.ez -.elixir_ls.claude/ +.elixir_ls +.claude/ diff --git a/lib/tcp/client.ex b/lib/tcp/client.ex index 7d2421c..6746825 100644 --- a/lib/tcp/client.ex +++ b/lib/tcp/client.ex @@ -325,7 +325,7 @@ defmodule Modbux.Tcp.Client do end def handle_info({:tcp_closed, _port}, state) do - Logger.info("(#{__MODULE__}, :tcp_close) Server close the port") + Logger.info("(#{__MODULE__}, :tcp_close) Server closed the port") new_state = close_socket(state) {:noreply, new_state} end diff --git a/lib/tcp/server/server_handler.ex b/lib/tcp/server/server_handler.ex index b36214c..a2e4783 100644 --- a/lib/tcp/server/server_handler.ex +++ b/lib/tcp/server/server_handler.ex @@ -68,7 +68,7 @@ defmodule Modbux.Tcp.Server.Handler do :ok = :gen_tcp.send(socket, resp) {:error, reason} -> - Logger.debug("(#{__MODULE__}) An error has occur: #{reason}") + Logger.debug("(#{__MODULE__}) An error has occurred: #{reason}") nil -> Logger.debug("(#{__MODULE__}) Message for another slave")