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

improve spi traits #282

Merged
merged 2 commits into from
Dec 12, 2021
Merged

Conversation

andrewgazelka
Copy link
Contributor

@andrewgazelka andrewgazelka commented Nov 26, 2021

  • rename to SPI traits to possibly better names

    • SpiAll -> HalSpi
    • SpiEnabled -> HalEnabledSpi
    • SpiDisabled -> HalDisabledSpi
  • reset no longer needs to take ownership of the given SPI peripheral

@richardeoin
Copy link
Member

The rename seems fine, thanks! I agree that the reset method should use a &mut self receiver, but your implementation seems a bit extreme. My suggestion would be instead:

  • Move the implementation from the trait to the ordinary API, so that the reset method is available even when not using the traits
  • Add a private method like fn internal_disable(&mut self) that contains the register writes, and use that method in both the pub fn disable(self) and pub fn reset(&mut self) methods. If you like, private methods can be implemented for both typestates (inside impl<EN> Spi<$SPIX, EN, $TY> { ... }) whilst keeping the external typestate API.

Copy link
Member

@richardeoin richardeoin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, thanks!

@richardeoin
Copy link
Member

bors r+

@bors bors bot merged commit 946d54b into stm32-rs:master Dec 12, 2021
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.

2 participants