Data length error

[harshit-malik7]

{"level":"info","message":"Timestamp: 2025-01-17T22:02:20.880Z, msg: "HVAC Id: 1 Fetching HVAC mode", correlationId: 9432, function: getHVACMode, level: info"}
{"level":"error","message":"Timestamp: 2025-01-17T22:02:20.962Z, msg: "HVAC Id: 1 Failed to get HVAC mode because of Error: Data length error, expected 7 got 6", correlationId: 9432, function: getHVACMode, level: error"}
const response = await requestedHVAC.readHoldingRegisters(803,1);
HVACMode = response.data[0];
it seems to happen sometimes but sometimes it goes through. Same issue with readcoils and readDiscreteInputs as well.

[Poseidonas]

I think I can account for this one. It is not the device sending a short reply — it is a reply from an earlier request arriving after that request had already timed out.

Reproduced

1) readCoils on txn=1, expects 6 bytes  → timed out, device answers late
2) transaction id wraps back to 1 after 256 requests
3) readHoldingRegisters on txn=1, expects 7 bytes
   the late readCoils reply arrives and is matched to it
   → Data length error, expected 7 got 6

Why the two get confused

_transactionIdWrite wraps at MAX_TRANSACTIONS (256) in ports/tcpport.js:429, and entries in this._transactions are never removed — the object is only ever reset wholesale on reconnect. So after 256 requests an identifier comes round again, and a reply still in flight from the previous holder of that identifier looks, to _onReceive, exactly like the reply to the current one. The transaction id is the only thing distinguishing them and it is no longer unique in time.

That fits your symptoms: intermittent rather than constant, since it needs a slow reply and a wrap to coincide; and appearing across readCoils, readDiscreteInputs and readHoldingRegisters alike, since any pairing of two different expected lengths produces it. With a busy HVAC controller and correlation ids in your logs, 256 requests come round quickly.

Why the message is misleading

In index.js the length check sits at line 544 and the function code check at line 567. The stale reply fails the length check first, so you are told about a byte count when the real problem is that the frame belongs to a different request. Moving the function code check ahead of the length check reports what actually happened:

before:  Data length error, expected 7 got 6
after:   Unexpected data error, expected code 3 got 1

That is a two-line move and the existing suite still passes (167 tests, lint clean). It does not stop the mismatch — it names it, which at least points at the cause rather than away from it.

Actually preventing it is a design question I would not want to answer on the maintainers' behalf. A transaction could be removed once it has settled and its identifier held back from reuse until the socket has been quiet, or the port could refuse to reuse an identifier whose transaction is still outstanding. Either changes how identifiers are allocated, so it is worth a decision rather than a patch from a passer-by.

Happy to open a PR for the check reordering if that is useful on its own, and to attach the reproduction as a test.

Meanwhile, something that should reduce it for you today: the wrap is what makes a stale reply collide with a live request, so anything that lowers the number of in-flight and timed-out requests helps. A longer setTimeout on the client means fewer requests are abandoned while the device is still working on them, and serialising the polling loop rather than letting cycles overlap keeps identifiers from cycling as fast.

[Poseidonas]

Opened #625 with the check reordering and a regression test. It only corrects the message — the identifier reuse itself is untouched, for the reasons above.