Spektrum SRXL2 support incl. telemetry

[andyp1per]

Full support for SRXL2 and Spektrum Telemetry when using SRXL2.

Protocol is half-duplex, non-inverted. Setup is identical to that for FrSky passthrough.
The protocol requires half-duplex and therefore will not work on the IOMCU or with soft serial.

Flight test on a KakuteF7 Mini with these options:

BRD_ALT_CONFIG=1
SERIAL6_PROTOCOL=23
SERIAL6_OPTIONS=12
RSSI_TYPE=3

[tridge]

This is far too much of a cut&paste PR. The SRXL code is C code in a global namespace, it doesn't following our coding style and includes heaps of code that just isn't relevant to us. The telem code is a cut&paste from the frsky telem code. I'd like to see the use of common code so we don't just grow the flash usage by a huge amount. The use of static functions and callbacks makes no sense to me as it is communicating within a class. The CRC is duplicated from an existing CRC.

[andyp1per]

@tridge I can factor out the common pieces of the telemetry code - not a problem.
As far as the srxl2.c library was concerned I was hoping in the spirit of opensource we could use this as a library and do a PR against https://github.com/SpektrumRC/SRXL2 for the changes that we deem necessary. The reason I think this is a good idea is because the protocol is relatively complicated and the implementation is provided by the manufacturer and is being used by betaflight - so we get the benefit of bugfixing and hardening which otherwise we have to do all over again. Given the relatively low priority that spektrum support appears to have in ArduPilot that also seems like a reasonable use of valuable developer time. So I guess my question would be are there any circumstances under which this would be acceptable to you as a reasonable approach? For instance:

• ifdef out unneeded parts of code
• change the global statics to parameters to functions / structures

of course I then need to have the conversation with HH as to whether they would accept changes of that nature, but a conversation worth having I think.
If we do it this way then we get the benefit of future bugfixes.
Here's a simple sketch:

typedef struct SrxlEngine 
{
    SrxlChannelData srxlChData;
    SrxlTelemetryData srxlTelemData;
    SrxlVtxData srxlVtxData;

    void (*srxlChangeBaudRate)(void* uart, uint32_t baudRate);
    void (*srxlSendOnUart)(void* uart, uint8_t* pBuffer, uint8_t length);
    void (*srxlFillTelemetry)(SrxlTelemetryData* pTelemetryData);
    void (*srxlReceivedChannelData)(SrxlChannelData* pChannelData, bool isFailsafe);
    bool (*srxlOnBind)(SrxlFullID device, SrxlBindData info);
#ifdef SRXL_INCLUDE_VTX_CODE
    void (*srxlOnVtx)(SrxlVtxData* pVtxData);
#endif
#ifdef SRXL_INCLUDE_FWD_PGM_CODE
    void (*srxlOnFwdPgm)(uint8_t* pData, uint8_t dataLength);
#endif
    void (*srxlEnterCriticalSection)(void);
    void (*srxlExitCriticalSection)(void);
    uint16_t (*srxlCrc16)(uint8_t* packet);
} SrxlEngine;

bool srxlParsePacket(SrxlEngine* engine, uint8_t busIndex, uint8_t *packet, uint8_t length);
void srxlRun(SrxlEngine* engine, uint8_t busIndex, int16_t timeoutDelta_ms);
bool srxlEnterBind(SrxlEngine* engine, uint8_t bindType, bool broadcast);

[andyp1per]

@tridge is b89610f what you had in mind?

[peterbarker]

On Fri, 3 Apr 2020, Andy Piper wrote: https://github.com/SpektrumRC/SRXL2 for the changes that we deem necessary. The reason I think this is a good idea is because the protocol is relatively

Any reason not to use a submodule?

[andyp1per]

No reason, except I thought we changed this for CMSIS. Happy to change to a submodule if that helps? andy

…

On 03/04/2020 23:51, Peter Barker wrote: On Fri, 3 Apr 2020, Andy Piper wrote: > https://github.com/SpektrumRC/SRXL2 for the changes that we deem necessary. The reason I think this is a good idea is because the protocol is relatively Any reason not to use a submodule? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#13923 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWCLTBPYDYOXT2GTSTC2DDRKZR5XANCNFSM4LV7ZEDQ>.

[andyp1per]

I did not realize rcinput ran in a separate thread. I see I have made a big error here by removing the semaphores. I will put them back. It's easily possible that this is the cause of my lockups.

[andyp1per]

Ok refactored into AP_Telemetry to reduce duplication. I think I have also fixed the lockups which were a combination of semaphores and having the wrong options set on the outgoing uart. I have also fixed data rates based on info from HH and the result is much more stable.

[andyp1per]

Just in case I didn't make this clear @tridge and @peterbarker spm_srxl.c and spm_srxl.h are identical to the ones in the Spektrum github project and is also the code they run in their hardware, so we really want to adopt it with as few changes as possible if at all possible.

[peterbarker]

On Sat, 4 Apr 2020, Andy Piper wrote: Ok refactored into AP_Telemetry to reduce duplication. I think I have also fixed the lockups which were a combination of semaphores and having the wrong

Something less generic than AP_Telemetry, perhaps? AP_RC_Telemetry would be better, but perhaps there is something more specific still?

[andyp1per]

AP_RC_Telemetry would be better, but perhaps there is something more specific still?

Happy to go with whatever. Will use AP_RC_Telemetry unless you can tell me a better name!

[tridge]

@andyp1per I like the changes you've made to use a common AP_Telemetry library, that helps a lot.
I would prefer the spm_srxl to be built as C++ to benefit from the extra compiler checks. I've done a branch here that makes that change:
https://github.com/tridge/ardupilot/commits/pr-srxl2
I'd like to discuss those changes with you. I know it means the code is not identical to the upstream code, but perhaps some of the changes would be accepted, otherwise we could maintain some patches against the upstream.
It still seems to use it's own crc16? It would be nice to avoid that. We could use one of the following options:

• #ifdef for whether to use an existing crc16
• support using the hardware crc on stm32

[andyp1per]

@tridge I have pulled in the necessary changes from your branch with a couple of slight modifications:

• PACKED is required as only the structures that go on the wire are defined as packed
• It's going to be hard to persuade them to drop the C linkage support, so instead I defined the included functions as having C++ linkage. I think this gives you what you want - compiling under C++ and not modifying crc.h without preventing their code from working as a C library in other C++ environments.

I have done a PR with these changes SpektrumRC/SRXL2#1

I've also made the packaging modifications you requested and the naming changes that @yaapu requested.

It turns out that the UART changes I made for non-blocking/no-rtscts have made the bootup hang go away so I have removed that code.

I have done the timing calculation and reduced the outbound budget to 2ms.

Turns out we prevent DMA in half-duplex in ChibiOS so I have not tried to use 400k baud

[yaapu]

@andyp1per you missed PR #13846, you need to rebase

[andyp1per]

@yaapu done - I don't have hardware to test - are you able to check I haven't broken anything?

[yaapu]

@andyp1per sure, I can test both sport and fport, for frsky telemetry there's also a PR #13850 to test basic functionality in Tools/autotest

[yaapu]

@andyp1per may I ask why did you remove the AP_Specktrum_Telem subtree, having both AP_RCTelemetry.* and AP_Specktrum_Telem.* in the same path it's not a clean layout IMO

[tridge]

now we have @sys in master I'd like to see how much free stack we have left with SRXL2 running

[andyp1per]

before the protocol is enabled:

rcin          PRI=177 STACK_LEFT=488

after

rcin          PRI=177 STACK_LEFT=296

[kruftindustries]

Built ArduPilot for target mRO X2.1-777 both with andyp1per:pr-srxl2 and master after these changes were merged and cannot seem to get rcinput or telemetry to work with the SPM4651T. Sliders on Required Input>>Calibrate Radio are either blank or stationary (infrequently show anything when trying things but don't change)
Bind button on the receiver works but no other confirmation the thing is even connected.
Steps to reproduce:
SPM4651T signal pin connected to Telem1 Tx pin, +5V and GND

Build command: "c:/cygwin64/bin/python3.6m waf" (uavcan incorrect syntax error with python2.7)
Target: "configure --board mRoX21-777"

Dont think this is the problem but tried the following
ardurover.bin
arducopter.bin

Board settings have been combinations of
SERIAL1_PROTOCOL=23
SERIAL1_OPTIONS=4,8,12(Also tried variations of TX pull-up, tx invert and others)
RSSI_TYPE=0,3
SERIAL1_PROTOCOL=57,115,400(not sure 400 would work or is a valid option)

Any ideas?

Edit: Also tried setting BRD_SER1_RTSCTS=0
Some specifics found while reviewing datasheets
SRXl2 bus is 3.3v idle, active low
Telem1 bus is described as Serial1 but connected to UART2 on the STM32F7
220ohm resistors between STM32 and connector pins

Have not reviewed the software but depending on implementation there might be a hardware problem? The handful of notes found on the subject involve a max3232 as a buffer but only saw mention of the TX pin used for connection

If it is hardware, the STM32 should support open drain so could just connect RX and TX together to the SPM4651T signal pin, if open drain isn't configured with the software a possible solution would be connect SPM4651T signal to RX and drive a pull-down transistor with the TX pin(invert in settings?). Might also need a pull-up resistor but the SPM4651T should have a high value pull up already

[andyp1per]

Sounds like you are doing the right things. I have this working on a Kakute F7 Mini and a Pixracer (F4). On the F7 I've swapped TX/RX (so option 12), on the F4 I am connected to the TX pin (so options 4). I've not tried that particular RX. There have been some firware updates, I guess you might want to try that first. It might be worth putting some debug in AP_RCProtocol.cpp:216 to see if you are able to read any data at all.

[andyp1per]

Maybe also try disabling DMA on the port. I think we do this anyway for half-duplex, but worth a shot

[andyp1per]

Baud should be 115

[kruftindustries]

Switched firmware again and it is now working. The pop-up menus and text descriptions are missing on the full parameter list but here are the settings in case someone else runs into this

SERIAL1_BAUD=115
SERIAL1_OPTIONS=4
SERIAL1_PROTOCOL=23

[kruftindustries]

There is a problem. I haven't quite figured out why yet but communication is only intermittent. I put a scope on the data lines, levels seem okay and I see lots of 0xA6 0x21 packets when the receiver light is illuminated then after some time the light begins flashing irregularly before staying off completely and the majority of packets are something like 0x05 0x80. The receiver seems to quit talking.

I did also notice some voltage sag going to the receiver while it is transmitting and pk-pk current draw is around 50-90ma. It does not seem to be the supply to the FC, but could be because it is powered via USB. I'll try powering externally this evening but is a controller address 0x05 normal?

[andyp1per]

For me, USB power was unreliable I had to power from the battery to get any kind of telemetry.

[kruftindustries]

Yes, it was power. Have to connect an external supply to the receiver power pin. Connecting to the Vin on the FC does not seem to do the trick so this guy must be a bit hard on the peripheral supply.

Thanks again!

[andyp1per]

Interesting! I think I might change from 3.3v Vdd to something a little more beefy

[NitinJSanket]

Doesn't work with Kakute F7 AIO when connected either to SERIAL6 or SERIAL4. I flashed the 4.1.0dev firmware (master trunk) and set the following params:

BRD_ALT_CONFIG=1
SERIAL6_PROTOCOL=23
SERIAL6_OPTIONS=12
RSSI_TYPE=3

Anything I am missing?
Powered using 5V line on the AIO board (tried powering both from battery and USB).
I dont see the RC values on Mission Planner.

[andyp1per]

@NitinJSanket on the AIO you have both RX and TX pins on RX6 - are you connecting to the RX pin? It should be:
If using RX pin: SERIAL6_OPTIONS=12
If using TX pin: SERIAL6_OPTIONS=4
If that doesn't work can you try building with debug on in the SRXL2 module?

[NitinJSanket]

Works like a charm, I think I had messed-up the SERIAL6_OPTIONS with the TX/RX pin combo.