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

serial_bytes_available: don't double-count bytes on usb cdc #9400

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

jepler
Copy link
Member

@jepler jepler commented Jul 3, 2024

In 9.0.x, serial_bytes_available returned a bool and tud_cdc_available was harmlessly checked twice for any characters.

When the routine was changed to return an int, the double checking led to over-reporting the number of characters available. In code that would attempt to read this many bytes from sys.stdin, this made the read call block since only 1 byte was actually available.

This behavior came up in the discussion of #9393. I don't mark this bug as closing that one, because that issue seems to be reporting multiple things that this change would not address, such as delays in sys.stdout.write() or problems seen while using webserial.

ping @gitcnd

In 9.0.x, serial_bytes_available returned a bool and tud_cdc_available was
harmlessly checked twice for any characters.

When the routine was changed to return an int, the double checking led to
over-reporting the number of characters available. In code that would attempt
to read this many bytes from sys.stdin, this made the read call block since
only 1 byte was actually available.

This behavior came up in the discussion of adafruit#9393. I don't mark this bug as
closing that one, because that issue seems to be reporting multiple things that
this change would not address, such as delays in `sys.stdout.write()` or
problems seen while using webserial.
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for figuring this out!

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.

3 participants