Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reunite for Serial Tx/Rx #386

Open
horazont opened this issue Jan 8, 2022 · 0 comments · May be fixed by #390
Open

Add reunite for Serial Tx/Rx #386

horazont opened this issue Jan 8, 2022 · 0 comments · May be fixed by #390

Comments

@horazont
Copy link
Contributor

horazont commented Jan 8, 2022

With some functions (e.g. reconfiguration) only safely available when the peripherial is fully under control of the caller, it would make sense to be able to recombine the Tx and Rx parts after a split.

reunite [1] was attempted in #36 (under the name "release"), however, it was closed because the ReleaseToken was deemed unnecessary. Upon attempting an implementation without the ReleaseToken, I came into a questionable situation:

To avoid the release token:

  • the PINS type information must be carried on to the Tx and Rx instances somehow.
  • the USART and PINS instances need to be carried on to either Tx or Rx in order to be able to recover them when reuniting

These points can be addressed by moving the PINS and USART instances to Tx and adding a PINS PhantomData to the Rx (or vice versa).

However: This has fallout to the {T,R}x{Dma,}{1,2,3} type aliases, which then become generic over PINS. That seemed somehow excessive and is for sure a breaking change for downstream code, so I wanted to run this by you folks before venturing further down this direction.

  • Should the type aliases be generalized or should there be explicit aliases for the two options for each USART?

    Edit: I just realized that there are many more than just two options: The pins can not just be remapped, but also in OpenDrain/PushPull and the different input modes, making for many many more than just two combinations.

  • Should we move ahead with the release-token-free way or does this caveat warrant keeping that token (which I personally also find a bit awkward)?

  • If not generalizing the type aliases, what would be good names? Tx1 and Tx1Alt?

    [1]: The name reunite has prior art in the owned halves of of tokio::net::TcpStream. There is also unsplit in tokio, but given the choice I'd prefer reunite.

P.S.: If it wasn't clear: I'm happy to make an implementation if we can settle the discussion points :)

horazont added a commit to horazont/stm32f1xx-hal that referenced this issue Jan 9, 2022
This becomes relevant as more functions like reconfigure are added to the
Serial structs.

Fixes stm32-rs#386.
horazont added a commit to horazont/stm32f1xx-hal that referenced this issue Jan 9, 2022
This becomes relevant as more functions like reconfigure are added to the
Serial structs. We use the Tx to stash away the usart instance used by
the (Erased-)Serial struct, but this choice is arbitrary and irrelevant
because it is zero-sized anyway.

Fixes stm32-rs#386.
@horazont horazont linked a pull request Jan 9, 2022 that will close this issue
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 a pull request may close this issue.

1 participant