add split traits to fill functionality gaps in downstream crates - #748
add split traits to fill functionality gaps in downstream crates#748celogic wants to merge 2 commits into
Conversation
|
Splitting semantics get tricky, you haven't accounted for anything beyond just splitting (think: configuration that affects both sides etc) . I also don't believe this is a e-hal specific problem so I'm not sure it belongs here. |
|
I got some concerns:
trait BorrowedSplit {
type Reader<'a>: Read where Self: 'a;
type Writer<'a>: Write where Self: 'a;
fn split_rw(&mut self) -> (Self::Reader<'_>, Self::Writer<'_>);
}
trait OwnedSplit {
type Reader: Read;
type Writer: Write;
fn split_rw(self) -> (Self::Reader, Self::Writer);
}
|
|
@celogic why close without any comment? My concerns don't necessarily mean we shouldn't add a Split trait, just that we should have a discussion beforehand of what'd be the best design for it. It's a bit disrespectful to the maintainer to outright close with no comment after the maintainer has put in time to review. |
|
@Dirbaio my apologies, my intention in closing this pr, was, to show that my design, as proposed, has issues and is not ready to be merged. p.s. should I reopen this pr? |
|
@Dirbaio what about this design? trait SplitRW /* borrowed */ {
type Reader<'a>: Read where Self: 'a;
type Writer<'a>: Write where Self: 'a;
// p.s. i guess this fn is the same as yours, but explicit
fn split_rw<'s>(&'s mut self) -> Result<(Self::Reader<'s>, Self::Writer<'s>), () /* optionally suitable error type or no result at all*/>;
}
// provide owned variant if it makes sense (not my primary interest)
trait IntoSplitRW /* owned */ {
type Reader: Read;
type Writer: Write;
type Controller; // can be used to handle the underlaying socket e.g. set_timeout, close, set_keep_alive
fn into_split_rw(self) -> Result<(Self::Reader, Self::Writer, Self::Controller), ()>;
}
// optionally the underlaying socket provides a fn to reassemble itself?
fn Socket::reassemble(Reader, Writer, Controller) -> Result<Self, ()>;note that GAT (generic associated types) is a new feature in rust (1.65). maybe thats the reason why this trait (borrowed variant) is not provided in std::io example: use embassy_futures::join::join;
socket.open();
let (mut reader, mut writer) = socket.split().unwrap();
// or threads
join(
async {
reader.read(...);
...
},
async {
writer.write();
...
},
)
.await;
socket.close() |
|
here is a quick example how I would implement it: https://github.com/celogic/split_demo if somebody is curious: pub fn split(&mut self) -> (TcpReader<'_>, TcpWriter<'_>)is the implicit form of pub fn split<'s>(&'s mut self) -> (TcpReader<'s>, TcpWriter<'s>)which implies, that impl<'a> TcpSocket<'a> {
pub fn split<'s>(&'s mut self) -> (TcpReader<'a>, TcpWriter<'a>)
}if they would want pub struct TcpReader<'s, 'a> {
io: TcpIo<'a>,
_socket: PhantomData<&'s TcpSocket<'a>>,
}
impl<'a> TcpSocket<'a> {
pub fn split<'s>(&'s mut self) -> (TcpReader<'s, 'a>, TcpWriter<'s, 'a>);
}this mismatch causes lifetime issues in implementing |
Adds a
SplitRWtrait for both async and non async versions.It abstracts the capability, making them available for downstream libraries.
Example:
embedded_tls::TlsConnectionbuilds on (abstract)<Socket: embedded_io_async::Read + embedded_io_async::Write>.embedded_tls::TlsConnectionimpl the fnsplit, but it requiresSocket: Clone.this is not effective as
Socketdoesn't need to be completely cloned, which causes functionality gaps.e.g. if i use
embassy_net::tcp::TcpSocketas the underlayingSocket, I cant callTlsConnection::split,because
embassy_net::tcp::TcpSocketdoesn't implClone, despite providingsplititselfstd::io::Split, which is anIteratorSplitRWdoesn't requireRead+Write, because i don't think its necessary and leaves the implementor more flexible