Read holding registers CRC mismatch

[mitchellpontague]

CRCs seem to be generated wrong on 3.1.3 for 16bit registers. From the below output the crc is being calculated from the bytes cf 03 04 0f 0a 00 00, 0f and 0a of which are from the response proceeding it but are missing the most significant byte for each. The MSB was lost because the response values were stored as the 16bit values into the array value but read back assuming the array contained 8 bit bytes.

https://github.com/Cloud-Automation/node-modbus/blob/ba2a6a1db4975076af67bb30fb4822acf8de3762/src/response/read-holding-registers.js#L103

  modbus-client issuing new read holding registers request +1s
  rtu-client-request-handler registrating new request +1s
  user-request creating new user request with timeout 5000 +1s
  client-request-handler flushing +1s
  client-request-handler flushing new request <Buffer cf 03 00 1f 00 02 e5 e3> +11ms
  @serialport/stream _write 8 bytes of data +1s
  @serialport/binding-abstract write 8 bytes +1s
  @serialport/bindings:unixWrite Starting write 8 bytes offset 0 bytesToWrite 8 +0ms
  @serialport/bindings:unixWrite write returned null 8 +5ms
  @serialport/bindings:unixWrite wrote 8 bytes +0ms
  @serialport/bindings:unixWrite Finished writing 8 bytes +1ms
  @serialport/stream binding.write write finished +13ms
  client-request-handler request fully flushed, ( error: undefined ) undefined +19ms
  @serialport/bindings:poller received "readable" +1s
  @serialport/binding-abstract read +34ms
  @serialport/bindings:unixRead Starting read +1s
  @serialport/bindings:unixRead Finished read 9 bytes +2ms
  @serialport/stream binding.read finished +28ms
  modbus-client received data +64ms
  rtu-response-handler receiving new data +0ms
  rtu-response-handler buffer <Buffer cf 03 04 00 0f 00 0a 65 fb> +1ms
  rtu-response address 207 buffer <Buffer cf 03 04 00 0f 00 0a 65 fb> +0ms
  response-factory fc 3 payload <Buffer 03 04 00 0f 00 0a 65 fb> +0ms
  ReadHoldingRegistersResponseBody ReadHoldingRegistersResponseBody values [ 15, 10 ] +0ms
  rtu-response-handler crc 64357 +16ms
  rtu-response-handler reset buffer from 9 to 0 +1ms
  rtu-response-handler not enough data available to parse +1ms
  rtu-client-request-handler new response coming in +86ms
  rtu-client-request-handler create crc from response <Buffer cf 03 04 0f 0a 00 00> +4ms
  rtu-client-request-handler CRC does not match 64357 !== 59894 +1ms
{ err: 'crcMismatch',
  message: 'the response payload does not match the crc' }
{ address: 31, length: 2, value: -1 }
  @serialport/stream _read reading +38ms
  @serialport/binding-abstract read +44ms
  @serialport/bindings:unixRead Starting read +41ms
  @serialport/bindings:unixRead waiting for readable because of code: EAGAIN +2ms
  @serialport/bindings:poller Polling for "readable" +51ms

[stefanpoeter]

In that case the crc is actually correct but the composition of the read holding register message is wrong when the data is fetched from the array. Do you agree?

[mitchellpontague]

Yeah you are correct, the CRC math is fine but the data is fetched/stored wrong.

Locally I've replaced line 103 linked above with the following payload.writeUInt16BE(value, 2 + 2*i) and confirm it works. But prefer not to make a pull to fix as I'm not sure if this is an issue anywhere else, or if this is the most appropriate fix.

[stefanpoeter]

Can you run npm test and see if anything fails with your change?

[mitchellpontague]

Tests pass, unsure if the error after is related to tests or not.

  139 passing (329ms)

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: connect ECONNREFUSED 127.0.0.1:8888

[stefanpoeter]

@mitchellpontague I've created a pull request, can you confirm that this is working.