AdaCore / Ada_Drivers_Library

Ada source code and complete sample GNAT projects for selected bare-board platforms supported by GNAT.
BSD 3-Clause "New" or "Revised" License
241 stars 142 forks source link

Add support for use of VL53L0x heightfinder in Crazyflie flow deck v1 #342

Closed simonjwright closed 4 years ago

simonjwright commented 4 years ago

The Crazyflie flow deck v1 uses a VL53L0x "time of flight" sensor as a rangefinder to measure the aircraft’s height.

The VL53L0x can be set to generate an interrupt when a measurement is available, but according to this schematic the GPIO concerned (IO_2 on the left side expansion port) is multiplexed. The Crazyflie software handles this by setting the VL53L0x to continuous running, sampling when required. This change adds Start_Continuous to initiate continuous running.

Additionally, because the code to determine whether a measurement is actually available depends on the VL53L0x’s GPIO being configured to "generate an interrupt" when a new sample is available, preconditions are added to make sure that this is the case (if it isn’t, the software will loop indefinitely waiting for the sample).

simonjwright commented 4 years ago

The build checks failed because the microbit zfp doesn’t support Ada.Real_Time. There ought to be a way to tell the CI not to try that! I suppose I could make the offending subprogram into a generic which users have to instantiate with a procedure which is expected to either be null or to delay for a millisecond. But other code in the library uses Ada.Real_Time?

simonjwright commented 4 years ago

OK, just seen bosch_bno055.ads. I don’t know whether anyone is using the existing vl53l0x code, so will make just the offending subprogram (Read_Range_Single_Millimeters) generic, similarly.

Or I could revert this change and warn people about the spinloop? (my intended use doesn’t use this subprogram).

Fabien-Chouteau commented 4 years ago

Hi @simonjwright ,

Thanks for the contribution.

The build checks failed because the microbit zfp doesn’t support Ada.Real_Time. There ought to be a way to tell the CI not to try that!

The components drivers should be portable across any run-time profile. If you need timing features, the device should use an HAL.Time.Any_Delays implementation like here: https://github.com/AdaCore/Ada_Drivers_Library/blob/c958bb1d7fdf941b14b96eea61a78edb77216a10/components/src/screen/ST7735R/st7735r.ads#L53

If your run-time supports Ravenscar you can use this implementation: https://github.com/AdaCore/Ada_Drivers_Library/blob/c958bb1d7fdf941b14b96eea61a78edb77216a10/middleware/src/ravenscar-common/ravenscar_time.ads#L36

Otherwise it can be implemented by the BSP, like here for the micro:bit: https://github.com/AdaCore/Ada_Drivers_Library/blob/c958bb1d7fdf941b14b96eea61a78edb77216a10/boards/MicroBit/src/microbit-time.ads#L64

simonjwright commented 4 years ago

OK, just seen bosch_bno055.ads. I don’t know whether anyone is using the existing vl53l0x code, so will make just the offending subprogram (Read_Range_Single_Millimeters) generic, similarly.

I hope this will be OK ... as noted, there is a precedent!

Fabien-Chouteau commented 4 years ago

I don't like this approach because Ada's explicit generic instantiation make for an awkward API here. I would rather prefer a not null HAL.Time.Any_Delays argument in this procedure if you don't want to add it to the type discriminants.

simonjwright commented 4 years ago

I don't like this approach because Ada's explicit generic instantiation make for an awkward API here. I would rather prefer a not null HAL.Time.Any_Delays argument in this procedure if you don't want to add it to the type discriminants.

Decided to add to the type discriminants; might want to add other delays later.

Fabien-Chouteau commented 4 years ago

Thank you Simon!