SPI Full Duplex

[andyp1per]

This PR makes the use of the full duplex SPI API with DMA optimisation explicit to avoid contortions in expected behaviour by the underlying HALs and in the caller.

As a side effect this fixes a bug in the Linux HAL where the original implict semantics did not work.

[tpwrules]

Quite happy to see this! I can follow up to touch up comments and drivers.

I assume you've evaluated all the uses of transfer to make sure this semantic quirk isn't used anywhere else? It looks like some drivers could make use of the three-argument form of transfer_fullduplex as well.

[tpwrules]

Good news is this works and flies great on the Stampfly, which has a BMI270. I previously had to update the ESP32 HAL to account for the quirk and make that IMU actually work.

But I think I found one more place that needs changing to transfer_fullduplex:

ardupilot/libraries/AP_OSD/AP_OSD_MAX7456.cpp

Line 205 in dfbbef4

_dev->transfer(buffer, buffer_offset, buffer, buffer_offset);

[andyp1per]

Tested on Matek H7A3

[tpwrules]

I think we do need to somehow deprecate the old semantics in ChibiOS. I worry a bit about upstreaming broken drivers.

[tridge]

this is a copy of the one in Device.h, why is this here?

[tpwrules]

Removed, hopefully it's not necessary for Linux or something :)

[andyp1per]

It can't be removed or loads of hals do not build

[tridge]

which HAL doesn't build without the transfer_fullduplex() in AP_HAL/SPIDevice.h?

[tpwrules]

QURT at least had a build failure on CI. It seems to be a Clang warning that you have to override all the variations with different arguments of a particular function name to avoid ambiguity: https://stackoverflow.com/a/9995567

 [10/933] Compiling libraries/AC_AttitudeControl/AC_CommandModel.cpp
In file included from ../../libraries/AC_AttitudeControl/AC_CommandModel.cpp:1:
In file included from ../../libraries/AC_AttitudeControl/AC_CommandModel.h:6:
In file included from ../../libraries/AP_Param/AP_Param.h:27:
In file included from ../../libraries/AP_HAL/AP_HAL.h:8:
In file included from ../../libraries/AP_HAL/AP_HAL_Main.h:19:
In file included from ../../libraries/AP_HAL/HAL.h:11:
../../libraries/AP_HAL/SPIDevice.h:45:18: fatal error: 'AP_HAL::SPIDevice::transfer_fullduplex' hides overloaded virtual function [-Woverloaded-virtual]
    virtual bool transfer_fullduplex(const uint8_t *send, uint8_t *recv,
                 ^
../../libraries/AP_HAL/Device.h:137:18: note: hidden overloaded virtual function 'AP_HAL::Device::transfer_fullduplex' declared here: different number of parameters (2 vs 3)
    virtual bool transfer_fullduplex(uint8_t *send_recv, uint32_t len) {
                 ^
1 error generated.

I personally would just mark it as pure virtual = 0 instead of duplicating the implementation. More C++ learning!

[tpwrules]

Tested again and it works on the bench on ESP32 and Cube Orange. Removed the redundant function. Should be ready for merge!

[andyp1per]

@tpwrules the function you removed is necessary to make everything build correctly - I spent quite a long time fixing this - please don't make changes to other people's PR's unless you know it's correct!