rtu: Fix incorrectly calculating response length for ReadWriteMultipleRegisters

Author: nqv

asciiclient.go

@@ -168,7 +168,7 @@ func (mb *asciiSerialTransporter) Send(aduRequest []byte) (aduResponse []byte, e
 		defer mb.Close()
 	}
 	if mb.Logger != nil {
-		mb.Logger.Printf("modbus: sending %s\n", aduRequest)
+		mb.Logger.Printf("modbus: sending %q\n", aduRequest)
 	}
 	if _, err = mb.port.Write(aduRequest); err != nil {
 		return
@@ -193,7 +193,7 @@ func (mb *asciiSerialTransporter) Send(aduRequest []byte) (aduResponse []byte, e
 	}
 	aduResponse = data[:length]
 	if mb.Logger != nil {
-		mb.Logger.Printf("modbus: received %s\n", aduResponse)
+		mb.Logger.Printf("modbus: received %q\n", aduResponse)
 	}
 	return
 }

client.go

@@ -368,7 +368,7 @@ func (mb *client) MaskWriteRegister(address, andMask, orMask uint16) (results []
 //  Write registers value : N* bytes
 // Response:
 //  Function code         : 1 byte (0x17)
-//  Byte count            : 2 bytes
+//  Byte count            : 1 byte
 //  Read registers value  : Nx2 bytes
 func (mb *client) ReadWriteMultipleRegisters(readAddress, readQuantity, writeAddress, writeQuantity uint16, value []byte) (results []byte, err error) {
 	if readQuantity < 1 || readQuantity > 125 {

rtuclient.go

@@ -168,17 +168,15 @@ func calculateResponseLength(adu []byte) int {
 			length++
 		}
 	case FuncCodeReadInputRegisters,
-		FuncCodeReadHoldingRegisters:
+		FuncCodeReadHoldingRegisters,
+		FuncCodeReadWriteMultipleRegisters:
 		count := int(binary.BigEndian.Uint16(adu[4:]))
 		length += 1 + count*2
 	case FuncCodeWriteSingleCoil,
 		FuncCodeWriteMultipleCoils,
 		FuncCodeWriteSingleRegister,
 		FuncCodeWriteMultipleRegisters:
 		length += 4
-	case FuncCodeReadWriteMultipleRegisters:
-		count := int(binary.BigEndian.Uint16(adu[4:]))
-		length += 2 + count*2
 	case FuncCodeMaskWriteRegister:
 		length += 6
 	case FuncCodeReadFIFOQueue:

test/asciiclient_test.go

@@ -13,22 +13,24 @@ import (
 )
 
 const (
-	testASCIIDevice = "/dev/pts/6"
+	asciiDevice = "/dev/pts/6"
 )
 
-func TestASCIIClientReadCoils(t *testing.T) {
-	client := modbus.ASCIIClient(testASCIIDevice)
-	ClientTestReadCoils(t, client)
+func TestASCIIClient(t *testing.T) {
+	// Diagslave does not support broadcast id.
+	handler := modbus.NewASCIIClientHandler(asciiDevice)
+	handler.SlaveId = 17
+	ClientTestAll(t, modbus.NewClient(handler))
 }
 
 func TestASCIIClientAdvancedUsage(t *testing.T) {
-	handler := modbus.NewASCIIClientHandler(testASCIIDevice)
+	handler := modbus.NewASCIIClientHandler(asciiDevice)
 	handler.BaudRate = 19200
 	handler.DataBits = 8
 	handler.Parity = "E"
 	handler.StopBits = 1
-	handler.SlaveId = 17
-	handler.Logger = log.New(os.Stdout, "test: ", log.LstdFlags)
+	handler.SlaveId = 12
+	handler.Logger = log.New(os.Stdout, "ascii: ", log.LstdFlags)
 	err := handler.Connect()
 	if err != nil {
 		t.Fatal(err)
@@ -40,4 +42,8 @@ func TestASCIIClientAdvancedUsage(t *testing.T) {
 	if err != nil || results == nil {
 		t.Fatal(err, results)
 	}
+	results, err = client.ReadWriteMultipleRegisters(0, 2, 2, 2, []byte{1, 2, 3, 4})
+	if err != nil || results == nil {
+		t.Fatal(err, results)
+	}
 }

test/client.go

@@ -21,7 +21,7 @@ func ClientTestReadCoils(t *testing.T, client modbus.Client) {
 	AssertEquals(t, 3, len(results))
 }
 
-func ClientTestDiscreteInputs(t *testing.T, client modbus.Client) {
+func ClientTestReadDiscreteInputs(t *testing.T, client modbus.Client) {
 	// Read discrete inputs 197-218
 	address := uint16(0x00C4)
 	quantity := uint16(0x0016)
@@ -137,3 +137,17 @@ func ClientTestReadFIFOQueue(t *testing.T, client modbus.Client) {
 		AssertEquals(t, 0, len(results))
 	}
 }
+
+func ClientTestAll(t *testing.T, client modbus.Client) {
+	ClientTestReadCoils(t, client)
+	ClientTestReadDiscreteInputs(t, client)
+	ClientTestReadHoldingRegisters(t, client)
+	ClientTestReadInputRegisters(t, client)
+	ClientTestWriteSingleCoil(t, client)
+	ClientTestWriteSingleRegister(t, client)
+	ClientTestWriteMultipleCoils(t, client)
+	ClientTestWriteMultipleRegisters(t, client)
+	ClientTestMaskWriteRegisters(t, client)
+	ClientTestReadWriteMultipleRegisters(t, client)
+	ClientTestReadFIFOQueue(t, client)
+}

test/rtuclient_test.go

@@ -13,22 +13,24 @@ import (
 )
 
 const (
-	testRTUDevice = "/dev/pts/6"
+	rtuDevice = "/dev/pts/6"
 )
 
-func TestRTUClientReadCoils(t *testing.T) {
-	client := modbus.RTUClient(testRTUDevice)
-	ClientTestReadCoils(t, client)
+func TestRTUClient(t *testing.T) {
+	// Diagslave does not support broadcast id.
+	handler := modbus.NewRTUClientHandler(rtuDevice)
+	handler.SlaveId = 17
+	ClientTestAll(t, modbus.NewClient(handler))
 }
 
 func TestRTUClientAdvancedUsage(t *testing.T) {
-	handler := modbus.NewRTUClientHandler(testRTUDevice)
+	handler := modbus.NewRTUClientHandler(rtuDevice)
 	handler.BaudRate = 19200
 	handler.DataBits = 8
 	handler.Parity = "E"
 	handler.StopBits = 1
-	handler.SlaveId = 17
-	handler.Logger = log.New(os.Stdout, "test: ", log.LstdFlags)
+	handler.SlaveId = 11
+	handler.Logger = log.New(os.Stdout, "rtu: ", log.LstdFlags)
 	err := handler.Connect()
 	if err != nil {
 		t.Fatal(err)
@@ -40,4 +42,8 @@ func TestRTUClientAdvancedUsage(t *testing.T) {
 	if err != nil || results == nil {
 		t.Fatal(err, results)
 	}
+	results, err = client.ReadWriteMultipleRegisters(0, 2, 2, 2, []byte{1, 2, 3, 4})
+	if err != nil || results == nil {
+		t.Fatal(err, results)
+	}
 }

test/tcpclient_test.go

@@ -14,73 +14,19 @@ import (
 )
 
 const (
-	testTCPServer = "localhost:5020"
+	tcpDevice = "localhost:5020"
 )
 
-func TestTCPClientReadCoils(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestReadCoils(t, client)
-}
-
-func TestTCPClientReadDiscreteInputs(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	// Read discrete inputs 197-218
-	ClientTestDiscreteInputs(t, client)
-}
-
-func TestTCPClientReadHoldingRegisters(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestReadHoldingRegisters(t, client)
-}
-
-func TestTCPClientReadInputRegisters(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestReadInputRegisters(t, client)
-}
-
-func TestTCPClientWriteSingleCoil(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestWriteSingleCoil(t, client)
-}
-
-func TestTCPClientWriteSingleRegister(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestWriteSingleRegister(t, client)
-}
-
-func TestTCPClientWriteMultipleCoils(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestWriteMultipleCoils(t, client)
-}
-
-func TestTCPClientWriteMultipleRegisters(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestWriteMultipleRegisters(t, client)
-}
-
-func TestTCPClientMaskWriteRegisters(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestMaskWriteRegisters(t, client)
-}
-
-func TestTCPClientReadWriteMultipleRegisters(t *testing.T) {
-	client := modbus.TCPClient(testTCPServer)
-	ClientTestReadWriteMultipleRegisters(t, client)
-}
-
-func TestTCPClientReadFIFOQueue(t *testing.T) {
-	handler := modbus.NewTCPClientHandler(testTCPServer)
-	handler.Logger = log.New(os.Stdout, "test: ", log.LstdFlags)
-
-	client := modbus.NewClient(handler)
-	ClientTestReadFIFOQueue(t, client)
+func TestTCPClient(t *testing.T) {
+	client := modbus.TCPClient(tcpDevice)
+	ClientTestAll(t, client)
 }
 
 func TestTCPClientAdvancedUsage(t *testing.T) {
-	handler := modbus.NewTCPClientHandler(testTCPServer)
+	handler := modbus.NewTCPClientHandler(tcpDevice)
 	handler.Timeout = 5 * time.Second
-	handler.SlaveId = 0x01
-	handler.Logger = log.New(os.Stdout, "test: ", log.LstdFlags)
+	handler.SlaveId = 1
+	handler.Logger = log.New(os.Stdout, "tcp: ", log.LstdFlags)
 	handler.Connect()
 	defer handler.Close()