Drivers: SPI & I2C config simplifications (2) #14386
Conversation
Needed to distinguish runtime instance types of the same driver (e.g. bmi055 accel vs gyro).
bkueng
force-pushed
the
spi_i2c_config_refactoring_drivers_1
branch
2 times, most recently
from
2d2ed7f
to
ec2d13a
Compare
For SPI/I2C driver default options
To run a specific method on a work queue and wait for it to return.
Use it for custom methods as well (like reset), and run by default on the work queue, since they typically access the bus.
instead of a static per-driver array. Reduces BSS RAM usage by a couple of 100 Bytes (linear increase with num drivers). Downsides: - a bit more runtime overhead - less isolation, locking required - a bit more complex
Also: remove device type auto-detection as it will not work with together with the new SPI board config (which specifies a specific device type)
- there was almost nothing shared - it will fit better into the updated I2C driver structure
and increase update rate to 20Hz
Removes the calibration on startup, as these values were overwritten by the system calibration values anyway. So the only difference is that if all calibration scales were equal to 1, the driver startup would have failed.
bkueng
force-pushed
the
spi_i2c_config_refactoring_drivers_1
branch
from
ec2d13a
to
8d0d015
Compare
dagar
added
Drivers 🔧
| int custom1{0}; ///< driver-specific custom argument | ||
| int custom2{0}; ///< driver-specific custom argument | ||
| void *custom_data{nullptr}; ///< driver-specific custom argument | ||
|
|
||
| // driver defaults, if not specified via CLI | ||
| int default_spi_frequency{20 * 1000 * 1000}; ///< default spi bus frequency [Hz] |
There was a problem hiding this comment.
1 MHz SPI and 100 kHz I2C might be safer defaults
There was a problem hiding this comment.
What is the ripple effect if we inadvertently set lower frequencies without noticing on a bunch of boards? I've been there, done that and people didn't realize it until very late into the investigation. 20 MHz is high, but pretty much any SPI device takes 10 MHz.
There was a problem hiding this comment.
I was going for what most drivers do, so that we don't have the problems Lorenz mentions.
We can also have no defaults and enforce all drivers to have to set their values.
Any preferences?
There was a problem hiding this comment.
At the moment forcing all drivers their value seems appropriate.
These days we have quite a few sensors that operate below 10 MHz. https://docs.google.com/spreadsheets/d/1OUOSr8TDeFwcNdao5b00iQvMUCQIgpeUJf85VwGpIC0/edit#gid=0
Not a lot of drivers use the global default, which is somewhat arbitrary.
|
Tested on pixhawk4 v4 f-450 Modes Tested: Log: |
Tested on NXP FMUK66 v3Indoor test flight.
Procedure
Note
Log |
|
Tested on pixhawk4 mini v5 f-500 Modes Tested Stabilized Mode: Good. Log: |
|
Flight logs look good and I did a pass on the test rack and everything is equivalent or better (pixracer internal lis3mdl start was broken in master). Great work on this, let's keep going! |
Follow-up to #14221.
This converts a large set of the drivers to use the new SPI and I2C board configuration. Mostly missing are mag's and imu's.
General
instantiatemethod.Testing
Tested on:
@UAV-Pilot this will conflict with #14168 and I was hoping to get yours in first.
@dagar since you will probably have the fun to review this, let me know if you want to bring this in steps, and which step size you prefer.