From: Anderson A. Giacomolli <redacted>
Date: Tue, 10 Nov 2026 23:16:03 +0000 (-0300)
Subject: Preserve separate Modbus phase timeouts
X-Git-Url: https://git.giacomolli.uk/onehouse/commitdiff_plain/9ca7956528b2d1720cc0e63ca168fd8518609a28?ds=inline;hp=c87607884883cca4e47736db79a11c03f175138f

Preserve separate Modbus phase timeouts

Connect and write operations each have a one-second limit, and the
response gets two seconds after the write completes. Caller deadlines
remain earlier bounds. Deadline changes are serialized with context
interruption so a read cannot replace the cancellation deadline. A
simulated delayed write verifies the complete response window.

Issue: 8fdc39a
---

diff --git a/doc/m31.rst b/doc/m31.rst
index 64c913c..b0305e3 100644
--- a/doc/m31.rst
+++ b/doc/m31.rst
@@ -33,10 +33,10 @@ load. The conservative sixteen-module limit includes the host.
 ***************
 
 ``NewTCPClient`` supplies a caller-owned, serialized connection through
-``Client``. Connect selects the configured unit and has a two-second
-timeout. A request has a one-second write deadline and a two-second
-response deadline, both measured from its start. Caller deadlines and
-cancellation can stop it earlier. Each response must match the
+``Client``. Connect selects the configured unit and has a one-second
+timeout. Each write has a one-second deadline. Each response has a
+two-second deadline measured after the request write. Caller deadlines
+and cancellation can stop it earlier. Each response must match the
 transaction, unit, function, length, and write acknowledgement.
 Supported functions are 01, 02, 03, 05, 15, and 16. No external Modbus
 package is required.
diff --git a/driver/m31/tcp_client.go b/driver/m31/tcp_client.go
index 972f698..7ee6bbe 100644
--- a/driver/m31/tcp_client.go
+++ b/driver/m31/tcp_client.go
@@ -9,11 +9,12 @@ import (
 	"io"
 	"net"
 	"os"
+	"sync"
 	"time"
 )
 
 const (
-	connectTimeout = 2 * time.Second
+	connectTimeout = time.Second
 	writeTimeout   = time.Second
 	readTimeout    = 2 * time.Second
 )
@@ -25,7 +26,7 @@ type tcpClient struct {
 }
 
 // NewTCPClient supplies Modbus TCP functions 01, 02, 03, 05, 15, and 16 without
-// opening a connection. Connect has a two-second timeout. Writes have a
+// opening a connection. Connect has a one-second timeout. Writes have a
 // one-second timeout, and reads have a two-second timeout. Earlier caller
 // deadlines apply to each operation. The caller owns Close.
 func NewTCPClient() Client {
@@ -187,16 +188,17 @@ func (c *tcpClient) exchange(ctx context.Context, function byte, data []byte) (r
 		return nil, fmt.Errorf("set Modbus write deadline: %w", err)
 	}
 
-	err = c.conn.SetReadDeadline(operationDeadline(ctx, readTimeout))
-	if err != nil {
-		return nil, fmt.Errorf("set Modbus read deadline: %w", err)
-	}
-
 	interrupted := make(chan struct{})
 
-	var interruptErr error
+	var (
+		interruptErr error
+		deadlineMu   sync.Mutex
+	)
 
 	stop := context.AfterFunc(ctx, func() {
+		deadlineMu.Lock()
+		defer deadlineMu.Unlock()
+
 		interruptErr = c.conn.SetDeadline(time.Now())
 
 		close(interrupted)
@@ -222,6 +224,21 @@ func (c *tcpClient) exchange(ctx context.Context, function byte, data []byte) (r
 		return nil, fmt.Errorf("write Modbus request: %w", io.ErrShortWrite)
 	}
 
+	deadlineMu.Lock()
+
+	if ctx.Err() != nil {
+		deadlineMu.Unlock()
+
+		return nil, ctx.Err()
+	}
+
+	err = c.conn.SetReadDeadline(operationDeadline(ctx, readTimeout))
+	deadlineMu.Unlock()
+
+	if err != nil {
+		return nil, fmt.Errorf("set Modbus read deadline: %w", err)
+	}
+
 	var header [7]byte
 
 	_, err = io.ReadFull(c.conn, header[:])
diff --git a/driver/m31/tcp_client_test.go b/driver/m31/tcp_client_test.go
index 6e4979b..0067566 100644
--- a/driver/m31/tcp_client_test.go
+++ b/driver/m31/tcp_client_test.go
@@ -311,6 +311,59 @@ func TestTCPClientUsesCallerDeadlineAndStartsClosed(t *testing.T) {
 	})
 }
 
+func TestTCPClientReadTimeoutStartsAfterRequestWrite(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		conn, peer := net.Pipe()
+		c := &tcpClient{conn: conn, unit: 1}
+		t.Cleanup(func() {
+			require.NoError(t, c.Close())
+			require.NoError(t, peer.Close())
+		})
+
+		done := make(chan error, 1)
+		go func() {
+			time.Sleep(500 * time.Millisecond)
+
+			var request [12]byte
+
+			_, err := io.ReadFull(peer, request[:])
+			done <- err
+		}()
+
+		started := time.Now()
+		_, err := c.ReadCoils(t.Context(), 1)
+		require.Error(t, err)
+		require.NoError(t, <-done)
+		require.Equal(t, 2500*time.Millisecond, time.Since(started))
+	})
+}
+
+func TestTCPClientStopsAfterCancelledRequestWrite(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		conn, peer := net.Pipe()
+		t.Cleanup(func() {
+			require.NoError(t, peer.Close())
+		})
+
+		ctx, cancel := context.WithCancel(t.Context())
+		defer cancel()
+
+		transport := &interruptConn{
+			Conn:        conn,
+			cancel:      cancel,
+			interrupted: make(chan struct{}),
+		}
+		c := &tcpClient{conn: transport, unit: 1}
+		t.Cleanup(func() {
+			require.NoError(t, c.Close())
+		})
+
+		_, err := c.ReadCoils(ctx, 1)
+		require.ErrorIs(t, err, context.Canceled)
+		require.NotErrorIs(t, err, os.ErrDeadlineExceeded)
+	})
+}
+
 func TestTCPClientCancellationPreservesIndependentFailure(t *testing.T) {
 	synctest.Test(t, func(t *testing.T) {
 		conn, peer := net.Pipe()
@@ -363,11 +416,15 @@ type interruptConn struct {
 	interrupted chan struct{}
 }
 
-func (c *interruptConn) Write([]byte) (int, error) {
+func (c *interruptConn) Write(data []byte) (int, error) {
 	c.cancel()
 	<-c.interrupted
 
-	return 0, c.failure
+	if c.failure != nil {
+		return 0, c.failure
+	}
+
+	return len(data), nil
 }
 
 func (c *interruptConn) SetDeadline(deadline time.Time) error {