Skip to content

Conversation

@McGiverGim
Copy link
Member

The GPS Speed OSD element preview is wrong, depending on the units.
It is showing km/h for British, and mp/h for Imperial and Metric.

This PR changes this, using km/h for Metric, and mp/h for Imperial and British, like in the firmware part:
https://github.com/betaflight/betaflight/blob/025ee87a7aca068e3659fd066b8a9afbed123361/src/main/osd/osd_elements.c#L492-L505

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 8, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.2% 0.2% Duplication

@haslinghuis
Copy link
Member

Have implemented #2146 on user request. The user was from GB and insisted on using km for GPS speed. But it looks like British people are using one or the other depending on birthday?

@McGiverGim
Copy link
Member Author

The actual code was wrong indeed. It was showing bad the units for Metric.
Now we can discuss if British/Imperial must shown one or another and I can modify the PR, but I have applied the same than firmware if I've looked at the correct code. If something is wrong then it must be changed on firmware first.

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within a week.

Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

Compared with firmware and you are indeed correct.
My bad it was one of my first code PR's 😃

@blckmn
Copy link
Member

blckmn commented Jun 10, 2021

AUTOMERGE: (PASS)

  • Github identifies PR as mergeable -> PASS
  • PR is not in draft -> 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 -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> PASS

@mikeller mikeller added this to the 10.8.0 milestone Jun 10, 2021
@mikeller mikeller self-assigned this Jun 10, 2021
@blckmn blckmn merged commit 609a2a3 into betaflight:master Jun 11, 2021
@mikeller mikeller modified the milestones: 10.8.0, 10.7.1 Oct 10, 2021
mikeller pushed a commit that referenced this pull request Oct 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants