Update RSSI_DBM to include diversity antenna in OSD element (CRSF)

[haslinghuis]

Fixes: #12358

[JyeSmith]

Antenna is 0 indexed e.g. Ant 1 = 0.

I haven't tested, but will this only show for Ant 1?

[haslinghuis]

Indeed if (ant 1 = 0) antenna will fail to the else clause otherwise include the antenna (ant 2 = 1)

[JyeSmith]

This may either cause constant blinking of (2), or nothing to be shown and cause confusion to user if the element is working.

[haslinghuis]

Awaiting test results of that (as it should not affect non diversity receivers) - don't have one to test.
The code only adds (1) to RSNR when the second antenna is active.

EDIT: Just changed it by adding + should work less confusing and without additional configuration?

[haslinghuis]

@LoakAudio adding + to the element indicating using second antenna will have the least impact.

[blckmn]

AUTOMERGE: (FAIL)

• github identifies PR as mergeable -> FAIL
• assigned to a milestone -> PASS
• cooling off period lapsed -> PASS
• commit count less or equal to three -> PASS
• Don't merge label NOT found -> PASS
• at least one RN: label found -> PASS
• Tested label found -> FAIL
• assigned to an approver -> PASS
• approver count at least three -> FAIL

[LoakAudio]

Is it possible to have an "offset" of +1 for the antenna number, to show 1 or 2 as it is labeled on the rx itself instead of 0 or 1 ?

[ZzyzxTek]

I just did some testing on this.

With a full radio diversity RX, it's kinda hard to fully test. It will use packets from the first radio IRQ that gets a good one, and only falls back on the second if the first packet doesn't get one or gets a bad packet. So what that means is it pretty much sits on one radio until things go bad.

In my tests, it fortunately started using the second radio (ant 1), I guess that IRQ fires first. So I immediately saw the + after the RSNR element, yay! I managed to see the + blink off a few times when I obscured that antenna on my quad, so pretty sure it displays nothing after the RSNR when it switches to the first radio (ant 0). But ELRS is good enough that it's hard to make it fail to get a good packet on the bench!

I think I would have to start a DVR recording and walk downstairs and out into the garage to really see it switch radios for more than a packet or two. If you'd like that level of evidence, I could set that up later today or tomorrow and post the DVR. But based on what I mentioned above, I think this works as intended.

[LoakAudio]

Unfortunately I can't test as my FC died rebooting after BF 4.5-zulu + #12359 update. I've had the time to load a backup diff from 4.4 but it never finish any reboot after that. It starts and suddenly disconnect, normal mode or bootloader mode. Linux, Windows and several known working cables tested. I think it's time to buy a new HGLRC F722 mini.
I'm really sorry I can't test something I initiated, maybe in few weeks

[haslinghuis]

After some conversation on Discord changed the RSSI_DBM element instead of RSNR.

[SteveCEvans]

Just a thought. Instead of adding + how about displaying a 1: or 2: prefix but only after antenna is seen to be non-zero. Until the second antenna is spotted it would look as it does now.

[haslinghuis]

Thanks @SteveCEvans

[ZzyzxTek]

Just a thought. Instead of adding + how about displaying a 1: or 2: prefix but only after antenna is seen to be non-zero. Until the second antenna is spotted it would look as it does now.

This may be nitpicky, but seeing 1:-72 looks a little funky to have the :- together (since RSSI dBm is always a negative number). In goggles with analog resolution, it is a little fuzzy since the dots and dash are small. Doing it as a suffix so it would appear as -72:1 might be a bit clearer to see in goggles?

[ZzyzxTek]

Hmm, was trying to test this, and the build failed.
Entered #12359 as the commit, and the error was quick: make: *** No rule to make target 'IFLIGHT_BLITZ_F722'. Stop.
Log here: https://build.betaflight.com/api/builds/f0a9aab5adfb95907e95e6911e0a048d/log

[LoakAudio]

@glennvenghaus Cool, as I can't test by myself (building 4.5-zulu hex is broken for my FC), do you have a DVR or a link to, to see how it shows in OSD ?

[glennvenghaus]

@glennvenghaus Cool, as I can't test by myself (building 4.5-zulu hex is broken for my FC), do you have a DVR or a link to, to see how it shows in OSD ?

Sure. Here you go.
https://cloud.tachyon-consulting.com:555/index.php/apps/files_sharing/ajax/publicpreview.php?x=3058&y=890&a=true&file=osd-diversity.jpg&t=KE8JGjDXBfRqL1b&scalingup=0

[ZzyzxTek]

@glennvenghaus Cool, as I can't test by myself (building 4.5-zulu hex is broken for my FC), do you have a DVR or a link to, to see how it shows in OSD ?

Sure. Here you go. https://cloud.tachyon-consulting.com:555/index.php/apps/files_sharing/ajax/publicpreview.php?x=3058&y=890&a=true&file=osd-diversity.jpg&t=KE8JGjDXBfRqL1b&scalingup=0

Wait, what, the icon is in the middle? That's not right... My understanding is it should look like the LQ line with the icon first, then antenna:RSSI dBm (2:-63).

[LoakAudio]

Wait, what, the icon is in the middle? That's not right... My understanding is it should look like the LQ line with the icon first, then antenna:RSSI dBm (2:-63).

isn't it rather -63:2 that we should see ? @glennvenghaus seems to be in HD, maybe something in relation with that ?
In an other hand, it's a good way to avoid jiggling and the :- smiley artifact.

[ZzyzxTek]

Wait, what, the icon is in the middle? That's not right... My understanding is it should look like the LQ line with the icon first, then antenna:RSSI dBm (2:-63).

isn't it rather -63:2 that we should see ? @glennvenghaus seems to be in HD, maybe something in relation with that ? In an other hand, it's a good way to avoid jiggling and the :- smiley artifact.

Nah, the case was made to be consistent with the LQ element, which puts mode first, then LQ. So RSSI dBm was changed to put the antenna before the RSSI dBm value. Just scroll up and read the thread for this PR, it's all up there...

The icon in the middle is the way the code is written right now (not an HD thing):
tfp_sprintf(element->buff, "%d:%c%3d", antenna + 1, SYM_RSSI, getRssiDbm());
But using the same reasoning as to why antenna was moved before value, imho the icon should be first to be consistent with all of the other elements.

[glennvenghaus]

I know and understand this is probably low on the list of prios now , but when do you think this can be merged ?
Or if you expect it will take very long time likely , could you maybe do a rebase so i can use it in its current state ?

[glennvenghaus]

TOP !!!
Thank you very much man . Really appreciated. Works great.

[LoakAudio]

@glennvenghaus , does it mean it can be tested with a "zulu" and no additional PR ? 4.4.1 or 4.5 ?

[glennvenghaus]

In betaflight just specify this pr (#12359) in the "select commit " field , replacing the default "master" entry.
Then you get 4.5 master from about 2 days ago when this commit was created, with this commit on top.
Any commits after that date on master you wont get anymore.
Only when this pr is merged with master you can use the normal master.

[ZzyzxTek]

Before this gets approved and merged, the icon seems to be out of position with the latest changes:
tfp_sprintf(element->buff, "%d:%c%3d", antenna + 1, SYM_RSSI, getRssiDbm());

All the other elements have the icon first, and then the values. I think this should be:
tfp_sprintf(element->buff, "%c%d:%3d", SYM_RSSI, antenna + 1, getRssiDbm());

For example, the LQ element is:
tfp_sprintf(element->buff, "%c%1d:%2d", SYM_LINK_QUALITY, osdRfMode, osdLinkQuality);

[github-actions]

Do you want to test this code? Here you have an automated build:
Assets
WARNING: It may be unstable. Use only for testing! See: https://www.youtube.com/watch?v=I1uN9CN30gw for instructions for unified targets!

[haslinghuis]