Closed thomasgubler closed 4 years ago
There's quite a lot that could be discussed here. For me the worst part is that most of the drivers appear to have been developed largely by copy and paste. New drivers are added by duplicating something close, then making a few changes. Huge portions of near boilerplate code are duplicated multiple times. This has resulted in a large sprawling code base that's difficult to maintain and uses significantly more flash than is necessary.
As an illustration here's a relatively simple pull request that changed the way raw/unfiltered IMU was published. https://github.com/PX4/Firmware/pull/8517 Every single accel and gyro driver in the official repo does the same filtering, integration, scaling, publication, etc. This could easily be pushed into a base class (PX4Accel, PX4Gyro) instead of making the same identical change many times.
I agree with the comments above.
@thomasgubler , @dagar , We are working on a basic driver parent class to begin unifying driver construction and are interested in your input. Recent work on the lis3mdl driver and current work on the batt_smbus and mpu9250 driver represents some of the structure we have implemented, however, there might still be better architectures we haven't identified. If you have opinions you would like to see implemented please let me know.
-Mark
I'm excited to see interest here. From a code maintenance, platform scalability, and developer sanity standpoint I think abstracting the common pieces of each driver type is long overdue. As a start take a look at the Airspeed class used by all differential pressure sensor drivers. https://github.com/PX4/Firmware/tree/master/src/lib/drivers/airspeed
As much as I love to eliminate code bloat and make things as simple as possible, the most valuable changes we need really are in the sensor pipeline. In terms of system capabilities I view this as an opportunity to implement some of the core improvements that are particularly important for racers and system integrators right now. The most important is splitting the IMU drivers into a high rate publication path to the rate controller (400 Hz - 2 kHz), and a separate delta angle and delta velocity publication (250 Hz) for the EKF. We also need to increase the sampling rate as much as possible (>= 4 kHz) and do all of the filtering PX4 side to help with aliasing and clipping. This will require getting the drivers out of ISRs and enabling SPI DMA.
Notes
Relevant issues/PRs
Distance sensor drivers are progressing, see #11891, #11853, #11859, et. al.
Believe it or not nearly all of this has been done.
Goals: