Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Aug 5, 2020

On slack there was a demand for a British unit for speed and distance related elements in OSD.
It's tested on a IFLIGHT SUCCEX-D F7 TWING with DJI goggles and this PR adds visibility in OSD preview.

Units

Unit Speed Distance
Imperial MPH Miles
Metric KPH KM
British MPH KM

Affected OSD Elements

OSD Element Imperial Metric British
Altitude Feet Metre Metre
GPS Speed MPH KPH MPH
Home Distance Feet Metre Metre
Numerical Vario FTPS MPS MPS
Flight Distance Feet Metre Metre
OSD Efficiency Miles KM KM

Firmware part: betaflight/betaflight#10080

@haslinghuis
Copy link
Member Author

haslinghuis commented Aug 5, 2020

Added Celsius / Fahrenheit to British unit.

@haslinghuis haslinghuis force-pushed the feature-osd-unit-british branch from d8704fa to 31bd2f2 Compare August 5, 2020 22:05
@haslinghuis haslinghuis changed the title Add support for british units in osd Add support for British units in OSD Aug 5, 2020
Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

Fix the two Sonar issues, apart of this, the PR seems ok to me.

Maybe is better to start using the constants and not the position in the conditions, to make the code clearer. Is not needed for this PR, but I think it will left a better code.

@haslinghuis haslinghuis force-pushed the feature-osd-unit-british branch from 31bd2f2 to 24dd653 Compare August 6, 2020 09:00
@haslinghuis haslinghuis force-pushed the feature-osd-unit-british branch from 24dd653 to cb67ffd Compare August 6, 2020 10:13
McGiverGim
McGiverGim previously approved these changes Aug 6, 2020
Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

It seems good to me!!!

@McGiverGim
Copy link
Member

It seems Travis is failing downloading the innosetup package... I will check to see if it is only Travis or is a general error... :(

@McGiverGim
Copy link
Member

It seems only a Travis problem... the url https://registry.yarnpkg.com/@quanle94/innosetup/-/innosetup-6.0.2.tgz works perfectly for me.

I think is the URL that appears in the logs because is the first devDevpendency in the list:

"devDependencies": {
"@quanle94/innosetup": "^6.0.2",
"chai": "^4.2.0",
"command-exists": "^1.2.8",
"cordova-lib": "^9.0.1",

@haslinghuis
Copy link
Member Author

haslinghuis commented Aug 6, 2020

Now I am happy too. I missed two occasions. Fixed also. Including forgotten ; Also Travis is fine now too.

This is going bigger.... maybe is better to use several lines, one to select the unit and another one to return the string.

@haslinghuis haslinghuis force-pushed the feature-osd-unit-british branch from e36f3cb to 3172183 Compare August 6, 2020 12:09
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 6, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mikeller mikeller merged commit 6370a01 into betaflight:master Aug 24, 2020
@haslinghuis haslinghuis deleted the feature-osd-unit-british branch August 27, 2020 11:04
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.

4 participants