tokio-serial icon indicating copy to clipboard operation
tokio-serial copied to clipboard

Whats up with `SerialPortBuilderExt`?

Open WolleTD opened this issue 3 years ago • 1 comments

I was just checking out this crate for a project and stumbled upon this part in the docs. Apparently, there are two SerialPortBuilderExt traits doing more or less the same thing, one defined in mio-serial, one in tokio-serial (with a different SerialPort type, which doesn't help much).

Also I understand neither the native nor the async part of the method name as it just wraps around the sync and non-native SerialStream::open()?

WolleTD avatar May 06 '22 23:05 WolleTD

Ah, it took some time, but I think I get it: SerialPortBuilder is actually re-exported all the way from the serialport crate. That's why open() returns some Box<dyn SerialPort> and not a mio_serial::SerialStream and the trait is added to create a SerialStream

So, mio_serial::SerialPortBuilderExt::open_native_async() is actually an open_stream() or mio_open() or something. It's just saving boilerplate and/or confusing documentation by reusing the SerialPortBuilder and actually would want to override it's open().

Then, tokio_serial::SerialPortBuilderExt is quite similar, but one level higher. We still want to reuse the SerialPortBuilder as a whole, but now produce a tokio_serial::SerialStream. So tokio_serial::SerialStream::open() should be an async fn to match the interface of other I/O objects and therefore this open_native_async() should be open_async() and actually return a future.

In code, something like this:

// mio-serial
pub trait SerialPortBuilderExt {
  fn mio_open(self) -> Result<mio_serial::SerialStream>;
}

// tokio-serial
impl SerialPort {
  async fn open(builder: &SerialPortBuilder) -> Result<Self>;
}

pub trait SerialPortBuilderExt {
  // with the appropriate async-in-trait-workarounds, of course
  async fn open_async(self) -> Result<tokio_serial::SerialStream>;
}

It should probably be either mio_open/async_open or open_mio/open_async, though.

WolleTD avatar May 08 '22 13:05 WolleTD