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

Change the way transmit buffer lengths are populated? #258

Closed
robomancer-or opened this issue Jul 31, 2018 · 5 comments
Closed

Change the way transmit buffer lengths are populated? #258

robomancer-or opened this issue Jul 31, 2018 · 5 comments

Comments

@robomancer-or
Copy link
Contributor

robomancer-or commented Jul 31, 2018

So I'm implementing phy::TxToken for my board, and it seems really weird to me that the signature is:

fn consume<R, F>(self, timestamp: Instant, len: usize, f: F) -> Result<R>
    where F: FnOnce(&mut [u8]) -> `Result<R>;

What this suggests is that the number of bytes populated by the call f(&mut buf) is known before f is called... which strikes me as wacky. Is there some reason the signature isn't

fn consume<R, F>(self, timestamp: Instant, f: F) -> Result<R>
    where F: FnOnce(&mut [u8]) -> `Result<(usize, R)>;

where f returns the number of bytes actually populated?

Would you be open to a PR that makes this change?

@whitequark
Copy link
Contributor

Yes. With the signature you suggest, how large the buffer you pass to f is going to be?

@robomancer-or
Copy link
Contributor Author

robomancer-or commented Jul 31, 2018

Ah, I see, I forget that some people can use dynamic memory. So it's up to the caller to request exactly as many bytes as their packet is going to end up being... that still seems odd. What about

fn consume<R, F>(self, timestamp: Instant, capacity: usize, f: F) -> Result<R>
    where F: FnOnce(&mut [u8]) -> `Result<(usize, R)>;

where the understanding is that the one returned from the closure is the one that represents the number of bytes to transmit.

@whitequark
Copy link
Contributor

What's the point of this change exactly? smoltcp is specifically designed to know the exact number of bytes it's going to transmit upfront.

@robomancer-or
Copy link
Contributor Author

robomancer-or commented Jul 31, 2018

Oh. I didn't realize that. Sorry, the documentation on docs.rs is pretty out of date, so I'm trying to work out how to use it all by reading the source, I missed that tidbit. Forget I said anything :-)

@whitequark
Copy link
Contributor

Ah, sorry for that, I should make a release soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants