Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve features (+ cli) #13494

Merged
merged 10 commits into from
May 20, 2024
Merged

Improve features (+ cli) #13494

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
49eecd4
Features - refactor featureNames[]
Apr 3, 2024
f78d858
Features - add featuresSupportedByBuild
Apr 3, 2024
d51426b
Features - simplify feature sanitization code
Apr 3, 2024
654b658
Features - improve cli feature handling
Apr 3, 2024
10a70e4
fixup! Features - improve cli feature handling
Apr 3, 2024
a4cded9
Features - print available and not supported features separately
Apr 10, 2024
5279f70
Merge remote-tracking branch 'upstream/master' into improvement-features
ledvinap May 16, 2024
3729ad1
Update src/main/cli/cli.c
ledvinap May 16, 2024
e4d75af
features - print full state without command
ledvinap May 17, 2024
74ed6e7
Merge remote-tracking branch 'origin/improvement-features' into impro…
ledvinap May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
141 changes: 75 additions & 66 deletions src/main/cli/cli.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ static bool signatureUpdated = false;
#endif // USE_BOARD_INFO

static const char* const emptyName = "-";
static const char* const emptyString = "";

#define MAX_CHANGESET_ID_LENGTH 8
#define MAX_DATE_LENGTH 20
Expand All @@ -228,14 +227,31 @@ static const char * const mixerNames[] = {
#endif

// sync this with features_e
#define _R(_flag, _name) [LOG2(_flag)] = _name
static const char * const featureNames[] = {
"RX_PPM", "", "INFLIGHT_ACC_CAL", "RX_SERIAL", "MOTOR_STOP",
"SERVO_TILT", "SOFTSERIAL", "GPS", "",
"RANGEFINDER", "TELEMETRY", "", "3D", "RX_PARALLEL_PWM",
"RX_MSP", "RSSI_ADC", "LED_STRIP", "DISPLAY", "OSD",
"", "CHANNEL_FORWARDING", "TRANSPONDER", "AIRMODE",
"", "", "RX_SPI", "", "ESC_SENSOR", "ANTI_GRAVITY", "", NULL
_R(FEATURE_RX_PPM, "RX_PPM"),
_R(FEATURE_INFLIGHT_ACC_CAL, "INFLIGHT_ACC_CAL"),
_R(FEATURE_RX_SERIAL, "RX_SERIAL"),
_R(FEATURE_MOTOR_STOP, "MOTOR_STOP"),
_R(FEATURE_SERVO_TILT, "SERVO_TILT"),
_R(FEATURE_SOFTSERIAL, "SOFTSERIAL"),
_R(FEATURE_GPS, "GPS"),
_R(FEATURE_RANGEFINDER, "RANGEFINDER"),
_R(FEATURE_TELEMETRY, "TELEMETRY"),
_R(FEATURE_3D, "3D"),
_R(FEATURE_RX_PARALLEL_PWM, "RX_PARALLEL_PWM"),
_R(FEATURE_RSSI_ADC, "RSSI_ADC"),
_R(FEATURE_LED_STRIP, "LED_STRIP"),
_R(FEATURE_DASHBOARD, "DISPLAY"),
_R(FEATURE_OSD, "OSD"),
_R(FEATURE_CHANNEL_FORWARDING, "CHANNEL_FORWARDING"),
_R(FEATURE_TRANSPONDER, "TRANSPONDER"),
_R(FEATURE_AIRMODE, "AIRMODE"),
_R(FEATURE_RX_SPI, "RX_SPI"),
_R(FEATURE_ESC_SENSOR, "ESC_SENSOR"),
_R(FEATURE_ANTI_GRAVITY, "ANTI_GRAVITY"),
};
#undef _R

// sync this with rxFailsafeChannelMode_e
static const char rxFailsafeModeCharacters[] = "ahs";
Expand Down Expand Up @@ -3232,58 +3248,56 @@ static void cliMcuId(const char *cmdName, char *cmdline)
static void printFeature(dumpFlags_t dumpMask, const uint32_t mask, const uint32_t defaultMask, const char *headingStr)
{
headingStr = cliPrintSectionHeading(dumpMask, false, headingStr);
for (uint32_t i = 0; featureNames[i]; i++) { // disabled features first
if (strcmp(featureNames[i], emptyString) != 0) { //Skip unused
for (unsigned i = 0; i < ARRAYLEN(featureNames); i++) { // disabled features first
if (featureNames[i]) { //Skip unused
const char *format = "feature -%s";
const bool equalsDefault = (~defaultMask | mask) & (1 << i);
const bool equalsDefault = (~defaultMask | mask) & (1U << i);
headingStr = cliPrintSectionHeading(dumpMask, !equalsDefault, headingStr);
cliDefaultPrintLinef(dumpMask, (defaultMask | ~mask) & (1 << i), format, featureNames[i]);
cliDefaultPrintLinef(dumpMask, (defaultMask | ~mask) & (1U << i), format, featureNames[i]);
cliDumpPrintLinef(dumpMask, equalsDefault, format, featureNames[i]);
}
}
for (uint32_t i = 0; featureNames[i]; i++) { // enabled features
if (strcmp(featureNames[i], emptyString) != 0) { //Skip unused
for (unsigned i = 0; i < ARRAYLEN(featureNames); i++) { // enabled features
if (featureNames[i]) { //Skip unused
const char *format = "feature %s";
if (defaultMask & (1 << i)) {
cliDefaultPrintLinef(dumpMask, (~defaultMask | mask) & (1 << i), format, featureNames[i]);
if (defaultMask & (1U << i)) {
cliDefaultPrintLinef(dumpMask, (~defaultMask | mask) & (1U << i), format, featureNames[i]);
}
if (mask & (1 << i)) {
const bool equalsDefault = (defaultMask | ~mask) & (1 << i);
if (mask & (1U << i)) {
const bool equalsDefault = (defaultMask | ~mask) & (1U << i);
headingStr = cliPrintSectionHeading(dumpMask, !equalsDefault, headingStr);
cliDumpPrintLinef(dumpMask, equalsDefault, format, featureNames[i]);
}
}
}
}

static void printFeatureList(const char* header, uint32_t mask, const char* delimiter, bool lineFeed)
{
if (header) {
cliPrint(header);
}
for (unsigned i = 0; i < ARRAYLEN(featureNames); i++) {
if (featureNames[i] && (mask & (1U << i))) {
cliPrintf("%s%s", i ? delimiter : "", featureNames[i]);
}
}
if (lineFeed) {
cliPrintLinefeed();
}
}

static void cliFeature(const char *cmdName, char *cmdline)
{
uint32_t len = strlen(cmdline);
const uint32_t mask = featureConfig()->enabledFeatures;
if (len == 0) {
cliPrint("Enabled: ");
for (uint32_t i = 0; ; i++) {
if (featureNames[i] == NULL) {
break;
}
if (mask & (1 << i)) {
cliPrintf("%s ", featureNames[i]);
}
}
cliPrintLinefeed();
printFeatureList("Enabled: ", mask, " ", true);
} else if (strncasecmp(cmdline, "list", len) == 0) {
cliPrint("Available:");
for (uint32_t i = 0; ; i++) {
if (featureNames[i] == NULL)
break;
if (strcmp(featureNames[i], emptyString) != 0) //Skip unused
cliPrintf(" %s", featureNames[i]);
}
cliPrintLinefeed();
return;
printFeatureList("Available: ", featuresSupportedByBuild, " ", true);
// only defined features are printed, extra bits are ignored
printFeatureList("NotSupported: ", ~featuresSupportedByBuild, " ", true);
ledvinap marked this conversation as resolved.
Show resolved Hide resolved
} else {
uint32_t feature;

bool remove = false;
if (cmdline[0] == '-') {
// remove feature
Expand All @@ -3292,36 +3306,31 @@ static void cliFeature(const char *cmdName, char *cmdline)
len--;
}

for (uint32_t i = 0; ; i++) {
if (featureNames[i] == NULL) {
cliPrintErrorLinef(cmdName, ERROR_INVALID_NAME, cmdline);
break;
unsigned found = 0;
int featureIdx = -1;
for (unsigned i = 0; !found && i < ARRAYLEN(featureNames); i++) {
if (featureNames[i] && strncasecmp(cmdline, featureNames[i], len) == 0) {
found++;
featureIdx = i;
}

if (strncasecmp(cmdline, featureNames[i], len) == 0) {
feature = 1 << i;
#ifndef USE_GPS
if (feature & FEATURE_GPS) {
cliPrintLine("unavailable");
break;
}
#endif
#ifndef USE_RANGEFINDER
if (feature & FEATURE_RANGEFINDER) {
cliPrintLine("unavailable");
break;
}
#endif
if (remove) {
featureConfigClear(feature);
cliPrint("Disabled");
} else {
featureConfigSet(feature);
cliPrint("Enabled");
}
cliPrintLinef(" %s", featureNames[i]);
break;
}
if (found == 1) {
uint32_t feature = 1U << featureIdx;
const char *verb;
if ((feature & featuresSupportedByBuild) == 0) {
verb = "Unavailable";
} else if (remove) {
featureConfigClear(feature);
verb = (mask & feature) ? "Disabled" : "AlreadyDisabled";
} else {
featureConfigSet(feature);
verb = (mask & feature) ? "AlreadyEnabled" : "Enabled";
}
cliPrintLinef("%s %s", verb, featureNames[featureIdx]);
} else if (found > 1) {
cliPrintErrorLinef(cmdName, "Multiple features match %s", cmdline);
} else /* found <= 0 */ {
cliPrintErrorLinef(cmdName, ERROR_INVALID_NAME, cmdline);
}
}
}
Expand Down
102 changes: 12 additions & 90 deletions src/main/config/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,98 +415,20 @@ static void validateAndFixConfig(void)
gyroConfigMutable()->gyro_hardware_lpf = GYRO_HARDWARE_LPF_NORMAL;
}

// clear features that are not supported.
// I have kept them all here in one place, some could be moved to sections of code above.

#ifndef USE_RX_PPM
featureDisableImmediate(FEATURE_RX_PPM);
#endif

#ifndef USE_SERIALRX
featureDisableImmediate(FEATURE_RX_SERIAL);
#endif

#if !defined(USE_SOFTSERIAL)
featureDisableImmediate(FEATURE_SOFTSERIAL);
#endif

#ifndef USE_RANGEFINDER
featureDisableImmediate(FEATURE_RANGEFINDER);
#endif

#ifndef USE_TELEMETRY
featureDisableImmediate(FEATURE_TELEMETRY);
#endif

#ifndef USE_RX_PWM
featureDisableImmediate(FEATURE_RX_PARALLEL_PWM);
#endif

#ifndef USE_RX_MSP
featureDisableImmediate(FEATURE_RX_MSP);
#endif

#ifndef USE_LED_STRIP
featureDisableImmediate(FEATURE_LED_STRIP);
#endif

#ifndef USE_DASHBOARD
featureDisableImmediate(FEATURE_DASHBOARD);
#endif

#ifndef USE_OSD
featureDisableImmediate(FEATURE_OSD);
#endif

#ifndef USE_SERVOS
featureDisableImmediate(FEATURE_SERVO_TILT | FEATURE_CHANNEL_FORWARDING);
#endif

#ifndef USE_TRANSPONDER
featureDisableImmediate(FEATURE_TRANSPONDER);
#endif

#ifndef USE_RX_SPI
featureDisableImmediate(FEATURE_RX_SPI);
#endif

#ifndef USE_ESC_SENSOR
featureDisableImmediate(FEATURE_ESC_SENSOR);
#endif

#if !defined(USE_ADC)
featureDisableImmediate(FEATURE_RSSI_ADC);
#endif

if (systemConfig()->configurationState == CONFIGURATION_STATE_UNCONFIGURED) {

#ifdef USE_DASHBOARD
featureEnableImmediate(FEATURE_DASHBOARD);
#endif
#ifdef USE_LED_STRIP
featureEnableImmediate(FEATURE_LED_STRIP);
#endif
#ifdef USE_OSD
featureEnableImmediate(FEATURE_OSD);
#endif
#ifdef USE_RANGEFINDER
featureEnableImmediate(FEATURE_RANGEFINDER);
#endif
#ifdef USE_SERVOS
featureEnableImmediate(FEATURE_CHANNEL_FORWARDING);
featureEnableImmediate(FEATURE_SERVO_TILT);
#endif
// clear features that are not supported.
featureDisableImmediate(~featuresSupportedByBuild);

if (systemConfig()->configurationState == CONFIGURATION_STATE_UNCONFIGURED) {
// enable some compiled-in features by default
uint32_t autoFeatures =
FEATURE_DASHBOARD | FEATURE_LED_STRIP | FEATURE_OSD | FEATURE_RANGEFINDER
| FEATURE_CHANNEL_FORWARDING | FEATURE_SERVO_TILT
Comment on lines +424 to +425
Copy link
Member

Choose a reason for hiding this comment

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

Think these are most common, the other ones should be enabled manually

Suggested change
FEATURE_DASHBOARD | FEATURE_LED_STRIP | FEATURE_OSD | FEATURE_RANGEFINDER
| FEATURE_CHANNEL_FORWARDING | FEATURE_SERVO_TILT
FEATURE_LED_STRIP | FEATURE_OSD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@haslinghuis : I tried to avoid changing behavior of code where not necessary. We can easily change defaults in separate PR, probably together with target defaults

#if defined(SOFTSERIAL1_RX_PIN) || defined(SOFTSERIAL2_RX_PIN) || defined(SOFTSERIAL1_TX_PIN) || defined(SOFTSERIAL2_TX_PIN)
featureEnableImmediate(FEATURE_SOFTSERIAL);
| FEATURE_SOFTSERIAL
#endif
#ifdef USE_TELEMETRY
featureEnableImmediate(FEATURE_TELEMETRY);
#endif
#ifdef USE_TRANSPONDER
featureEnableImmediate(FEATURE_TRANSPONDER);
#endif

}
;
featureEnableImmediate(autoFeatures & featuresSupportedByBuild);
}

#if defined(USE_BEEPER)
#ifdef USE_TIMER
Expand Down
61 changes: 61 additions & 0 deletions src/main/config/feature.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,67 @@ PG_RESET_TEMPLATE(featureConfig_t, featureConfig,
.enabledFeatures = DEFAULT_FEATURES | DEFAULT_RX_FEATURE | FEATURE_ANTI_GRAVITY | FEATURE_AIRMODE,
);

// bitmask of features that are supported in current build configuration
uint32_t featuresSupportedByBuild =
0
#ifdef USE_PPM
| FEATURE_RX_PPM,
#endif
| FEATURE_INFLIGHT_ACC_CAL // always available
#ifdef USE_SERIALRX
| FEATURE_RX_SERIAL
#endif
| FEATURE_MOTOR_STOP // always available
#ifdef USE_SERVOS
| FEATURE_SERVO_TILT
#endif
#ifdef USE_SOFTSERIAL
| FEATURE_SOFTSERIAL
#endif
#ifdef USE_GPS
| FEATURE_GPS
#endif
#ifdef USE_RANGEFINDER
| FEATURE_RANGEFINDER
#endif
#ifdef USE_TELEMETRY
| FEATURE_TELEMETRY
#endif
| FEATURE_3D // always available
#ifdef USE_PWM
| FEATURE_RX_PARALLEL_PWM
#endif
#ifdef USE_RX_MSP
| FEATURE_RX_MSP
#endif
#ifdef USE_ADC
| FEATURE_RSSI_ADC
#endif
#ifdef USE_LED_STRIP // but cms will try to use it
| FEATURE_LED_STRIP
#endif
#ifdef USE_DASHBOARD
| FEATURE_DASHBOARD
#endif
#ifdef USE_OSD
| FEATURE_OSD
#endif
#ifdef USE_SERVOS
| FEATURE_CHANNEL_FORWARDING
#endif
#ifdef USE_TRANSPONDER
| FEATURE_TRANSPONDER
#endif
| FEATURE_AIRMODE // always available
#ifdef USE_RX_SPI
| FEATURE_RX_SPI
#endif
#ifdef USE_ESC_SENSOR
| FEATURE_ESC_SENSOR
#endif
| FEATURE_ANTI_GRAVITY // always available
;

static uint32_t runtimeFeatureMask;

void featureInit(void)
Expand Down
7 changes: 7 additions & 0 deletions src/main/config/feature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@

#endif // DEFAULT_RX_FEATURE

// features must be listed in
// config/feature.c:featuresSupportedByBuild
// cli/cli.c:featureNames
typedef enum {
FEATURE_RX_PPM = 1 << 0,
FEATURE_INFLIGHT_ACC_CAL = 1 << 2,
Expand Down Expand Up @@ -72,6 +75,10 @@ typedef struct featureConfig_s {

PG_DECLARE(featureConfig_t, featureConfig);

// Mask of features that have code compiled in with current config.
// Other restrictions on available features may apply.
extern uint32_t featuresSupportedByBuild;

void featureInit(void);
bool featureIsEnabled(const uint32_t mask);
bool featureIsConfigured(const uint32_t mask);
Expand Down