Drivers Code Cleanup

[thomasgubler]

Goals:

• Remove code duplication
• remove drivers framework
• completely drop IOCTLs uORB replace all IOCTL usage #8891
• base class for each driver type (accel, baro, differential pressure, etc)
• support multiple instances of each driver drivers device instances #8813
  • use the Device library to keep track of all instances and deconflict
• drivers ModuleBase equivalent ModuleBase allow multiple instances of a module #9143
• port all nuttx drivers to linux and qurt Port mpu9250 to linux #8963, drivers unification #8315
• split CDev (system block device) from Device hierarchy drivers device interface improvements #8317 (partial step)
• run drivers in a work_queue per bus PX4 general work queue #8814
• split rc.sensors per board
  • think about board manifest board manifest ideas #8812
• generalize serial port configuration generalize serial port configuration #8302
• drivers should apply their own temperature compensation (not the sensors module)
• merge DriverFramework (DriverFramework merge with in tree PX4 drivers #10607)
• [please extend list]

[dagar]

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. #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.

[mcsauder]

I agree with the comments above.

[mcsauder]

@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

[dagar]

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

• Virtually all code used to manage the IMU drivers (start, stop, status, test, calibration) should be the same. Integration and publication should also be something common provided by the base class.
• All IOCTL code except for applying calibration and getting the device id could be gutted immediately. Eventually I want to get rid of it entirely (calibration).
• the current Ringbuffers aren't needed at all (CDev read)
• for the most part nearly all drivers should be independent of the physical interface (I2C or SPI). Other than the unnecessary use of CDev (eg /dev/bmp280_i2c_int) the way this is handled now is actually pretty good. I'd also like to see this extended so that simulation data can enter the driver at this level.
• properly hanlde multiple instances of each driver (eg this must stop https://github.com/PX4/Firmware/blob/master/src/drivers/imu/mpu9250/main.cpp#L121-L137)
• the split between mpu6000 and mpu9250 likely shouldn't exist, unless we get to a point where the actual specific driver code is so minimal it makes more sense to have separate mpu6000, mpu6500, icm20602, icm20689, mpu9250, etc
• calibration (a longer discussion)
• split CDev from the Device driver base class (I'll explain separately)
• depending on the calibration discussion we may not need CDev at all for drivers. At a minimum we definitely don't need a CDev per driver interface (eg /dev/mpu9250_accel1).
• NuttX SPI DMA can only be enabled across all SPI simultaneously and we might be short on DMA channels (especially with interest in DSHOT)
• synchronization of IMU. We want a synchronized IMU (accel + gyro) message for the EKF (easy with most sensors we support), but also have to support doing this with completely separate accel + gyro.
• PX4 side something like a high priority work queue per sensor bus is needed
• mechanism for syncing the entire rate pipeline with output (50-400 Hz PWM ESCs, oneshot, dshot, UAVCAN, etc). I want to the entire block (gyro sample -> rate controller -> output) synchronized with effectively all latency and jitter removed.
• support data ready interrupts
• regardless of the specific usage all drivers should build on all platforms. PX4 I2C and SPI already support the differences across NuttX, Linux, QuRT

Relevant issues/PRs

• Increase mpu6000/ mpu9250 rate and disable on-chip dlpf #7446 (in particular Increase mpu6000/ mpu9250 rate and disable on-chip dlpf #7446 (comment))
• WIP: High-rate IMU sampling #8731
• drivers device interface improvements #8317
• drivers device instances #8813

[mcsauder]

Distance sensor drivers are progressing, see #11891, #11853, #11859, et. al.

[dagar]

Believe it or not nearly all of this has been done.