Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: uncaught error in read device identification #557

Merged

Conversation

everhardt
Copy link
Contributor

I ran into the following error when communicating with a solar logger over Modbus TCP:

RangeError [ERR_OUT_OF_RANGE]: The value of "offset" is out of range. It must be >= 0 and <= 12. Received 43
     at __node_internal_captureLargerStackTrace (node:internal/errors:490:5)
     at new NodeError (node:internal/errors:399:5)
     at boundsError (node:internal/buffer:88:9)
     at Buffer.readUInt8 (node:internal/buffer:254:5)
     at _readFC43 (/app/node_modules/modbus-serial/index.js:267:40)
     at ModbusRTU._onReceive (/app/node_modules/modbus-serial/index.js:538:13)
     at TcpPort.emit (node:events:513:28)
     at Socket.<anonymous> (/app/node_modules/modbus-serial/ports/tcpport.js:122:22)
     at Socket.emit (node:events:513:28)
     at addChunk (node:internal/streams/readable:324:12)
     at readableAddChunk (node:internal/streams/readable:297:9)
     at Readable.push (node:internal/streams/readable:234:10)
     at TCP.onStreamRead (node:internal/stream_base_commons:190:23)

Two problems here:

  1. the error was uncaught, ie. it crashed my program
  2. there are two interpretations of numOfObjects: the number of objects in the response or the number of objects in all responses combined. The spec states the former, but the example on page 45 shows the latter. The solar logger I was communicating with apparently uses the latter as well.

This PR addresses both problems.

@yaacov yaacov merged commit b5bdb8a into yaacov:master Jun 8, 2024
3 checks passed
@yaacov
Copy link
Owner

yaacov commented Jun 8, 2024

thanks 💚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants