Conversation
|
Thanks a lot, i will review this as soon as I can. This seems to be a of a patch character so can you bump the patch version in package.json from 5.0.0 to 5.0.1. |
Thank you, sure please take your time. I'm testing in the meanwhile with multiple ModbusTCP systems - so far looks great, but I'm open for feedback! |
|
Hey @Barneey96, sorry for the delay but I got time to look at your code. Let's start with the TCP recovery, which I don't think we need. TCP itself handles validation and sequencing of packets, so the only thing we would be working against is a wrongly implemented counterpart — that's nothing we want to support. Either one speaks the right protocol or not. The second part is the RTU recovery, and here is the hard part. RTU does get flipped bits and such — that's what the CRC checksum is for — but your changes remove the error response entirely, then search for the right function code in the remaining bytes. That is too unreliable and can lead to even more issues, since it is not unlikely to find an FC 3 or 4 in there. Even adding the expected_slaveAddress boosts the probability but still doesn't make it reliable. On top of that, the corrupted=true path means CRC mismatches are no longer surfaced as err: 'crcMismatch' — they silently become timeouts instead, which is a breaking change for anyone handling that error. So in all cases these changes introduce more problems than the original issues they are trying to solve. What is your use case here? Where are you experiencing this? |
The fix verifies the RTU CRC during parsing and exposes a corrupted flag mirroring the existing ModbusRTURequest behaviour, so a misparse realigns the buffer instead of consuming it. It also gives each transport a real resynchronization step: Modbus/TCP skips a complete-but-unparsable frame whole using the exact length in its MBAP header and drops a single byte when the header itself is implausible, while Modbus/RTU drops a byte whenever the function code cannot begin any response body or the CRC fails. Recovery from a desynchronizing frame falls from 25 request timeouts to 1 on TCP and from 38 to 1 on RTU, and the same 256 stray-byte cases now all return the correct reading immediately, with the buffer-size limit retained as a backstop for the one case nothing can prove — a frame stalled behind a valid function code.