-
-
Notifications
You must be signed in to change notification settings - Fork 917
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
Enforce OSD option #4251
Enforce OSD option #4251
Conversation
✅ Deploy Preview for origin-betaflight-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
33c46b1
to
f786bdc
Compare
For the wording: as this is meant to be a beginner-accessible feature (most people have no clue what a "MAX7456" is), maybe just Digital and Analog? Other than that, looks great |
b17b4ad
to
eaff493
Compare
SonarCloud is annoying, but most likely fine |
src/js/tabs/firmware_flasher.js
Outdated
} | ||
|
||
// remove osdProtocols from generalOptions | ||
data.generalOptions = data.generalOptions.filter(option => option.value !== 'USE_FRSKYOSD' && option.value !== 'USE_OSD_SD' && option.value !== 'USE_OSD_HD'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added "Group" and "GroupedName" to the results. This will simplify the code here.
src/js/tabs/firmware_flasher.js
Outdated
} | ||
|
||
// remove osdProtocols from generalOptions | ||
data.generalOptions = data.generalOptions.filter(option => option.value !== 'USE_FRSKYOSD' && option.value !== 'USE_OSD_SD' && option.value !== 'USE_OSD_HD'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data.generalOptions = data.generalOptions.filter(option => option.value !== 'USE_FRSKYOSD' && option.value !== 'USE_OSD_SD' && option.value !== 'USE_OSD_HD'); | |
data.generalOptions = data.generalOptions.filter(option => !option.group); |
src/js/tabs/firmware_flasher.js
Outdated
@@ -220,8 +220,26 @@ firmware_flasher.initialize = function (callback) { | |||
return; | |||
} | |||
|
|||
data.osdProtocols = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data.osdProtocols = [ | |
data.osdProtocols = data.generalOptions.filter(option => option.group === 'OSD'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can map to a new item, using the groupedName if you want to change the name to a different name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you will also need to cater for the "none" option... so maybe a helper function to add this, and map. note the none option should have an empty value.
2eb1e48
to
8dd138c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
@nerdCopter do you have more info on what kind of build? Works with cloud build target Edit please check again perhaps ckoudBuildOptions was null |
1bb0bf8
to
0f54217
Compare
Quality Gate failedFailed conditions |
That is how it is named in cloud build API.
None is only selected if analog, digital, frsky is not present. |
https://www.merriam-webster.com/dictionary/analogue versus https://www.merriam-webster.com/dictionary/analog. i think it is British English vs American English issue.
fair, i only saw loading screen , no FC. |
I have updated this to be "american". It will flow through in an hour or so. I does however raise a good point. We should localise these display values. We can do so by adding the american version into the messages.json as a key, and then adding the localisation after request to API. This would allow for crowdin to do the work. However we could also consider adding the localisation to the API based on the language setting provided in the header. |
Lots of issues reported with users not reading release notes so decided to enforce OSD option.