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

Add BufReader impl #627

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DaneSlattery
Copy link

std::io implements a useful std::io::BufReader.

This is my attempt at implementing for embedded-io.

@DaneSlattery DaneSlattery requested a review from a team as a code owner August 30, 2024 09:57
@DaneSlattery
Copy link
Author

This idea spawned from esp-rs/esp-idf-svc#469

@MathiasKoch
Copy link

I think this already exists with https://github.com/rmja/buffered-io?

Whether to add it directly to embedded-io is of course another question.

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

I personally don't think this belongs inside embedded-io itself.

I do think it is useful though, and I've personally reached for https://github.com/rmja/buffered-io in my own projects. I think it's a question of

  • Is it useful/common enough to warrant living in here?
  • If so where should it go, a new utility crate?

This is probably something that needs to be discussed in the WG meeting.

@DaneSlattery
Copy link
Author

DaneSlattery commented Aug 30, 2024

I think this already exists with https://github.com/rmja/buffered-io?

I see that buffered-io only supports embedded-io-async, perhaps I can retarget this PR towards that crate for the non-async feature?

If so where should it go, a new utility crate?

Perhaps into /embedded-io-adapters?

This is probably something that needs to be discussed in the WG meeting

Let me know what comes of that WG meeting. Happy to make some changes, and perhaps to also replicate some of the ideas from buffered-io, such as passing in the buffer rather than allocating.

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