Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework LQ/RSSI (dBm) OSD elements for non-CRSF Rxs #9280

Open
tonycake opened this issue Sep 6, 2023 · 7 comments
Open

Rework LQ/RSSI (dBm) OSD elements for non-CRSF Rxs #9280

tonycake opened this issue Sep 6, 2023 · 7 comments

Comments

@tonycake
Copy link
Contributor

tonycake commented Sep 6, 2023

Current Behavior

Critical LQ/RSSI (dBm) values available only to users of the CRSF protocol.

Desired Behavior

Unlike Betaflight, critical iNav OSD elements for long-range flight such as LQ, and RSSI (dBm) are only available for CRSF receivers.
I propose a bit of a rework, to simplify this, and enable it for all receivers, Ghost included.

Suggested Solution

  1. Rename OSD_CRSF_RSSI_DBM, OSD_CRSF_LQ, OSD_CRSF_SNR_DB OSD_CRSF_TX_POWER to remove the CRSF_ prefix.
  2. osd.c, remove the #if defined(USE_SERIALRX_CRSF) around the rendering of these elements (possibly add this under a USE_SERIALRX_RSSILQ #define if space saving is required)
  3. Modify rx drivers to update rxLinkStatistics with the available parameters (uplinkRSSI, uplinkLQ, uplinkSNR, etc)
  4. Rework configurator to allow these elements to be positioned in the OSD regardless of CRSF Rx selection.
  5. Rework the rendering of some of these OSD elements to place the CRSF-specific behavior in a CRSF conditional block (for example, 300% LQ?????)

I am happy to help out with the changes required for this feature, ideally with contributors who are familiar in this area of code.

Who does this impact? Who is this for?

Everyone doing long-range flight with non-CRSF receivers

@OptimusTi
Copy link
Contributor

OptimusTi commented Sep 6, 2023

I initially get the blame on this and I welcome these changes, specifically the legacy 300% LQ thing that nobody uses. I could take a shot at it but it's going to take me a long time. Too busy at work.

@mmosca
Copy link
Collaborator

mmosca commented Sep 9, 2023

I agree that making it less CRSF specific is a good idea, but it is still at the mercy of the rf protocol reporting those values.

@OptimusTi
Copy link
Contributor

OptimusTi commented Sep 9, 2023

The goal is to have CRSF and Ghost share them. I wish I had some free time.

Point #2 and #3 are the ones that require work. #4 is a bit complicated but doable. #1 and #5 are easy.

@tonycake
Copy link
Contributor Author

I agree that making it less CRSF specific is a good idea, but it is still at the mercy of the rf protocol reporting those values.

There is nothing we can do about that, but at least having a generic framework within iNav will encourage radio manufacturers to support it. Certainly on the Ghost side I am 100% committed to get these values reported correctly.

@mmosca
Copy link
Collaborator

mmosca commented Sep 14, 2023

There is nothing we can do about that, but at least having a generic framework within iNav will encourage radio manufacturers to support it. Certainly on the Ghost side I am 100% committed to get these values reported correctly.

Does Ghost currently implement a LQ telemetry element? Or is it something that would need to be added on both sides?

@tonycake
Copy link
Contributor Author

There is nothing we can do about that, but at least having a generic framework within iNav will encourage radio manufacturers to support it. Certainly on the Ghost side I am 100% committed to get these values reported correctly.

Does Ghost currently implement a LQ telemetry element? Or is it something that would need to be added on both sides?

Ghost implements it, infact it is already processed in the iNav driver and passed to lqTrackerSet (although I am not quite sure what this is doing...)

@OptimusTi
Copy link
Contributor

I believe lqTracker is not related to the LQ% statistic if I remember correctly. It's part of the rx protocol and more of an INAVs "are we live and connected to an rx link" thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants