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

GPIO: If a common Instance trait is used for multiple peripherals, those traits should not have any logic implemented in them. #2713

Open
Tracked by #2492
bugadani opened this issue Dec 9, 2024 · 4 comments
Labels
1.0-blocker investigation Not needed for 1.0, but don't know if we can support without breaking the driver. peripheral:gpio GPIO peripheral

Comments

@bugadani
Copy link
Contributor

bugadani commented Dec 9, 2024

This mostly covers the different Pin traits, I believe.

@tom-borcin
Copy link

Hide all pin trait methods

@tom-borcin tom-borcin added the investigation Not needed for 1.0, but don't know if we can support without breaking the driver. label Dec 18, 2024
@bugadani
Copy link
Contributor Author

The only relevant trait with methods is Pin. Pasting here the rustdoc output, we have these:

pub trait Pin: Sealed {
    // Required method
    fn number(&self) -> u8;

    // Provided method
    fn degrade(self) -> AnyPin
       where Self: Sized { ... }
}
pub trait InputPin:
    Pin
    + Into<AnyPin>
    + 'static { }
pub trait OutputPin:
    Pin
    + Into<AnyPin>
    + 'static { }

I think in terms of public functions, this is reasonable. The other traits are intended to be hidden by #2693

@MabezDev
Copy link
Member

I think the only question is should number and degrade be inherent impls on each gpio struct, instead of in the trait?

I can see a world where a user uses our traits to be generic over some i/o pins and might want to access something like pin number. It's whether we expose it now, or not. We also have to be careful to not put the kitchen sink in the trait methods over the long term (this guideline should prevent us from doing this, in theory).

@bugadani
Copy link
Contributor Author

bugadani commented Dec 19, 2024

I guess degrade is questionable - InputPin and OutputPin both implement Into<AnyPin>. fn number is as close to a GPIO fn info() as possible right now, and we don't really publish those currently, so one could argue that we can hide that as well for the sake of consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0-blocker investigation Not needed for 1.0, but don't know if we can support without breaking the driver. peripheral:gpio GPIO peripheral
Projects
Status: Todo
Development

No branches or pull requests

3 participants