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

Improved noise tolerance on recv #2

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

phooky
Copy link
Contributor

@phooky phooky commented Dec 3, 2017

Sorry, I made this fix a bit back but forgot to make another PR. This is mostly cleanup and a few improvements on the receiving end's handling of not-so-clean connections. (In particular it handles cases where some noise is sent to the receiver before good data comes through; the hardware I was receiving from was sending some spurious bytes at the start of transmission that lrzsz could handle but xmodem.rs would fail on.)

@awelkie
Copy link
Owner

awelkie commented Dec 5, 2017

So it looks like the main change here is that if the receive function gets an unexpected byte when it expects a packet header, we'll just keep trying instead of considering that an error. I'm not sure if this is what we want to do, though, since it seems like we should treat unexpected bytes as something bad. For example, it looks like if the transmitter sends the same unexpected byte over and over, this receive function will never return.

It seems like the right way to handle this is to allow the user some control over the types of errors that are allowed. So the user could specify to quit after some number of CRC errors, some number of packet timeouts, and maybe an infinite number of unexpected bytes. Or maybe the user could supply a callback between each packet to determine at run-time whether to quit the receive.

Thoughts?

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