Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
472 stars 178 forks source link

3 RTU test failing (locally)? #199

Closed martijnthe closed 6 years ago

martijnthe commented 6 years ago

Hi Stefan,

I just checked out your repo and to try it out. First thing I tried is running the unit tests. Somehow 3 of them are failing (but it looks like they are not failing in CI!?). See below for the logs. I'll dig into it to try to figure out why.

I'm using Node 10.3.0.

  Modbus/RTU Client Response Tests
    1) should handle a valid read coils response
    2) should handle a exception
    3) should handle a chopped response

...

  3 failing

  1) Modbus/RTU Client Response Tests should handle a valid read coils response:
     TypeError: Cannot read property 'address' of undefined
      at Context.<anonymous> (test/rtu-client-response-handler.test.js:31:30)

  2) Modbus/RTU Client Response Tests should handle a exception:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(response !== undefined)

      + expected - actual

      -false
      +true

      at Context.<anonymous> (test/rtu-client-response-handler.test.js:51:12)

  3) Modbus/RTU Client Response Tests should handle a chopped response:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(response !== undefined)

      + expected - actual

      -false
      +true

      at Context.<anonymous> (test/rtu-client-response-handler.test.js:79:12)
martijnthe commented 6 years ago

The first test is failing because of this line: https://github.com/Cloud-Automation/node-modbus/blob/c27c8f10270cba5d134f80d8f67ba801c01b99c7/src/rtu-response.js#L22

The bodyCount property does not exist, so 1 + body.bodyCount end up being NaN.

I searched for bodyCount in the repo, but this line is the only instance of the word I could find.

Any ideas?

stefanpoeter commented 6 years ago

What version have you checked out? v3.0.0 is fine, there is a v3.0.1 that contains some untested code for modbus rtu.

martijnthe commented 6 years ago

@stefanpoeter I tried master as well as v3.0.1_dev. I've created a fix, PR coming in a minute.

stefanpoeter commented 6 years ago

My tests are all fine. Tell me your node.js and npm version as well as your Operating System.

stefanpoeter commented 6 years ago
  Buffer and status conversions.
    ✓ should convert 10 coils status to buffer 2 bytes
    ✓ should convert a buffer with hex to coils/discrete array status
    ✓ should convert a buffer with bin to coils/discrete array status
    ✓ should return empty array if buffer is not instance of Buffer
    ✓ should return empty buffer if coils is not instance of Array

  Buffer manipulation tests
    ✓ should return a short buffer
    ✓ should return a longer buffer
    ✓ should return a correct only byte
    ✓ should return a correct buffer
    ✓ should return another correct buffer

  ReadCoils Tests.
    ReadCoils Response
      ✓ should create a buffer from a read coils message
      ✓ should create a message object from a buffer
      ✓ should mask out extra bits
      ✓ should return an individual coil if requested
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code
      ✓ should return <55> when addres = 0 and count = 8 for coils <55 55 55>
      ✓ should return <55> when addres = 6 and count = 8 for coils <55 55 55>
      ✓ should return <55 01> when addres = 0 and count = 9 for coils <55 55 55>
      ✓ should return <2A> when addres = 1 and count = 7 for coils <55 55 55>
      ✓ should return <05> when addres = 0 and count = 4 for coils <55 55 55>
      ✓ should return <02> when addres = 1 and count = 3 for coils <55 55 55>
    ReadCoils Requests
      ✓ should create a buffer from a read coils message
      ✓ should create a message object from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  ReadDiscreteInputs Tests.
    ReadDiscreteInputs Response
      ✓ should create a buffer from a read discrete inputs message
      ✓ should create a message object from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code
    ReadDiscreteInputs Requests
      ✓ should create a buffer from a discrete inputs message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  ReadHoldingRegisters Tests.
    ReadHoldingRegisters Request
      ✓ should create a buffer from a read holding registers message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  ReadInputRegisters Tests.
    ReadInputRegisters Request
      ✓ should create a buffer from a read input registers message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  Modbus Response Tests.
    Read Coils Tests.
      ✓ should create request from buffer
      ✓ should handle invalid buffer content

  Modbus/RTU Client Request Tests
    Register Test.
      ✓ should register an rtu request
    Handle Data Tests.
      ✓ should register an rtu request and handle a response
      ✓ should register an rtu request and handle a exception response

  Modbus/RTU Client Response Tests
    ✓ should handle a valid read coils response
    ✓ should handle a exception
    ✓ should handle a chopped response

  TCP Client Tests.
    Read Coils Tests.
      ✓ should create request from buffer
      ✓ should handle invalid buffer content
      ✓ should handle a invalid request (invalid quantity)
      ✓ should handle a valid request with a exception response
      ✓ should handle a valid request with timeout (101ms)
      ✓ should handle a valid request while offline
      ✓ should handle two valid request while offline
      ✓ should handle two valid requests
      ✓ should handle a valid request with an out of sync response
      ✓ should handle two valid request with an out of sync response
      ✓ should handle a valid request with an out of sync response (wrong fc)
      ✓ should handle a valid request with a wrong protocol response
    Read Discrete Inputs Tests.
      ✓ should handle a valid request
      ✓ should handle a invalid request (invalid start address)
      ✓ should handle a invalid request (invalid quantity)
    Read Holding Registers Tests.
      ✓ should handle a valid request
      ✓ should handle a invalid request (invalid start address)
      ✓ should handle a invalid request (invalid quantity)
    Read Input Registers Tests.
      ✓ should handle a valid request
      ✓ should handle a invalid request (invalid start address)
      ✓ should handle a invalid request (invalid quantity)
    Write Single Coil Tests.
      ✓ should handle a valid request
      ✓ should handle a invalid request (invalid start address)
    Write Single Register Tests.
      ✓ should handle a valid request
      ✓ should handle a invalid request (invalid start address)
      ✓ should handle a invalid request (invalid value, to big)
      ✓ should handle a invalid request (invalid value, float)
      ✓ should handle a invalid request (invalid value, negative)
    Write Multiple Coils Tests.
      ✓ should handle a valid request (with array)
      ✓ should handle a valid request (with buffer)
      ✓ should handle a invalid request (invalid start address)
      ✓ should handle a invalid request (invalid array size)
      ✓ should handle a invalid request (invalid buffer size)
      ✓ should handle a invalid request (inconsistent buffer size)
    Write Multiple Registers Tests.
      ✓ should handle a valid request (with array)
      ✓ should handle a valid request (with buffer)
      ✓ should handle a invalid request (invalid start address)
      ✓ should handle a invalid request (invalid array size)
      ✓ should handle a invalid request (invalid buffer size)

  TCP Modbus Request Tests
    ✓ should write a tcp request.

  TCP Request Tests
    ✓ should return a valid TCPRequest object for function 15

  Modbus/TCP Client Response Handler Tests
    ✓ should handle a valid read coils response
    ✓ should handle a exception
    ✓ should handle a chopped response

  Modbus/TCP Server Response Handler Tests
    ✓ should handle a valid read coils request
    ✓ should handle a valid read discrete inputs request
    ✓ should handle a valid read holding registers request
    ✓ should handle a valid read input registers request
    ✓ should handle a valid write coil request
    ✓ should handle a valid write register request
    ✓ should handle a valid write multiple coils request
    ✓ should handle a valid write multiple registers request

  TCP Server Tests.
    Write Single Coil Tests.
      ✓ should force a coil in the server buffer at address 1
      ✓ should force a coil in the server buffer at address 8
    Write Multiple Coils Tests.
      ✓ should write <0F> in the server buffer coils at address 0
      ✓ should write <FF> in the server buffer coils at address 0
      ✓ should write <FF 01> in the server buffer coils at address 0
      ✓ should write <0F> in the server buffer coils at address 6
      ✓ should write <0F> in the server buffer coils at address 8
    Write Multiple Registers Tests.
      ✓ should write <D903> in the server buffer at address 0
      ✓ should write <D903 D903> in the server buffer at address 0
      ✓ should write <D903 D903> in the server buffer at address 4

  WriteMultipleCoils Tests.
    WriteMultipleCoils Request
      ✓ should create a buffer from a write multiple coils message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  WriteMultipleRegisters Tests.
    WriteMultipleRegisters Request
      ✓ should create a buffer from a write multiple registers message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  WriteSingleCoil Tests.
    WriteSingleCoil Response
      ✓ should create a buffer from a write single coil message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  WriteSingleRegister Tests.
    WriteSingleRegister Request
      ✓ should create a buffer from a write single register message
      ✓ should create a message from a buffer
      ✓ should return null on not enough buffer data
      ✓ should return null on wrong function code

  128 passing (161ms)
stefanpoeter commented 6 years ago

you are using node 10.3.0. Can you execute the tests with node 8?

martijnthe commented 6 years ago

Hah, it works with Node 8. But I still think there's a bug. See my PR https://github.com/Cloud-Automation/node-modbus/pull/200. I suspect that the crc calculator is returning 0 when you give it NaN. And 0 happens to be your test's expectation for the crc value. I will take a look what's going on now.

martijnthe commented 6 years ago

OK, mystery solved. so buffer.readUInt16BE(NaN) returns 0 in Node 8, but in Node 10 it will throw.

Node 8

Buffer.prototype.readUInt16BE = function(offset, noAssert) {
  offset = offset >>> 0;  /// <---- NaN >>> 0 === 0
  if (!noAssert)
    checkOffset(offset, 2, this.length);
  return (this[offset] << 8) | this[offset + 1];
};

Node 10

function readUInt16BE(offset = 0) {
  checkNumberType(offset);
  const first = this[offset];
  const last = this[offset + 1];
  if (first === undefined || last === undefined)
    boundsError(offset, this.length - 2);

  return first * 2 ** 8 + last;
}
stefanpoeter commented 6 years ago

Good work! Will be fixed in 3.0.1.