rust-osdev / uart_16550

Minimal support for uart_16550 serial output.
MIT License
30 stars 23 forks source link

Add try_create and loopback_test for SerialPort #31

Open Wasabi375 opened 9 months ago

Wasabi375 commented 9 months ago

fixes #30

This adds try_create and loopback_test to SerialPort.

I was thinking it might be nice if this crate, rather than marking SerialPort::new as unsafe, instead rolled init into new (I don't see that there would ever be a reason not to call init?)

The reason AFAICT is that new is a const fn which would not be possible if init is done at the same time. Therefor loopback_test is a separate function that can be used independent of the new try_create.

I was considering also adding a loopback_test_with_data to allow the caller to specify what byte to use for the loopback test, but I can't think of any good reason why this would be needed, and it can always be added in the future.

There are 2 issues I can still see with the PR.

  1. Result<(), ()> and Result<Self, ()> feels a bit cluncky. For the second we could use Option but I personally don't like using option for errors, even if there isn't a specific reason. Maybe add we could add a struct LoopbackTestFailure {}.
  2. So far I haven't implemented this for the memory mapped version of uart. The reason is that I don't see how a loopback test would work there. Either the memory is not mapped properly in which case init will crash anyways or it is mapped but just to regular memory in which case a loopback test will always succed.

Let me know if there is anything that I should adjust.