New FrSkyX protocol

petsmith wrote: Added some logging into the code to dump the pkt[6]. <snip>

We really need a way to identify invalid packet and not to process them, so as not to corrupt the telemetry display. Do we know the checksum algorithm of the telemetry packet?

Nice work! Were you logging out the serial port or another method?

The last two bytes of the telemetry packet are not a checksum. They're the receiver RSSI and LQI values appended when APPEND_STATUS in PKTCTRL1 is set. Unfortunately, unlike D8, the D16 protocol does not enable the cc2500 CRC option, possibly due to a bug noted in the errata. (Might be able to improve D8 performance by implementing the workaround for that bug).

For your fix I recommend just skipping the sport parsing if the length test fails. It's unlikely the bytes would have valid data.

If we could fully figure out the forward/backward sequence number algorithm we could skip the sport processing on packets with sequence errors. Might be worth trying even with the current implementation to see if it improves extended telemetry performance.

hi all,
thanks petsmith, i just test your version 96d38b0 for devo8s and in 20minutes i don't have any dropout at all in my house so tomorow i'll test outside and test if i can have the same range of a d4rii. i use an xsr receiver...

for information with the test build 2704370 i still have dropout less but was still here...

hexfet wrote: Were you logging out the serial port or another method?

Yeah, I used the serial port for logging. However, the extra time in performing the serial logging will also result in dropout. Thus, you've to be selective to choose when to log. Example, only log data when pkt[6] is larger than 100.

For your fix I recommend just skipping the sport parsing if the length test fails. It's unlikely the bytes would have valid data.

The reason I didn't skip the packet is that I believe most of the time only a few bits corrupt, the actual stream data my still be valid (besides the pkt[6] length byte). Anyway, I guess skipping the packet all together will probably be more safe. Feel free to apply any fix as you deem fit.

The last two bytes of the telemetry packet are not a checksum. They're the receiver RSSI and LQI values appended when APPEND_STATUS in PKTCTRL1 is set. Unfortunately, unlike D8, the D16 protocol does not enable the cc2500 CRC option, possibly due to a bug noted in the errata. (Might be able to improve D8 performance by implementing the workaround for that bug).

If we could fully figure out the forward/backward sequence number algorithm we could skip the sport processing on packets with sequence errors. Might be worth trying even with the current implementation to see if it improves extended telemetry performance.

Thanks for the info. If there are no CRC/checksum in the telemetry packet, how can the Frsky Transmitter detects corrupting data? It's unreliable to just rely on the sequence numbering. It's possible the sequence number in the packet is valid, only the actual sport data corrupt. Do we know how Frsky handles it?

lamnesique wrote: hi all,
thanks petsmith, i just test your version 96d38b0 for devo8s and in 20minutes i don't have any dropout at all in my house so tomorow i'll test outside and test if i can have the same range of a d4rii. i use an xsr receiver...

for information with the test build 2704370 i still have dropout less but was still here...

Thanks for helping to test. I've just flown 5 batteries in the field without any dropouts. Although I only flew within 100m (FPV racing), it will be great if you can help to test longer range.

Great news guys, I have on X series receiver, but not yet flown with it with my u7e, good that the culprit seems to be found......

nice work guys

i did an outdoor test this afternoon, and it was a very good success....as you can see i go for nearly 250m at around 3m altitude without any problems (i'm in 25mw fpv) my devo8s is set at 100mw

@lamnesique, great news! Thanks for testing.

@hexfet, do you think we can we apply the fix to the nightly build? If so, please help to make a pull request. Below is the code snippet with your suggestions added to skip packets with invalid length byte. I've tested this version and it's working fine. Feel free to make any amendments. Thanks again for bringing the FrskyX to us.

Great work with the frSkyx drop out fix,is there still a drop out issue with the regular frSky protocol for the d series rx?

The frskyx_drop test build and branch are updated.
- Add petsmith's check for sport telemetry length validity
- Change freq synth calibration to latest DIY-Multiprotocol method, and revert state machine to match
- Add checksum validation for telemetry packet

Something bugged me about this comment - that guy's not always right Looked at the spi captures again and there is a checksum on the telemetry packet in the two bytes before the RSSI and LQI. Same algorithm used for the transmit packets.

The freq synth calibration change saves some ram bytes, but essentially works as before.

If this works well I'll make a pull request based on this branch.

hexfet wrote: The frskyx_drop test build and branch are updated.
- Add petsmith's check for sport telemetry length validity
- Change freq synth calibration to latest DIY-Multiprotocol method, and revert state machine to match
- Add checksum validation for telemetry packet

Something bugged me about this comment - that guy's not always right Looked at the spi captures again and there is a checksum on the telemetry packet in the two bytes before the RSSI and LQI. Same algorithm used for the transmit packets.

The freq synth calibration change saves some ram bytes, but essentially works as before.

If this works well I'll make a pull request based on this branch.

i just try the new drop test build and it don't work at all....the problem is that it don't bind at all....devo8s for me

pkt[6[ cannot take values more than 6.
While is good checking for pkt[6]max value the len value is 17 si pkt[6]<=(17-7) is a little too large.
You have already a check for frame length and checksum on the frame so len>7 check is also not too much useful.

midelic wrote: pkt[6[ cannot take values more than 6.

Agree that in the spi captures the telemetry packets are always the same length and pkt[6] is always 0, 3, or 6. Still, there doesn't seem to be anything in the protocol that requires that. Some future receiver could return larger packets if the x-series transmitter modules were written to handle it, though that seems unlikely.

It also seems possible the large values of pkt[6] could be because the upper bits are used as flags of some sort that were never seen in the spi captures...maybe?

The length check also protects against a buggy/malicious receiver sending incorrect pkt[6] value in telemetry with good crc.

lamnesique wrote: i just try the new drop test build and it don't work at all....the problem is that it don't bind at all....devo8s for me

Thanks for testing! Missed initializing the calibration. Decided to revert back to the current nightly and just add the telemetry fixes. I'll leave the other changes til I have equipment to test with.

The test build is updated to match the current nightly, plus the telemetry crc and length checks.

You have already lengths check pkt[0] +3 other wise frame is discarded,SO the lenght is related with pkt[0].On top of hat you have check sum. Too many things must go wrong to go wrong
So the len is not the problem,,Erroneous pkt[6] may come from Sport data passing from RX ,AFAIK the sport RX is passed transparently when counting valid SPORT bytes.So erroneous pkt[6] may appear valid.
The pkt[6] cannot be more than 6 the number of SPORT bytes in the frame,otherwise the length is not pk[0]+3 the frame is longer which cannot be,The frame length is fixed in registry in PKTLEN.
Up to you it will still working like this anyway.

hexfet wrote:

lamnesique wrote: i just try the new drop test build and it don't work at all....the problem is that it don't bind at all....devo8s for me

Thanks for testing! Missed initializing the calibration. Decided to revert back to the current nightly and just add the telemetry fixes. I'll leave the other changes til I have equipment to test with.

The test build is updated to match the current nightly, plus the telemetry crc and length checks.

hi, i just test the be982ef build and the bind is ok but telemetry don't work at all, with the 96d38b0 test the telemetry "work" i had rssi volt 1 and volta correct, volta sometimes actualise an hazardous value.

@hexfet, the reason we couldn't get any telemetry in the last build is 'cos you're including the 2 crc bytes in calculating the checksum.
should be like to this, "len-5" needs to be changed to "len-7"
Making changes without the hardware to test is very difficult.

Another note, I agree with hexfet, the length check should still be there as the last safeguard. There will be some cases that will pass all the above check, including the newly added CRC check. How do I know? With previous versions that had dropout issues, some corrupted packets passed all initial checks. At that time, we didn't have the CRC check yet. Now, we've added CRC check. Some corrupted packets will still pass this check. I had observed some corrupted packets with len = 7. These corrupted short packets will pass the current CRC check easily as the CRC value for them are 0.
When len = 7, the current CRC function will always return 0 because we ask it to calculate the CRC for 0 byte of data. Now, for the pkt[7-4] and pkt [7-3] to be both 0 in the corrupted data is not difficult. In this case, it will pass all these checks and be treated as a valid telemetry packet.

I will suggest we added another check in the beginning, like the following.
The PACKET_MIN_SIZE should be the minimum valid packet size, which I expect it to be larger than 7.

Cannot pass what you say because they are too many checks.
because the pkt[0]= len -3 and is checked.ALso is checked,the txid pkt[1],and pkt[2]
You said len=7 but cannot be for the same reason pkt[0] is sent by RX as first packet and is always 0x0E=len-3 .So all is checked included the length.If len=7 cannot pass because of pkt[0].
You cannot take bits and pieces from all around.
Imo all checks are in place as it should be, except the checksum that is not needed.The check sum is passed from Taranis openTX PXX protocol so it is native there not from Frsky X protocol,I think everything will work just fine even without checksum bytes or checks on them.

But I agree that pkt'6]>6 may be the reason for dropouts ,the counting sport valid bytes from RX may not be accurate.
There is no packet minimum size should be fixed always 0x0e+2 bytesRSSI/LQI+1=len.Read CC2500 data sheet about the structure of payload.
The reason of not having telemetry I think is that checksum .check.

See here the structure of telemetry frame.

midelic wrote: Cannot pass what you say because they are too many checks.
because the pkt[0]= len -3 and is checked.ALso is checked,the txid pkt[1],and pkt[2]
You said len=7 but cannot be for the same reason pkt[0] is sent by RX as first packet and is always 0x0E=len-3 .So all is checked included the length.If len=7 cannot pass because of pkt[0].
You cannot take bits and pieces from all around.

I'm sorry to say, but you're wrong! I didn't make up len=7. It came up from the logs. Why is it so hard for you to believe others. If all these checks were as good as you said, I wouldn't be here trying to solve the dropout issue. I respect you as a developer, but I hope you can do the same for others. I didn't make up anything I said. They're all from my testing & logs.

Imo all checks are in place as it should be, except the checksum that is not needed.The check sum is passed from Taranis openTX PXX protocol so it is native there not from Frsky X protocol,I think everything will work just fine even without checksum bytes or checks on them.

Sorry to say, you're wrong again. From my testing with the checksum added, I haven't seen any corrupted packets passing through. If they were useless as you said, I should still have corrupted packets. I'm not familiar with Frsky protcol, so I won't argue with you where the checksum are originally from. However, from my logs, the checksum had blocked all the corrupted packets so far

There is no packet minimum size should be fixed always 0x0e+2

The reason I said packet minimum size was because I don't know the correct size of the packet or even if they're fixed or variable length. If they're fixed in size, then packet length check should be against whatever that size is.

I'm not wrong the arithmetic is there.If passing maybe you mess up with something ,.....but should not pass.If passing you should check why is passing not add another check.The len MUST be 17,... not >7.
On multiprotocol we are using
if(pkt[1] == rx_tx_addr[3] && pkt[2] == rx_tx_addr[2] && len ==(pkt[0] + 3))

the len MUST be pkt[0]+3 ; this is not something to choose if I or you want it or not.Again read CC2500 data sheet.

And we test the FrskyX and did not see any dropouts.Maybe we find if we test longer time.
It is not my invention the packet length is variable configured by the
PKTCTRL0 register and take the value of the first byte after sync word and guess what,..... the value is pkt[0]=0x0E.
So the check pkt[0]= len -3 is not from me ,from my developing skills and me trying to show you wrong......... is from CC2500 setiings.

Guys do whatever you feel is good.I tried to help ..but I'm out now ,do whatever you want.