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

Potentially confusing read_exact timeout semantics #177

Open
RossSmyth opened this issue Apr 11, 2024 · 1 comment
Open

Potentially confusing read_exact timeout semantics #177

RossSmyth opened this issue Apr 11, 2024 · 1 comment

Comments

@RossSmyth
Copy link
Contributor

Since this library uses the default_read_exact implementation, read_exact has different timeout semantics compared to read. Since the implementation of this default method is to call read in a loop, each loop has a new timeout. This could potentially lead to confusing timeouts:

  1. Set some baud & timeout. Say 9600 bps and 110ms
  2. Call read_exact with 100 bytes. This would mean it would take about 100ms for a full transfer if each byte is contiguous
  3. In the degenerate case, one byte could come every 109ms, leading to the method call hanging for ~11s.

At minimum this should be documented somewhere.

@sirhcel
Copy link
Contributor

sirhcel commented May 3, 2024

Thank you very much for spotting this issue and the steps for reproducing it! It definitely looks like we are not playing nice with respect to the principle of least surprise here. I agree that this should be documented. But as documentation is seldomly read, a custom implementation of read_exact adhering the configured timeout for each call would be even better.

Since you asked how to get things forward here in #171: Creating a PR for the documentation would be a good first step. And filing a PR with a test for reproducing this issue in tests/ and an implementation of read_exact which takes care of the timeout would be awesome. Sure, this issue likely affects all platforms. But this also includes Windows. 😉

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

No branches or pull requests

2 participants