Open
Conversation
When we call read_timeout to fill the report buffer, if we specify a size smaller than the max input report length, the report can become truncated. The report will indicate that it has length M, where the read size is N; when M > N, the data would be lost. So we need to buffer up to the max input report length to avoid losing data. This also fixes a bug that resulted in a panic: the existing code might try to fill the last N bytes of the data output buffer, while in actuality attempting to write M bytes to the data slice, resulting in an out-of-range panic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello,
Thanks for writing this crate!
Unfortunately, I ran into a couple problems:
hidforhidapi, I ran into an issue wherereadwould request N bytes from the HID layer, which would return abufferwhereinbuffer[0](i.e. the RX FIFO report length) -- call it M -- was such that M > N. This would then entail writing more bytes todatathandata.len(), causing apanic.read's timeout.To resolve No. 2, I did the only thing I could think of: create a buffer within
HidUartthat can handle the worst case scenario of up to the max report length being unused; subsequent reads would first drain that buffer before hitting the HID layer. (Admittedly I could save a couple bytes --INTERRUPT_REPORT_LENGTHisn't strictly necessary, since one byte from the report is always the length, and we'll always read at least one byte (otherwise we wouldn't have requested the report in the first place) -- so something likeINTERRUPT_REPORT_LENGTH - 2for this buffer's size should suffice, but I figured I'd keep it simple).Hopefully the other changes are pretty straightforward, but I'd be happy to answer any questions and/or implement any feedback.
I'm currently using this branch while I develop my ut61e-rs crate, which provides a means for reading measurement messages from the UT61E digital multi-meter, and it's going well so far, for what that's worth.