fix RTU crc incorrect byte order

README.md

@@ -14,7 +14,7 @@ For questions use Github [Discussions](https://github.com/aldas/go-modbus-client
 ## Installation
 
 ```bash
-go install github.com/aldas/go-modbus-client
+go get github.com/aldas/go-modbus-client
 ```
 
 ## Supported functions

builder_test.go

@@ -93,7 +93,7 @@ func TestBuilder_ReadHoldingRegistersRTU(t *testing.T) {
  assert.NotNil(t, resp)
 
  received := <-receivedChan
- assert.Equal(t, []byte{0x0, 0x3, 0x0, 0x12, 0x0, 0x4, 0xdd, 0xe5}, received)
+ assert.Equal(t, []byte{0x0, 0x3, 0x0, 0x12, 0x0, 0x4, 0xe5, 0xdd}, received)
 }
 
 func TestBuilder_ReadInputRegistersTCP(t *testing.T) {
@@ -180,7 +180,7 @@ func TestBuilder_ReadInputRegistersRTU(t *testing.T) {
  assert.NotNil(t, resp)
 
  received := <-receivedChan
- assert.Equal(t, []byte{0x0, 0x4, 0x0, 0x12, 0x0, 0x4, 0x1d, 0x50}, received)
+ assert.Equal(t, []byte{0x0, 0x4, 0x0, 0x12, 0x0, 0x4, 0x50, 0x1d}, received)
 }
 
 func TestField_ModbusAddress(t *testing.T) {

client.go

@@ -24,10 +24,11 @@ const (
  // RS232 / RS485 ADU = 253 bytes + Server address (1 byte) + CRC (2 bytes) = 256 bytes.
  // TCP MODBUS ADU = 253 bytes + MBAP (7 bytes) = 260 bytes.
  tcpPacketMaxLen = 7 + 253 // 2 trans id + 2 proto + 2 pdu len + 1 unit id + 253 max data len
+ rtuPacketMaxLen = 256 // 1 unit id + 253 max data len + 2 crc
 
- defaultWriteTimeout = 2 * time.Second
- defaultReadTimeout = 4 * time.Second
- defaultConnectTimeout = 5 * time.Second
+ defaultWriteTimeout = 1 * time.Second
+ defaultReadTimeout = 2 * time.Second
+ defaultConnectTimeout = 1 * time.Second
 )
 
 // ErrPacketTooLong is error indicating that modbus server sent amount of data that is bigger than any modbus packet could be
@@ -46,13 +47,13 @@ type Client struct {
 
  mu sync.RWMutex
  address string
- conn net.Conn // FIXME: maybe use `io.ReadWriteCloser` so we can use serial connection also here
- logger ClientLogger
+ conn net.Conn
+ hooks ClientHooks
 }
 
-// ClientLogger allows to log bytes send/received by client.
+// ClientHooks allows to log bytes send/received by client.
 // NB: Do not modify given slice - it is not a copy.
-type ClientLogger interface {
+type ClientHooks interface {
  BeforeWrite(toWrite []byte)
  AfterEachRead(received []byte, n int, err error)
  BeforeParse(received []byte)
@@ -139,10 +140,10 @@ func WithTimeouts(writeTimeout time.Duration, readTimeout time.Duration) func(c
  }
 }
 
-// WithLogger is option to set logger in client
-func WithLogger(logger ClientLogger) func(c *Client) {
+// WithHooks is option to set hooks in client
+func WithHooks(logger ClientHooks) func(c *Client) {
  return func(c *Client) {
- c.logger = logger
+ c.hooks = logger
  }
 }
 
@@ -213,8 +214,8 @@ func (c *Client) Do(ctx context.Context, req packet.Request) (packet.Response, e
  if err != nil {
  return nil, err
  }
- if c.logger != nil {
- c.logger.BeforeParse(resp)
+ if c.hooks != nil {
+ c.hooks.BeforeParse(resp)
  }
  return c.parseResponseFunc(resp)
 }
@@ -223,15 +224,16 @@ func (c *Client) do(ctx context.Context, data []byte, expectedLen int) ([]byte,
  if err := c.conn.SetWriteDeadline(c.timeNow().Add(c.writeTimeout)); err != nil {
  return nil, err
  }
- if c.logger != nil {
- c.logger.BeforeWrite(data)
+ if c.hooks != nil {
+ c.hooks.BeforeWrite(data)
  }
  if _, err := c.conn.Write(data); err != nil {
  return nil, err
  }
 
- received := [tcpPacketMaxLen]byte{}
- tmp := [tcpPacketMaxLen]byte{}
+ // make buffer a little bit bigger than would be valid to see problems when somehow more bytes are sent
+ const maxBytes = tcpPacketMaxLen + 10
+ received := [maxBytes]byte{}
  total := 0
  for {
  select {
@@ -241,9 +243,9 @@ func (c *Client) do(ctx context.Context, data []byte, expectedLen int) ([]byte,
  }
 
  _ = c.conn.SetReadDeadline(c.timeNow().Add(500 * time.Microsecond)) // max 0.5ms block time for read per iteration
- n, err := c.conn.Read(tmp[:tcpPacketMaxLen])
- if c.logger != nil {
- c.logger.AfterEachRead(tmp[:n], n, err)
+ n, err := c.conn.Read(received[total:maxBytes])
+ if c.hooks != nil {
+ c.hooks.AfterEachRead(received[total:total+n], n, err)
  }
  // on read errors we do not return immediately as for:
  // os.ErrDeadlineExceeded - we set new deadline on next iteration
@@ -254,19 +256,14 @@ func (c *Client) do(ctx context.Context, data []byte, expectedLen int) ([]byte,
  }
  total += n
  if total > tcpPacketMaxLen {
- // TODO: call flush
  return nil, ErrPacketTooLong
  }
- if n > 0 {
- copy(received[total-n:], tmp[:n])
- }
  // check if we have exactly the error packet. Error packets are shorter than regulars packets
  if errPacket := c.asProtocolErrorFunc(received[0:total]); errPacket != nil {
  // TODO: call flush
  return nil, errPacket
  }
  if total >= expectedLen {
- // TODO: call flush if needed
  break
  }
  if errors.Is(err, io.EOF) {

client_test.go

@@ -125,13 +125,13 @@ func TestWithOptions(t *testing.T) {
  WithProtocolErrorFunc(packet.AsRTUErrorPacket),
  WithParseResponseFunc(packet.ParseRTUResponse),
  WithTimeouts(99*time.Second, 98*time.Second),
- WithLogger(new(mockLogger)),
+ WithHooks(new(mockLogger)),
  )
  assert.NotNil(t, client.asProtocolErrorFunc)
  assert.NotNil(t, client.parseResponseFunc)
  assert.Equal(t, 99*time.Second, client.writeTimeout)
  assert.Equal(t, 98*time.Second, client.readTimeout)
- assert.Equal(t, new(mockLogger), client.logger)
+ assert.Equal(t, new(mockLogger), client.hooks)
 }
 
 func TestClient_Do_receivePacketWith1Read(t *testing.T) {
@@ -156,7 +156,7 @@ func TestClient_Do_receivePacketWith1Read(t *testing.T) {
  logger.On("AfterEachRead", []byte{0x12, 0x34, 0x0, 0x0, 0x0, 0x5, 0x1, 0x1, 0x2, 0x0, 0x1}, 11, nil).Once()
  logger.On("BeforeParse", []byte{0x12, 0x34, 0x0, 0x0, 0x0, 0x5, 0x1, 0x1, 0x2, 0x0, 0x1}).Once()
 
- client := NewTCPClient(WithLogger(logger))
+ client := NewTCPClient(WithHooks(logger))
  client.conn = conn
  client.timeNow = func() time.Time {
  return exampleNow
@@ -200,7 +200,7 @@ func TestClientRTU_Do_receivePacketWith1Read(t *testing.T) {
  Return(7, nil).
  Run(func(args mock.Arguments) {
  b := args.Get(0).([]byte)
- copy(b, []byte{0x10, 0x1, 0x2, 0x1, 0x2, 0xae, 0xc5})
+ copy(b, []byte{0x10, 0x1, 0x2, 0x1, 0x2, 0xc5, 0xae})
  }).Once()
 
  client := NewRTUClient()

packet/error.go

@@ -124,7 +124,9 @@ func (re ErrorResponseRTU) Bytes() []byte {
  result[0] = re.UnitID
  result[1] = re.Function + functionCodeErrorBitmask
  result[2] = re.Code
- binary.BigEndian.PutUint16(result[3:5], CRC16(result[0:3]))
+ crc := CRC16(result[0:3])
+ result[3] = uint8(crc)
+ result[4] = uint8(crc >> 8)
 
  return result
 }

packet/error_test.go

@@ -158,12 +158,12 @@ func TestErrorResponseRTU_Bytes(t *testing.T) {
  {
  name: "ok",
  given: ErrorResponseRTU{UnitID: 1, Function: 2, Code: 3},
- expect: []byte{0x1, 0x82, 0x3, 0xa1, 0x0},
+ expect: []byte{0x1, 0x82, 0x3, 0x0, 0xa1},
  },
  {
  name: "ok2",
  given: ErrorResponseRTU{UnitID: 1, Function: 1, Code: 1},
- expect: []byte{0x01, 0x81, 0x01, 0x90, 0x81},
+ expect: []byte{0x01, 0x81, 0x01, 0x81, 0x90},
  },
  }
 

packet/packet_test.go

@@ -16,6 +16,11 @@ func TestCRC16(t *testing.T) {
  when: []byte{0x01, 0x04, 0x02, 0xFF, 0xFF},
  expect: 0x80B8,
  },
+ {
+ name: "ok",
+ when: []byte{0x01, 0x04, 0x02, 0xFF, 0xFF},
+ expect: 0x80B8,
+ },
  {
  name: "ok",
  when: []byte{0x11, 0x03, 0x00, 0x6B, 0x00, 0x03},

packet/readcoilsrequest.go

@@ -1,7 +1,6 @@
 package packet
 
 import (
- "encoding/binary"
  "fmt"
  "math"
  "math/rand"
@@ -98,7 +97,9 @@ func NewReadCoilsRequestRTU(unitID uint8, startAddress uint16, quantity uint16)
 func (r ReadCoilsRequestRTU) Bytes() []byte {
  result := make([]byte, 6+2)
  bytes := r.ReadCoilsRequest.bytes(result)
- binary.BigEndian.PutUint16(result[6:8], CRC16(bytes[:6]))
+ crc := CRC16(bytes[:6])
+ result[6] = uint8(crc)
+ result[7] = uint8(crc >> 8)
  return result
 }
 

packet/readcoilsrequest_test.go

@@ -224,7 +224,7 @@ func TestReadCoilsRequestRTU_Bytes(t *testing.T) {
  {
  name: "ok",
  given: func(r *ReadCoilsRequestRTU) {},
- expect: []byte{0x1, 0x1, 0x0, 0xc8, 0x0, 0xa, 0xf3, 0x3d},
+ expect: []byte{0x1, 0x1, 0x0, 0xc8, 0x0, 0xa, 0x3d, 0xf3},
  },
  {
  name: "ok2",
@@ -233,7 +233,7 @@ func TestReadCoilsRequestRTU_Bytes(t *testing.T) {
  r.StartAddress = 107
  r.Quantity = 3
  },
- expect: []byte{0x10, 0x01, 0x00, 0x6B, 0x00, 0x03, 0x96, 0xe},
+ expect: []byte{0x10, 0x01, 0x00, 0x6B, 0x00, 0x03, 0xe, 0x96},
  },
  }
 

packet/readcoilsresponse.go

@@ -74,10 +74,12 @@ func ParseReadCoilsResponseTCP(data []byte) (*ReadCoilsResponseTCP, error) {
 
 // Bytes returns ReadCoilsResponseRTU packet as bytes form
 func (r ReadCoilsResponseRTU) Bytes() []byte {
- length := r.len()
- result := make([]byte, length+2)
+ length := r.len() + 2
+ result := make([]byte, length)
  bytes := r.ReadCoilsResponse.bytes(result)
- binary.BigEndian.PutUint16(result[length:length+2], CRC16(bytes[:length]))
+ crc := CRC16(bytes[:length-2])
+ result[length-2] = uint8(crc)
+ result[length-1] = uint8(crc >> 8)
  return result
 }
 

packet/readcoilsresponse_test.go

@@ -162,7 +162,7 @@ func TestReadCoilsResponseRTU_Bytes(t *testing.T) {
  {
  name: "ok",
  given: func(r *ReadCoilsResponseRTU) {},
- expect: []byte{0x1, 0x1, 0x2, 0x0, 0x1, 0x3c, 0x78},
+ expect: []byte{0x1, 0x1, 0x2, 0x0, 0x1, 0x78, 0x3c},
  },
  {
  name: "ok2",
@@ -171,7 +171,7 @@ func TestReadCoilsResponseRTU_Bytes(t *testing.T) {
  r.CoilsByteLength = 2
  r.Data = []byte{0x1, 0x2}
  },
- expect: []byte{0x10, 0x1, 0x2, 0x1, 0x2, 0xae, 0xc5},
+ expect: []byte{0x10, 0x1, 0x2, 0x1, 0x2, 0xc5, 0xae},
  },
  }
 

packet/readdiscreteinputsrequest.go

@@ -1,7 +1,6 @@
 package packet
 
 import (
- "encoding/binary"
  "fmt"
  "math"
  "math/rand"
@@ -98,7 +97,9 @@ func NewReadDiscreteInputsRequestRTU(unitID uint8, startAddress uint16, quantity
 func (r ReadDiscreteInputsRequestRTU) Bytes() []byte {
  result := make([]byte, 6+2)
  bytes := r.ReadDiscreteInputsRequest.bytes(result)
- binary.BigEndian.PutUint16(result[6:8], CRC16(bytes[:6]))
+ crc := CRC16(bytes[:6])
+ result[6] = uint8(crc)
+ result[7] = uint8(crc >> 8)
  return result
 }
 

packet/readdiscreteinputsrequest_test.go

@@ -224,7 +224,7 @@ func TestReadDiscreteInputsRequestRTU_Bytes(t *testing.T) {
  {
  name: "ok",
  given: func(r *ReadDiscreteInputsRequestRTU) {},
- expect: []byte{0x1, 0x2, 0x0, 0xc8, 0x0, 0xa, 0xf3, 0x79},
+ expect: []byte{0x1, 0x2, 0x0, 0xc8, 0x0, 0xa, 0x79, 0xf3},
  },
  {
  name: "ok2",
@@ -233,7 +233,7 @@ func TestReadDiscreteInputsRequestRTU_Bytes(t *testing.T) {
  r.StartAddress = 107
  r.Quantity = 3
  },
- expect: []byte{0x10, 0x02, 0x00, 0x6B, 0x00, 0x03, 0x96, 0x4a},
+ expect: []byte{0x10, 0x02, 0x00, 0x6B, 0x00, 0x03, 0x4a, 0x96},
  },
  }
 

packet/readdiscreteinputsresponse.go

@@ -77,7 +77,9 @@ func (r ReadDiscreteInputsResponseRTU) Bytes() []byte {
  length := r.len()
  result := make([]byte, length+2)
  bytes := r.ReadDiscreteInputsResponse.bytes(result)
- binary.BigEndian.PutUint16(result[length:length+2], CRC16(bytes[:length]))
+ crc := CRC16(bytes[:length])
+ result[length] = uint8(crc)
+ result[length+1] = uint8(crc >> 8)
  return result
 }
 

packet/readdiscreteinputsresponse_test.go

@@ -171,7 +171,7 @@ func TestReadDiscreteInputsResponseRTU_Bytes(t *testing.T) {
  r.InputsByteLength = 2
  r.Data = []byte{0x1, 0x2}
  },
- expect: []byte{0x10, 0x2, 0x2, 0x1, 0x2, 0xea, 0xc5},
+ expect: []byte{0x10, 0x2, 0x2, 0x1, 0x2, 0xc5, 0xea},
  },
  }
 

packet/readholdingregistersrequest.go

@@ -1,7 +1,6 @@
 package packet
 
 import (
- "encoding/binary"
  "fmt"
  "math/rand"
 )
@@ -95,7 +94,9 @@ func NewReadHoldingRegistersRequestRTU(unitID uint8, startAddress uint16, quanti
 func (r ReadHoldingRegistersRequestRTU) Bytes() []byte {
  result := make([]byte, 6+2)
  bytes := r.ReadHoldingRegistersRequest.bytes(result)
- binary.BigEndian.PutUint16(result[6:8], CRC16(bytes[:6]))
+ crc := CRC16(bytes[:6])
+ result[6] = uint8(crc)
+ result[7] = uint8(crc >> 8)
  return result
 }
 

packet/readholdingregistersrequest_test.go

@@ -219,7 +219,7 @@ func TestReadHoldingRegistersRequestRTU_Bytes(t *testing.T) {
  {
  name: "ok",
  given: func(r *ReadHoldingRegistersRequestRTU) {},
- expect: []byte{0x1, 0x3, 0x0, 0xc8, 0x0, 0xa, 0x33, 0x44},
+ expect: []byte{0x1, 0x3, 0x0, 0xc8, 0x0, 0xa, 0x44, 0x33},
  },
  {
  name: "ok2",
@@ -228,7 +228,7 @@ func TestReadHoldingRegistersRequestRTU_Bytes(t *testing.T) {
  r.StartAddress = 107
  r.Quantity = 3
  },
- expect: []byte{0x10, 0x03, 0x00, 0x6B, 0x00, 0x03, 0x56, 0x77},
+ expect: []byte{0x10, 0x03, 0x00, 0x6B, 0x00, 0x03, 0x77, 0x56},
  },
  }
 

packet/readholdingregistersresponse.go

@@ -77,7 +77,9 @@ func (r ReadHoldingRegistersResponseRTU) Bytes() []byte {
  length := r.len()
  result := make([]byte, length+2)
  bytes := r.ReadHoldingRegistersResponse.bytes(result)
- binary.BigEndian.PutUint16(result[length:length+2], CRC16(bytes[:length]))
+ crc := CRC16(bytes[:length])
+ result[length] = uint8(crc)
+ result[length+1] = uint8(crc >> 8)
  return result
 }