Skip to content

fix(rtu): replace decode loop with rolling buffer scan#361

Open
obayemi wants to merge 1 commit intoslowtec:mainfrom
obayemi:fix/rtu-server-rolling-buffer
Open

fix(rtu): replace decode loop with rolling buffer scan#361
obayemi wants to merge 1 commit intoslowtec:mainfrom
obayemi:fix/rtu-server-rolling-buffer

Conversation

@obayemi
Copy link
Contributor

@obayemi obayemi commented Feb 18, 2026

Current implemention of RTU server has some issues in real world situations:

  • if an other actor on the bus writes slowly, sending the messages fragmented, the current implementation will drop the start of the payload before receiving the end even for valide potential frames.
  • if some data e.g. a big write frame or multiple read response is sent on the bus, if some of the bytes did accidentally look like a modbus frame, with invalid CRC, the decoder would skip the whole frame even if the start of the actual frame was in the bytes contained in this erroneous detection.

this made the implementation unusable for buses with multiple other slaves.

this PR does:
Never mutate buffer during scanning — use read-only slices to check all byte offsets for valid CRC-confirmed frames. Always return Ok(None) instead of Err to keep FramedRead stream alive with partial data.

Add tests for fragmented arrival, back-to-back frames, garbage with valid function codes but bad CRC, partial frame preservation, all-garbage buffers, no Err on excessive garbage, and deep frame recovery.

Never mutate buffer during scanning — use read-only slices to check all
byte offsets for valid CRC-confirmed frames. Always return Ok(None)
instead of Err to keep FramedRead stream alive with partial data.

Add tests for fragmented arrival, back-to-back frames, garbage with
valid function codes but bad CRC, partial frame preservation, all-garbage
buffers, no Err on excessive garbage, and deep frame recovery.
@flosse
Copy link
Member

flosse commented Feb 18, 2026

Thanks for the PR!
I hope I'll get around to reviewing it soon. In the meantime, I'd love to hear feedback from others 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments