sys/isrpipe: add isrpipe_write#17336
Conversation
sys/include/isrpipe.h
Outdated
| * @returns number of characters that could be added | ||
| * @returns -1 if buffer was full | ||
| */ | ||
| int isrpipe_write(isrpipe_t *isrpipe, uint8_t *buf, size_t n); |
There was a problem hiding this comment.
| int isrpipe_write(isrpipe_t *isrpipe, uint8_t *buf, size_t n); | |
| int isrpipe_write(isrpipe_t *isrpipe, const void *buf, size_t n); |
There was a problem hiding this comment.
Isn't it better to keep consistency with isrpipe_init which also uses uint8_t *buf ?
There was a problem hiding this comment.
You can add arbitrary data here, no? The API doesn't have to care about what buffer is used to back the data structure
There was a problem hiding this comment.
Sure, but ringbuffer for instance even uses const char *buf so maybe it's worth a thought to remodel those values in general.
There was a problem hiding this comment.
It's just more convenient to use an API that takes a void * for arbitrary data than a uint8_t * - let the function do the cast if it needs to operate on bytes, not the caller.
There was a problem hiding this comment.
I don't have a strong opinion here, maybe just in favor of keeping things aligned, currently isrpipe is lying on top of tsrb which is using uint8_t, and all current users use uint8_t.
There was a problem hiding this comment.
I added a const qualifier. I'm for keeping uint8_t as well, for uniformity.
c3ddfe1 to
d1cb0a0
Compare
d1cb0a0 to
774e765
Compare
Contribution description
The
isrpipemodule is lacking a function to add a batch of values so we don't need to externally loop onisrpipe_write_one. This PR introducesisrpipe_write.Testing procedure
There is no dedicated test for
isrpipebut it's basically covered by the test oftsrbalready.