Skip to content

Conversation

Wasabi375
Copy link

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.

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

Successfully merging this pull request may close these issues.

Suggestion: roll init into new, test with loopback + scratch pad register, and return Result instead of marking SerialPort as unsafe
1 participant