drivers/*: unify drivers params definition scheme#7937
drivers/*: unify drivers params definition scheme#7937aabadie wants to merge 9 commits intoRIOT-OS:masterfrom
Conversation
db0b16a to
1f10b80
Compare
|
Finally, I updated all drivers and their saul adaption when available. In the mean time, I also removed some |
150a91a to
f1120b8
Compare
|
Nice effort! I think most of the changes are very valid and its nice to see the style of parameter definition to be cleaned up. One think I don't like, that this PR is still using (slightly) different styles in the same context. For example looking at the #define AT86RF2XX_PARAMS { .spi = SPI_DEV(0), \
...
.reset_pin = GPIO_PIN(PORT_C, 1)}while the on-board sensors are configured using singular defines #define L3G4200D_I2C I2C_0
#define L3G4200D_ADDR 0x68
#define L3G4200D_DRDY GPIO_PIN(PORT_B,8)
#define L3G4200D_INT GPIO_PIN(PORT_B,11)So I would prefer to keep to one style. IMHO the singular defines are the nicest, as this allows us to use default values which we don't need to re-define. So taking the /**
* @name Define the interface for the L3G4200D gyroscope
* @{
*/
#define L3G4200D_PARAM_DRDY GPIO_PIN(PORT_B,8)
#define L3G4200D_PARAM_INT GPIO_PIN(PORT_B,11)
/** @} */The I2C dev and addr values then simply fallback to their default values The same would be true for the /**
* @name Define the interface to the AT86RF231 radio
*/
#define AT86RF2XX_PARAM_CS (GPIO_PIN(PORT_A, 1))
#define AT86RF2XX_PARAM_INT (GPIO_PIN(PORT_C, 2))
#define AT86RF2XX_PARAM_SLEEP (GPIO_PIN(PORT_A, 0))
#define AT86RF2XX_PARAM_RESET (GPIO_PIN(PORT_C, 1))Here again: SPI dev and clock speed use the default values... |
|
Thanks for having a look @haukepetersen.
I totally agree with this and will apply the required change. |
|
After looking at the board params (iotlab-m3, fox, etc), the actual status of this PR is leaving them broken. I'm working on fixing them... |
|
@haukepetersen, I'm nearly done with this one (at least the |
464623b to
087cc44
Compare
|
Ok, after some fight, CI is finally green here... but the diff has increased significantly. @gebart @haukepetersen, can you check that all mulle auto configured devices are still working ? It's ok on iotlab-m3. |
1b8b5b2 to
0b8a8ef
Compare
|
Maybe @smlng is also interested by this one ? |
smlng
left a comment
There was a problem hiding this comment.
tested with PhyNode (pba-d-01-kw2x): works.
Style-wise I don't like the superfluous use of parentheses around plain macros, like in #define AT86RF2XX_PARAM_CS (GPIO_PIN(PORT_A, 1)) or #define BTN0_PORT (PORTD).
They weren't there before and are not needed. I agree to have them to guard/enclose more complex expressions, but in general for instance the macro PORTD when resolved is already enclosed in parentheses. They don't hurt, but (IMHO) they do not enhance readability in any way either.
Nevertheless this would be non-blocking from my side.
I think I'm done but I'm not 100% sure. I admit I lost myself in all those branches and PRs...
But now I'm wondering what was the simplest. It will be a lot of work reviewing all that, right ? |
This PR is an attempt to address what was suggested in #7519 and especially the first step of this comment.
I started to update some of the drivers also available inauto_init/saulbut before going further, I'd like to know if I'm on the right way.