Skip to content

Conversation

SteveCEvans
Copy link
Member

@SteveCEvans SteveCEvans commented Aug 3, 2023

Use coloured fonts if possible.

Display RSSI dBm as critical if below threshold.

Display all RX related warnings as critical.

Tested on WS firmware 34.40.12.

https://youtu.be/HkRrbCb4Vl8

@SteveCEvans SteveCEvans self-assigned this Aug 3, 2023
@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #13005 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@haslinghuis haslinghuis added this to the 4.5 milestone Aug 3, 2023
@blckmn
Copy link
Member

blckmn commented Aug 4, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> PASS
  • 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 -> PASS

@McGiverGim
Copy link
Member

Tested on WS firmware 34.40.12.

I suppose this is not a public version, so this PR can't be tested until published.

How this works? It's able to colour a custom font uploaded or only valid for preinstalled font?

@SteveCEvans
Copy link
Member Author

Correct, not publicly available. This is recorded in this PR for the record. See https://betaflight.com/docs/development/API/DisplayPort MSP_DP_WRITE_STRING for details of the font usage. I anticipate these being user customisable.

Copy link
Member

@freasy freasy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good apart from config calls. Please evaluate if they are costly, if yes, I recommend local const initialization of those values.

const int16_t osdRssiDbm = getRssiDbm();
static bool diversity = false;

if (osdRssiDbm < osdConfig()->rssi_dbm_alarm) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this runs in the loop, I would load the config value in the init function, as config calls are expensive correct?

There are couple of config calls, I think we should initialize the all in a local variable in init.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These setting can be changed on the fly outside of initialisation. Config access is considered low cost as it's held in RAM. osdConfig()-> appears in https://github.com/betaflight/betaflight/blob/321fd468fd2f3be78681d45e3e578cd768c3f26c/src/main/osd/osd_elements.c 53 times for such comparisons, or, for example, determining which unit symbols to display.

@haslinghuis haslinghuis requested a review from McGiverGim August 5, 2023 12:58
@haslinghuis haslinghuis merged commit fb9587b into betaflight:master Aug 6, 2023
@SteveCEvans
Copy link
Member Author

It’s been reported that WTFOS is currently confused by this update. The simple workaround is to set ‎displayport_msp_fonts = 0,0,0,0 until they add support for four fonts.

@Themightybert95
Copy link

Thanks

davidbitton pushed a commit to davidbitton/betaflight that referenced this pull request Feb 5, 2024
@kenmccann
Copy link

It’s been reported that WTFOS is currently confused by this update. The simple workaround is to set ‎displayport_msp_fonts = 0,0,0,0 until they add support for four fonts.

You are my hero.

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

Successfully merging this pull request may close these issues.

9 participants