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

Tcp socket read, without read_with #17

Closed
MathiasKoch opened this issue Jun 29, 2020 · 5 comments
Closed

Tcp socket read, without read_with #17

MathiasKoch opened this issue Jun 29, 2020 · 5 comments
Labels
enhancement 👍 New feature or request

Comments

@MathiasKoch
Copy link
Member

Currently this crate implements a slightly modified version of the TcpStack trait from embedded-nal, with the addition of

fn read_with<F>(&self, socket: &mut Self::TcpSocket, f: F) -> nb::Result<usize, Self::Error>
    where
        F: FnOnce(&[u8], Option<&[u8]>) -> usize;

As this allows peeking into the buffer, to selectively do copy, only if a full valid packet is in place (eg an mqtt packet).

We need to figure out a way to either commit the read_with fn upstream, if deemed useful, or use the regular buffered read without introducing additional clones

NOTE
This is the blocking issue for releasing to crates.io, as git dependencies are not allowed on crates.io

@keisrk
Copy link
Contributor

keisrk commented Dec 10, 2020

Came from this. As mentioned earlier, we don't want to divide the ecosystem.
Still, can we have a polyfill trait ClosureBasedZeroCopy: TcpClient { fn read_with(..) .. } for the time being? It's just a temporary workaround.

@MathiasKoch
Copy link
Member Author

Yep, i think that would be a great temp solution 👍

I will add it in mqttrust, where the read_with is required

@MathiasKoch
Copy link
Member Author

Hmm.. nope, should probably be added somewhere else? Maybe ublox-cellular? or a completely new repo of temporary extension traits for our device?

@keisrk
Copy link
Contributor

keisrk commented Dec 10, 2020

Or maybe we can simply update your factbird-1.0 branch basing on v0.2.0. I just made a PR. I can explore the both approaches:)

@keisrk
Copy link
Contributor

keisrk commented Dec 11, 2020

Hi @MathiasKoch , perhaps we can work around the zero-copy part of discussion. I made another PR, so if you have a spare time, can you quickly have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 👍 New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants