From 79d3faec2163f26f201ddf4d35f082d32154d4a6 Mon Sep 17 00:00:00 2001 From: Sven Rebhan Date: Thu, 1 Feb 2024 13:55:14 +0100 Subject: [PATCH 1/4] Introduce custom error for invalid data-sizes --- client.go | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/client.go b/client.go index 6d4c383..2ababdb 100644 --- a/client.go +++ b/client.go @@ -9,6 +9,15 @@ import ( "fmt" ) +type DataSizeError struct { + Expected int + Actual int +} + +func (e *DataSizeError) Error() string { + return fmt.Sprintf("modbus: response data size '%d' does not match count '%d'", e.Actual, e.Expected) +} + // ClientHandler is the interface that groups the Packager and Transporter methods. type ClientHandler interface { Packager @@ -58,7 +67,7 @@ func (mb *client) ReadCoils(address, quantity uint16) (results []byte, err error count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = fmt.Errorf("modbus: response data size '%v' does not match count '%v'", length, count) + err = &DataSizeError{Expected: count, Actual: length} return } results = response.Data[1:] @@ -92,7 +101,7 @@ func (mb *client) ReadDiscreteInputs(address, quantity uint16) (results []byte, count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = fmt.Errorf("modbus: response data size '%v' does not match count '%v'", length, count) + err = &DataSizeError{Expected: count, Actual: length} return } results = response.Data[1:] @@ -126,7 +135,7 @@ func (mb *client) ReadHoldingRegisters(address, quantity uint16) (results []byte count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = fmt.Errorf("modbus: response data size '%v' does not match count '%v'", length, count) + err = &DataSizeError{Expected: count, Actual: length} return } if count != 2*int(quantity) { @@ -164,7 +173,7 @@ func (mb *client) ReadInputRegisters(address, quantity uint16) (results []byte, count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = fmt.Errorf("modbus: response data size '%v' does not match count '%v'", length, count) + err = &DataSizeError{Expected: count, Actual: length} return } if count != 2*int(quantity) { @@ -202,7 +211,7 @@ func (mb *client) WriteSingleCoil(address, value uint16) (results []byte, err er } // Fixed response length if len(response.Data) != 4 { - err = fmt.Errorf("modbus: response data size '%v' does not match expected '%v'", len(response.Data), 4) + err = &DataSizeError{Expected: 4, Actual: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -241,7 +250,7 @@ func (mb *client) WriteSingleRegister(address, value uint16) (results []byte, er } // Fixed response length if len(response.Data) != 4 { - err = fmt.Errorf("modbus: response data size '%v' does not match expected '%v'", len(response.Data), 4) + err = &DataSizeError{Expected: 4, Actual: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -286,7 +295,7 @@ func (mb *client) WriteMultipleCoils(address, quantity uint16, value []byte) (re } // Fixed response length if len(response.Data) != 4 { - err = fmt.Errorf("modbus: response data size '%v' does not match expected '%v'", len(response.Data), 4) + err = &DataSizeError{Expected: 4, Actual: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -331,7 +340,7 @@ func (mb *client) WriteMultipleRegisters(address, quantity uint16, value []byte) } // Fixed response length if len(response.Data) != 4 { - err = fmt.Errorf("modbus: response data size '%v' does not match expected '%v'", len(response.Data), 4) + err = &DataSizeError{Expected: 4, Actual: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -372,7 +381,7 @@ func (mb *client) MaskWriteRegister(address, andMask, orMask uint16) (results [] } // Fixed response length if len(response.Data) != 6 { - err = fmt.Errorf("modbus: response data size '%v' does not match expected '%v'", len(response.Data), 6) + err = &DataSizeError{Expected: 6, Actual: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -428,7 +437,7 @@ func (mb *client) ReadWriteMultipleRegisters(readAddress, readQuantity, writeAdd } count := int(response.Data[0]) if count != (len(response.Data) - 1) { - err = fmt.Errorf("modbus: response data size '%v' does not match count '%v'", len(response.Data)-1, count) + err = &DataSizeError{Expected: count, Actual: len(response.Data) - 1} return } results = response.Data[1:] @@ -462,7 +471,7 @@ func (mb *client) ReadFIFOQueue(address uint16) (results []byte, err error) { } count := int(binary.BigEndian.Uint16(response.Data)) if count != (len(response.Data) - 1) { - err = fmt.Errorf("modbus: response data size '%v' does not match count '%v'", len(response.Data)-1, count) + err = &DataSizeError{Expected: count, Actual: len(response.Data) - 1} return } count = int(binary.BigEndian.Uint16(response.Data[2:])) From 0a13a3da40f543f3ba84a0e2158102aec477cb58 Mon Sep 17 00:00:00 2001 From: Sven Rebhan Date: Thu, 1 Feb 2024 14:03:37 +0100 Subject: [PATCH 2/4] Return data if response is too large, fixes #52 --- client.go | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/client.go b/client.go index 2ababdb..5115b51 100644 --- a/client.go +++ b/client.go @@ -68,9 +68,11 @@ func (mb *client) ReadCoils(address, quantity uint16) (results []byte, err error length := len(response.Data) - 1 if count != length { err = &DataSizeError{Expected: count, Actual: length} - return + if length < count { + return + } } - results = response.Data[1:] + results = response.Data[1 : count+1] return } @@ -102,9 +104,11 @@ func (mb *client) ReadDiscreteInputs(address, quantity uint16) (results []byte, length := len(response.Data) - 1 if count != length { err = &DataSizeError{Expected: count, Actual: length} - return + if length < count { + return + } } - results = response.Data[1:] + results = response.Data[1 : count+1] return } @@ -136,13 +140,15 @@ func (mb *client) ReadHoldingRegisters(address, quantity uint16) (results []byte length := len(response.Data) - 1 if count != length { err = &DataSizeError{Expected: count, Actual: length} - return + if length < count { + return + } } if count != 2*int(quantity) { err = fmt.Errorf("modbus: response data size '%v' does not match request quantity '%v'", length, quantity) return } - results = response.Data[1:] + results = response.Data[1 : count+1] return } @@ -174,13 +180,15 @@ func (mb *client) ReadInputRegisters(address, quantity uint16) (results []byte, length := len(response.Data) - 1 if count != length { err = &DataSizeError{Expected: count, Actual: length} - return + if length < count { + return + } } if count != 2*int(quantity) { err = fmt.Errorf("modbus: response data size '%v' does not match request quantity '%v'", length, quantity) return } - results = response.Data[1:] + results = response.Data[1 : count+1] return } @@ -436,11 +444,14 @@ func (mb *client) ReadWriteMultipleRegisters(readAddress, readQuantity, writeAdd return } count := int(response.Data[0]) - if count != (len(response.Data) - 1) { - err = &DataSizeError{Expected: count, Actual: len(response.Data) - 1} - return + length := len(response.Data) - 1 + if count != length { + err = &DataSizeError{Expected: count, Actual: length} + if length < count { + return + } } - results = response.Data[1:] + results = response.Data[1 : count+1] return } @@ -470,11 +481,14 @@ func (mb *client) ReadFIFOQueue(address uint16) (results []byte, err error) { return } count := int(binary.BigEndian.Uint16(response.Data)) - if count != (len(response.Data) - 1) { - err = &DataSizeError{Expected: count, Actual: len(response.Data) - 1} - return + length := len(response.Data) - 1 + if count != length { + err = &DataSizeError{Expected: count, Actual: length} + if length < count { + return + } } - count = int(binary.BigEndian.Uint16(response.Data[2:])) + count = int(binary.BigEndian.Uint16(response.Data[2 : count+2])) if count > 31 { err = fmt.Errorf("modbus: fifo count '%v' is greater than expected '%v'", count, 31) return From f14741f98e302d289df32306987eafc2d215e789 Mon Sep 17 00:00:00 2001 From: Sven Rebhan Date: Thu, 1 Feb 2024 15:04:08 +0100 Subject: [PATCH 3/4] Comment new error type --- client.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client.go b/client.go index 5115b51..bdb9194 100644 --- a/client.go +++ b/client.go @@ -9,6 +9,8 @@ import ( "fmt" ) +// DataSizeError represents an error for invalid data-sizes i.e. for cases +// where the data-size does not match the expectation. type DataSizeError struct { Expected int Actual int From 828f9ade4663fe9f34e4fceb88cfe70a57d9f36f Mon Sep 17 00:00:00 2001 From: Sven Rebhan Date: Mon, 12 Feb 2024 14:06:10 +0100 Subject: [PATCH 4/4] Make clear expected and actual are in bytes --- client.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/client.go b/client.go index bdb9194..0c8e1b6 100644 --- a/client.go +++ b/client.go @@ -12,12 +12,12 @@ import ( // DataSizeError represents an error for invalid data-sizes i.e. for cases // where the data-size does not match the expectation. type DataSizeError struct { - Expected int - Actual int + ExpectedBytes int + ActualBytes int } func (e *DataSizeError) Error() string { - return fmt.Sprintf("modbus: response data size '%d' does not match count '%d'", e.Actual, e.Expected) + return fmt.Sprintf("modbus: response data size '%d' does not match count '%d'", e.ActualBytes, e.ExpectedBytes) } // ClientHandler is the interface that groups the Packager and Transporter methods. @@ -69,7 +69,7 @@ func (mb *client) ReadCoils(address, quantity uint16) (results []byte, err error count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = &DataSizeError{Expected: count, Actual: length} + err = &DataSizeError{ExpectedBytes: count, ActualBytes: length} if length < count { return } @@ -105,7 +105,7 @@ func (mb *client) ReadDiscreteInputs(address, quantity uint16) (results []byte, count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = &DataSizeError{Expected: count, Actual: length} + err = &DataSizeError{ExpectedBytes: count, ActualBytes: length} if length < count { return } @@ -141,7 +141,7 @@ func (mb *client) ReadHoldingRegisters(address, quantity uint16) (results []byte count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = &DataSizeError{Expected: count, Actual: length} + err = &DataSizeError{ExpectedBytes: count, ActualBytes: length} if length < count { return } @@ -181,7 +181,7 @@ func (mb *client) ReadInputRegisters(address, quantity uint16) (results []byte, count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = &DataSizeError{Expected: count, Actual: length} + err = &DataSizeError{ExpectedBytes: count, ActualBytes: length} if length < count { return } @@ -221,7 +221,7 @@ func (mb *client) WriteSingleCoil(address, value uint16) (results []byte, err er } // Fixed response length if len(response.Data) != 4 { - err = &DataSizeError{Expected: 4, Actual: len(response.Data)} + err = &DataSizeError{ExpectedBytes: 4, ActualBytes: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -260,7 +260,7 @@ func (mb *client) WriteSingleRegister(address, value uint16) (results []byte, er } // Fixed response length if len(response.Data) != 4 { - err = &DataSizeError{Expected: 4, Actual: len(response.Data)} + err = &DataSizeError{ExpectedBytes: 4, ActualBytes: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -305,7 +305,7 @@ func (mb *client) WriteMultipleCoils(address, quantity uint16, value []byte) (re } // Fixed response length if len(response.Data) != 4 { - err = &DataSizeError{Expected: 4, Actual: len(response.Data)} + err = &DataSizeError{ExpectedBytes: 4, ActualBytes: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -350,7 +350,7 @@ func (mb *client) WriteMultipleRegisters(address, quantity uint16, value []byte) } // Fixed response length if len(response.Data) != 4 { - err = &DataSizeError{Expected: 4, Actual: len(response.Data)} + err = &DataSizeError{ExpectedBytes: 4, ActualBytes: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -391,7 +391,7 @@ func (mb *client) MaskWriteRegister(address, andMask, orMask uint16) (results [] } // Fixed response length if len(response.Data) != 6 { - err = &DataSizeError{Expected: 6, Actual: len(response.Data)} + err = &DataSizeError{ExpectedBytes: 6, ActualBytes: len(response.Data)} return } respValue := binary.BigEndian.Uint16(response.Data) @@ -448,7 +448,7 @@ func (mb *client) ReadWriteMultipleRegisters(readAddress, readQuantity, writeAdd count := int(response.Data[0]) length := len(response.Data) - 1 if count != length { - err = &DataSizeError{Expected: count, Actual: length} + err = &DataSizeError{ExpectedBytes: count, ActualBytes: length} if length < count { return } @@ -485,7 +485,7 @@ func (mb *client) ReadFIFOQueue(address uint16) (results []byte, err error) { count := int(binary.BigEndian.Uint16(response.Data)) length := len(response.Data) - 1 if count != length { - err = &DataSizeError{Expected: count, Actual: length} + err = &DataSizeError{ExpectedBytes: count, ActualBytes: length} if length < count { return }