Remove CRSF RSSI Inversion

[jlpoltrack]

EdgeTx handles this fine and shows negative RSSI - same values the driver provides.

Close to plane (-22 dB):

Further from plane (-38 dB):

[olliw42]

I'm a bit unsure on this one.

I believe I ahd spend some time on figuring out what one should send, and thus believe that I did it along what I have seen been done elsewhere, e.g. in the OpenTx code.

I think we need to be quite carefull here, and check for possible unintended side effects. The RSSI is potentially used in other places, scripts, widgets, etc, and we don't want to break them. Especially Yaapu needs to be checked. We probably also do not want to confuse users. We need to check the situation for OpenTx too.

[jlpoltrack]

Good points, here's a few notes back.

• Yaapu takes the absolute value of 1RSS so sign doesn't matter: https://github.com/yaapu/FrskyTelemetryScript/blob/master/OTX_ETX/c480x272/SD/WIDGETS/yaapu/main.lua#L2249
• I believe OpenTx will handle this as well based on this comment in ELRS: https://github.com/ExpressLRS/ExpressLRS/blob/master/src/src/tx_main.cpp#L157
• Can confirm that in ELRS the 1RSS value is negative

Here's Yaapu working on EdgeTx with this change, reports RSSI of 100 at short range:

Docs on what the RTP and RS fields contain: https://github.com/yaapu/FrskyTelemetryScript/wiki/Passthrough-over-CRSF-and-ExpressLRS#rssi-and-link-quality

[olliw42]

Can confirm that in ELRS the 1RSS value is negative

ah, really

hmhmhm I may not have analyzed the ELRS code well enough, but I guess the lines following https://github.com/ExpressLRS/ExpressLRS/blob/master/src/src/tx_main.cpp#L157, namely -(ls->uplink_RSSI_1) as well as that the uplink_RSSI_1 fields are uint8_t (https://github.com/ExpressLRS/ExpressLRS/blob/master/src/lib/CrsfProtocol/crsf_protocol.h#L334-L335) made me conclude that they are sending positive values.

also note that we too return uint8_t in the function

hmhmhm, what's going on here?

[jlpoltrack]

I think on ELRS it works like this:

• RSSI is signed (negative) from the driver (https://github.com/ExpressLRS/ExpressLRS/blob/master/src/lib/SX1280Driver/SX1280.cpp#L654)
• On the Rx side it is inverted (now positive) for BF/INAV: https://github.com/ExpressLRS/ExpressLRS/blob/master/src/src/rx_main.cpp#L282
• This (positive) value is sent over the air to the Tx
• The Tx code inverts it to a negative value
• EdgeTx (at least) can display the negative

Docs show that expected value is negative (confirmed on my hardware as well):

[olliw42]

so, for sending from the tx module to the radio, they are using a unit8_t field to represent the negative value, right? (uplink_RSSI_1/uplink_RSSI_2 in the crsfLinkStatistics_t struture are unit8_t)(https://github.com/ExpressLRS/ExpressLRS/blob/master/src/lib/CrsfProtocol/crsf_protocol.h#L334-L335)

isn't that strange that on the tx side it is send negative to the radio but on the rx side it is send positive to the flight controller ... how is the flight controller handling it?

your suggested change would also affect the rx side, doesn't it. That is, ArduPilot would now receive the rssi also negatively, what does that imply? I think ArduPilot wants to get it positive, https://github.com/ArduPilot/ardupilot/blob/40a3e076fc4f965d41f7c599ec7866a3192c3a63/libraries/AP_RCProtocol/AP_RCProtocol_CRSF.cpp#L513, and the comment suggests that this is what TBS is doing. Which I would take as the sepecifcation. What would happen if someone would have the crazy idea and connect a mLRS receiver to a betaflight controller? What would happen if it would be iNAV?

it seems to me that on the rx side it should remain positive

this rssi stuff is a can of worms

[jlpoltrack]

Yes, you're right - this breaks RSSI on the FC side.

My next commit just makes the change on the Tx | CRSF side for the 1RSS and TRSS elements (and leaves everything else alone).

One more note, 'rxrssi' seems to be part of the 'SRx_RC_CHAN' stream rate (when I had this set to 0 I never got rxrssi in the Status page:

[olliw42]

so, we are going to end up with having different things on each side ... do we btw know what would be displayed with a TBS system? I would wish not everyone would have just cooked up their own soup ... anyway, that's the situation, so we cope with it

I guess I would want it to be better seperated out and reflected, i.e., suggest to introduce two functions, uint8_t crsf_cvt_rssi_rx(int8_t rssi_i8) and uint8_t crsf_cvt_rssi_rx(int8_t rssi_i8), and call these apporpriately.

One more note, 'rxrssi' seems to be part of the 'SRx_RC_CHAN' stream rate (when I had this set to 0 I never got rxrssi in the Status page

yes. I think I had mentioned this recently elsewhere, I think it was in the context of sending radio status on the tx side. (that's why I don't need this radio status thing :))

[OptimusTi]

Values are supposed to be negative. Signal strength is in dBm. e.g. CRSF protocol sends a positive value, INAV has to invert it to display it correctly in the OSD. They key here is the dBm values for the signal strength.

[olliw42]

sure, the "real" values should be negative, the question is how they are represented as you say, my understanding too was that the CRSF protocol sends them positively, and thus are e.g. converted in ArduPilot. Thx for pointing out that this needs to be done also in iNAV. So, at least for the receiver side, we can firmly conclude it is represented positively in CRSF. From this I concluded that it would/should be also send positively on the transmitter side. I mean, the message definition should be equal on both sides, and it is uint8 aka positively on the receiver side ... So what actually should have happened is that, like in iNAV, it should have been converted to negative in OpenTx/EdgeTx ... which apparently is not done ... and here we are having to fool around to correct for previous fooling around ... I "love" this

[jlpoltrack]

so, we are going to end up with having different things on each side ...

Not quite, this commit only changes the 'raw' RSSI elements in dBM - 1RSS and TRSS.

The RSSI on the FC side uses the positive value that is provided from the Rx an an input to then produce a percentage number between 0 and 100. This same percentage value is also available on the Tx side - RRSP.

On Ardupilot, I don't believe it's possible to see/display the 'raw' RSSI value in dBM at all but INAV/BF do allow for this. As mentioned, the value positive value is inverted (example for INAV: https://github.com/iNavFlight/inav/blob/master/src/main/rx/crsf.c#L238)

The only other instances that I see the 'crsf_cvt_rssi' function being called in crsf_interface_tx.h are for elements that OpenTx/EdgeTx don't use - so I'm not sure of a uint8_t crsf_cvt_rssi_tx(int8_t rssi_i8) is really needed. Could just eliminate all references to the function in crsf_interface_tx.h to get the desired behavior.

[olliw42]

Not quite, this commit only changes the 'raw' RSSI elements in dBM - 1RSS and TRSS. The RSSI on the FC side uses the positive value that is provided from the Rx an an input to then produce a percentage number between 0 and 100. This same percentage value is also available on the Tx side - RRSP.

that's wzhat I was trying to say :)

On Ardupilot, I don't believe it's possible to see/display the 'raw' RSSI value in dBM at all

yes, you don't get it. They convert.

The only other instances that I see the 'crsf_cvt_rssi' function being called in crsf_interface_tx.h are for elements that OpenTx/EdgeTx don't use - so I'm not sure of a uint8_t crsf_cvt_rssi_tx(int8_t rssi_i8) is really needed. Could just eliminate all references to the function in crsf_interface_tx.h to get the desired behavior.

it's not about whether one would need it. One can do as you suggest and/or did. I just don't like the fancy conversions are done inside the code, and moreover somewhat intransparently since differently for tx and rx. So having two convert functions would make the last at least immediately obvious to anyone reaidng the code.

how to prgress? I pr and do the change, or you do the change in this PR? both is fine with me :)

[jlpoltrack]

how to prgress? I pr and do the change, or you do the change in this PR? both is fine with me :)

Probably best if you do the PR - I will look for and test out :) Will close this.

[olliw42]

@jlpoltrack I reopened and PRed. also to give you the credit :) maybe you can test the follow up to see if it still meets your expectation :) MANY THX!

[jlpoltrack]

@olliw42 All good, tested with this commit (Tx only) and looks good to me. :)

[olliw42]

MANY THX sir it's really helpfull