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
Do not enable features by default #13608
base: master
Are you sure you want to change the base?
Conversation
haslinghuis
commented
Apr 29, 2024
•
edited
edited
- Fixes Dashboard build option impedes Betaflight initialization and creates I2C errors. #13590
- See also Improve features (+ cli) #13494 which should be merged first
Do you want to test this code? You can flash it directly from Betaflight Configurator:
WARNING: It may be unstable. Use only for testing! |
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.
This should be accompanied with documentation change.
Also, some features should be enabled - for example with OSD integrated FC, you want FEATURE_OSD almost universally.
#ifdef USE_LED_STRIP | ||
featureEnableImmediate(FEATURE_LED_STRIP); | ||
#endif | ||
#ifdef USE_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.
For consideration:
#if defined(USE_OSD) && USE_OSD >= 2
It will allow config.h to #define USE_OSD 2
and make <target> OPTIONS='USE_OSD=2
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.
Was thinking about missing default features from unified_targets and adding in config.h something like:
#define DEFAULT_FEATURES (FEATURES_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.
@haslinghuis : https://github.com/betaflight/betaflight/pull/13494/files#diff-384c3b23c60da197eb7ecccd9bbfd19d68d96245f45181ec4db541e9c1014b92R418-R432
All that is necessary is to define autoFeatures by target
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.
Updated and tested.
@haslinghuis problem is that you want osd only on targets with integrated osd. the same with other features... |
@ledvinap OSD feature is also used when using MSP instead of MAX7456 |
I think the situations where you want it enabled by default are as follows:
|
Currently
|
@@ -477,6 +477,10 @@ | |||
#endif | |||
#endif | |||
|
|||
#ifndef DEFAULT_FEATURES | |||
#define DEFAULT_FEATURES (FEATURE_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.
This should be depend on USE_OSD included.