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

arduino-hal: provide renamed types matching fields #559

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

LuigiPiucco
Copy link
Contributor

A very simple improvement to fix #558 and fix #555.

The approach is a basic application of renaming with paste!, but I choose to generate slightly different code than suggested:

pub struct Pins {
    // Notice the lack of documentation here, I thought it became redundant...
    pub a0: PinA0<
        mode::Input<mode::Floating>,
    >,
    // ...
}

// ...since I moved it to the type aliases themselves.

/// `A0`
///
/// * ADC0 (ADC input channel 0)
/// * PCINT8 (pin change interrupt 8)
pub type A0 = atmega_hal::port::PC0;
// ...

// It seemed more consistent to use the pin name in uppercase for the
// raw type and "Pin<pin name in uppercase>" for the wrapped version.

/// Safely wrapped version of:
/// `A0`
///
/// * ADC0 (ADC input channel 0)
/// * PCINT8 (pin change interrupt 8)
pub type PinA0<MODE> = Pin<MODE, atmega_hal::port::PC0>

In rust-analyzer the type aliases are resolved to the underlying type, so in this implementation the documentation popups still show the MCU names. We could fix this by using a newtype with Deref, but that seems like overkill.

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Hey, thanks a lot for taking care of this.

In rust-analyzer the type aliases are resolved to the underlying type, so in this implementation the documentation popups still show the MCU names. We could fix this by using a newtype with Deref, but that seems like overkill.

Same issue with compiler error messages. Such is the life of using type aliases in Rust at the moment... The only thing we can do is making sure people find the aliased type names in the docs easily.

Newtype won't work when HAL functions expect a specific pin. Yes, you can fix that using even more layers of traits and generics, but that's way worse still.

avr-hal-generic/src/port.rs Outdated Show resolved Hide resolved
Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Thanks a lot!!

@Rahix Rahix merged commit afdadeb into Rahix:main Jun 10, 2024
25 checks passed
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.

Enhance avr_hal_generic::renamed_pins! {} to also generate type aliases for each pin peripherals type alias
2 participants