Skip to content

Optimize recv() with suggested fixes#61

Merged
jquast merged 5 commits intomasterfrom
jq/dcliftreaves-32
Mar 4, 2026
Merged

Optimize recv() with suggested fixes#61
jquast merged 5 commits intomasterfrom
jq/dcliftreaves-32

Conversation

@jquast
Copy link
Collaborator

@jquast jquast commented Mar 4, 2026

Supersedes #32 by @dcliftreaves

  • bugfix: retry_limit was never actually triggered during the data transfer phase because errors never accumulated, and
  • enhancement: replace 3x getc() calls with a single recv() call, reducing timing and failed packets when using fast serial lines without flow control.

hh-decr and others added 3 commits February 27, 2026 20:52
Read sequence bytes, packet data, and checksum in a single getc() call
instead of multiple separate calls. This reduces timing issues when
communicating with embedded systems over fast serial lines without
hardware flow control, where multiple reads with separate timeouts can
stack up and cause buffer overruns.

Also fix the inner header loop to break and NAK on bad headers rather
than sleeping for the full timeout duration, which caused problems with
embedded systems.

Add comprehensive unit tests for recv() covering:
- Successful single-block and multi-block transfers
- Bad sequence numbers and bad sequence complements
- Corrupted CRC checksums
- Short (badly sized) blocks
- Timeout during block data read
- Checksum mode (crc_mode=0)
- Cancellation via 2xCAN during data phase
- Retry limit exhaustion on repeated errors

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Single CAN aborts transfer but spec requires 2x CAN
- Short batched read could crash _verify_recv_checksum with IndexError
@jquast
Copy link
Collaborator Author

jquast commented Mar 4, 2026

Because the retry_limit changes behavior a bit of a change, that somebody might have previously depended on "unlimited retries" I will bump to version 0.5.0

@jquast jquast marked this pull request as ready for review March 4, 2026 01:16
@jquast jquast merged commit 579a93b into master Mar 4, 2026
8 checks passed
@jquast jquast deleted the jq/dcliftreaves-32 branch March 4, 2026 01:17
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