japaric / stm32f103xx-hal

HAL for the STM32F103xx family of microcontrollers
Apache License 2.0
115 stars 40 forks source link

Less Macros more Traits? #78

Open rudihorn opened 6 years ago

rudihorn commented 6 years ago

A lot of the STM32 library is very dependent on macros, and I think that a lot of it could probably be avoided by making better use of traits.

I had been playing around with writing my own (not perfect) library, which may be a bit of an example of how some of it could be done: https://github.com/rudihorn/tslib/blob/master/src/gpio.rs. While macros are a nice way to simplify segments of code, I feel like wrapping large segments of code in them is somehow not ideal. Having some computation within such code is also fine as it usually ends up being easily optimized away in any case, especially with in-lining. Thoughts?

therealprof commented 6 years ago

@rudihorn I feel similarly that the macro usage is way too excessive however I don't think your code is optimal either due to the heavy use of generic types and unsafe code. Some middle ground would be nice, i.e. break out the pin handling into generic portions which the pins themselves having enough information so that methods implemented on them can handle autonomously, break out all the pins from the GPIO interface pretty much like you're doing while using macros to instantiate all available pins.

rudihorn commented 6 years ago

@therealprof I guess handling individual pins may not have been the best example, given that the only safe way to handle most of the pin functions is by calling the individual function names (maybe there is a better way to handle this from the svd2rust side? I'm not too familiar with this). Other modules such as the serial module might be better examples of where it is easier.

I think if I understand you correctly it would be to have non macroized Gpio modules, but then use macros for the pins?

I wanted to try seeing if its possible to define a rule such as set_mode as follows:

macro_rules! per_pin {
    ($pin : ident, $reg : ident, $modeop : ident) => {
        impl<'a, G, M, C> GpioPin<'a, G, $pin, M, C,>
        where G: GPIO, M: PinMode, C: PinCnf {
            fn set_mode_val_new(&self, value : u8) {
                (self.0).$reg.modify(|_,w| w.$modeop().bits(value));
            }
        }
    };
}

per_pin!(Pin0, crl, mode0);

But it seems to be that you can't then refer to the method set_mode_val_new in the generic context.

Edit: In my (incomplete) version of the serial (https://github.com/rudihorn/tslib/blob/master/src/usart.rs) I wanted to keep the "modules" separate, and then rather just take "proof" of e.g. afio being set to remapped to produce "proof" of the usart pins being set up, which can then be passed to the init function. I'm guessing most of the rest of the module should also be fine without macros.

therealprof commented 6 years ago

@rudihorn I haven't thought in-depth about those topics but recently starting on a STM32F767ZI implementation got me annoyed very quickly since the macro part is unwieldy while at the same time the GPIO pin definition for all 108 pins very repetitive and also error prone. I might revisit this part with a few ideas of yours at some point...

The worst thing about macros (especially if you instantiate them many times and when their expansion is huge) is that my tools are pretty much worthless helping me with the code while at the same time if there's an error in it, I'll be spammed (after a loooong processing time, too) with dozens of identical error messages preventing from seeing forest for the trees. That annoyance alone sometimes makes me actually prefer duplicating the code instead of using macros which of course is totally impractical for the the GPIOs...

rudihorn commented 6 years ago

I've only just recently become aware of the embedded_hal standardization, so I might just continue to play with my library to see if it works with embedded_hal.