Expose the Backend Trait publicly

[marco-swz]

For testing purposes, I want to use Port with a custom simulator as backend. This would be as simple as implementing the Backend trait for my simulator struct. Unfortunately, Backend is currently not accessible outside the crate.

This pull request removes the Sealed trait and consequently makes Backend publicly accessible. Users can now implement their own backend and instantiate ports with Port::from_backend().

Let me know what you think!

[stphnt]

@marco-swz, unsealing the Backend was always something I expected to do, but was waiting until there was a need (and now there is). It seems like this and the Mock backend are solving the same problem but in different ways, so I'm trying to better understand your use case here. What is the problem you're trying to solve? Why does the Mock Backend not work for you? What does this give you that you don't get with Mock?

[marco-swz]

At the earlier stage of my project, sending fixed responses to messages was sufficient to test my program. Now that the main functionality is finished, I want to focus on better tests. It is not enough anymore to check the messages itself. I want to make sure the order of execution is correct and complicated edge cases are handled properly, potentially even going in the direction of simulation testing.

Basically, I don't want to send fixed responses, but make them dependent on the internal state of the backend. Unfortunately, this is not possible with the Mock backend in its current form. For example, write_callback would have to provide a state as an additional callback argument to make this work. Implementing this change seems like an odd choice, when alternatively, I can just implement the Backend trait myself and achieve the same goal with more flexibility.

Before the Mock backend was public, I even thought about implementing the Backend trait instead. But for quick tests, Mock provided a simpler interface. I still think Mock is useful to the user, even though a public Backend would make it obsolete. It is substantially more work to implement all traits, instead of defining a simple callback function for Mock.
Maybe this is a path to explore: Provide auto-implementations for io::Write and io::Read (if this is possible), to simplify the implementation of Backend and make Mock private again.

[stphnt]

I want to make sure the order of execution is correct and complicated edge cases are handled properly, potentially even going in the direction of simulation testing.

This doesn't sound trivial. It's also worth noting that long commands may be split across multiple packets, which your backend would need to merge together. Similarly, for replies longer than the maximum packet size, it would need to split them (see https://www.zaber.com/protocol-manual#topic_line_continuation for details). Mock doesn't do this, but the assumption there is that the use cases are simple.

Basically, I don't want to send fixed responses, but make them dependent on the internal state of the backend. Unfortunately, this is not possible with the Mock backend in its current form. For example, write_callback would have to provide a state as an additional callback argument to make this work.

Right, the signature of the callback in set_write_callback is fn(_: &[u8], _: &mut dyn Write), which only function pointers implement, not closures. Changing it impl FnMut(_: &[u8], _: &mut dyn Write) would allow the callback to be a closure that maintains state. Doing so would be a simple, backwards compatible change and a smaller change to the crate's API.

For instance it would let you do something like the following:

let mut sim = MySimulatedDeviceChain::new();
let mut port = Port::open_mock();
port.backend_mut().set_write_callback(move |message, buffer| { sim.handle(message, buffer); });

I've gone ahead an implemented this in #185, as it's plainly better. Does that work for your use case?

Before the Mock backend was public, I even thought about implementing the Backend trait instead. But for quick tests, Mock provided a simpler interface. I still think Mock is useful to the user, even though a public Backend would make it obsolete. It is substantially more work to implement all traits, instead of defining a simple callback function for Mock.

I agree, for simple cases Mock seems useful. Though, I think its API could be improved.

Maybe this is a path to explore: Provide auto-implementations for io::Write and io::Read (if this is possible), to simplify the implementation of Backend ...

This isn't possible, unfortunately, for multiple reasons. First, there is not single implementation that would be correct for all backends. Second, std::io::Write and std::io::Read are external traits so the compiler will not allow me to implement it for external types (including blanket implementations), in order to avoid having conflicting implementations (this is the orphan rule).

Regarding this change in general, I'm not opposed to exposing the Backend trait if its necessary, though I do not want to expose Port::from_backend in its current form: it doesn't mirror any of the existing ways of opening a Port. I'd want to think about that more before unsealing the Backend trait.

[marco-swz]

This doesn't sound trivial. It's also worth noting that long commands may be split across multiple packets, which your backend would need to merge together. Similarly, for replies longer than the maximum packet size, it would need to split them (see https://www.zaber.com/protocol-manual#topic_line_continuation for details). Mock doesn't do this, but the assumption there is that the use cases are simple.

Thanks for the info! Fortunately, I don't need to implement the entire functionality in my simulator, only the few commands I use in my program.

I've gone ahead an implemented this in #185, as it's plainly better. Does that work for your use case?

This is a great improvement, since fn(_: &[u8], _: &mut dyn Write) was unnecessarily strict. I tested it and it works for my application. It is just a little bit less convenient due to the Rc<RefCell<>> I need to access the simulator outside the closure (unless there is another way I am not aware of).

Nevertheless, I will probably stay on my fork with public Backend for now, because I am unsure whether I would need more modification to the Mock backend in the future. I already feel bad enough, that I constantly make you add changes just because of me 😅. I keep forgetting, that as an author of a public crate, you can't keep changing your API because you changed your mind.