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.
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
Remove the single byte read/write inherent functions #2915
Remove the single byte read/write inherent functions #2915
Changes from 9 commits
7fa3564
154c792
3bbb707
73ab2e3
0d29d95
e64f3eb
475a7ec
cc96ed5
f2c40fd
8c48ab4
f15db5b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could expand on this a little bit - like mentioning to use a one byte sized buffer and checking the returned length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll have to take a bit of care here. UART's
read_bytes
is not blocking, SPI'sread_bytes
is blocking. As the peripherals work differently, it makes logical sense, but I'm not sure if we want the function use the same name, but do different things.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this should be resolved in #2882 which is a bit stalled due to this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please elaborate how it will be resolved, that PR still has a non-blocking
pub fn read_bytes(&mut self, buf: &mut [u8]) -> usize {
. Non-blocking in the sense that it doesn't fill up the buffer, but returns early if there is not enough data to read.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we could add some documentation or rename, it can also be done here, no hard opinions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imo this is fine as is, and if anything we should change uart's
read_bytes
back to being blocking and introduce a nb API post 1.0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little bit more documentation wouldn't hurt probably. "Read bytes from SPI" is very brief
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it matches
write_bytes
andtransfer
😆There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do the UART changes once these PRs land.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave a non-blocking one around, in my application I don't know how much data the transmitter is going to send so I need the non-blocking version to just read what is available 😅