avr-hal icon indicating copy to clipboard operation
avr-hal copied to clipboard

`impl_generic_pin` `impl_port` and `impl_board_pins` macros are confusing

Open fneddy opened this issue 4 years ago • 1 comments

the impl_generic_pin impl_port and impl_board_pins macro matches on a syntax that looks like native rust code, but in this case it is just the macro syntax: https://github.com/Rahix/avr-hal/blob/d17a441a667dd65e5a9be75b0e71d1beadf93e84/avr-hal-generic/src/port.rs#L177-L182 https://github.com/Rahix/avr-hal/blob/d17a441a667dd65e5a9be75b0e71d1beadf93e84/avr-hal-generic/src/port.rs#L678-L695 https://github.com/Rahix/avr-hal/blob/d17a441a667dd65e5a9be75b0e71d1beadf93e84/avr-hal-generic/src/port.rs#L365-L380

this is relay confusing for someone who is new and expects structs to behave the same everywhere where they are used.

I would like to propose to use an API here that is more clear. In my opinion the match chosen in the stm hal is cleaner and better to understand:

macro_rules! gpio {
($GPIOX:ident, $gpiox:ident, $rcc_bit:expr, $PXx:ident, $extigpionr:expr, [
    $($PXi:ident: ($pxi:ident, $i:expr, $MODE:ty, $exticri:ident),)+
]) => {

this matches on something like this:

gpio!(GPIOA, gpioa, 0, PA, 0, [
PA0: (pa0, 0, Input<Floating>, exticr1),
PA1: (pa1, 1, Input<Floating>, exticr1),
PA2: (pa2, 2, Input<Floating>, exticr1),
PA3: (pa3, 3, Input<Floating>, exticr1),
PA4: (pa4, 4, Input<Floating>, exticr2),
PA5: (pa5, 5, Input<Floating>, exticr2),
PA6: (pa6, 6, Input<Floating>, exticr2),
PA7: (pa7, 7, Input<Floating>, exticr2),
PA8: (pa8, 8, Input<Floating>, exticr3),
PA9: (pa9, 9, Input<Floating>, exticr3),
PA10: (pa10, 10, Input<Floating>, exticr3),
PA11: (pa11, 11, Input<Floating>, exticr3),
PA12: (pa12, 12, Input<Floating>, exticr4),
PA13: (pa13, 13, Input<Floating>, exticr4),
PA14: (pa14, 14, Input<Floating>, exticr4),
PA15: (pa15, 15, Input<Floating>, exticr4),
]);

(sure needs adoptions to avr)

https://github.com/stm32-rs/stm32f4xx-hal/blob/4b04112d37bc14ed752d1c4cd6591950d9d26b2a/src/gpio.rs#L239-L242

fneddy avatar Dec 14 '20 11:12 fneddy

Such changes should probably be considered with the pending rework of the PORT HAL in #114.

Rahix avatar Dec 14 '20 12:12 Rahix