Improve features (+ cli) #13494

ledvinap · 2024-04-03T12:45:49Z

Description in commits.

654b658 needs some discussion:

is it OK to change feature responses (set/reset/unavailable)?
is AlreadyDisabled / AlreadyEnabled usefull?
feature list returns all known features. Maybe return only features that are available in firmware? Or mark unavailable ones (!GPS)?

Use designated initializers for used features. NULL values are stored in gaps. Use ARRAYLEN() for featureNames iteration Use `unsigned` for bitmasks

bitmask of features that are supported in current build configuration. Copied from init.c sanitization

featuresSupportedByBuild makes things much easier

- refuse all features that are not compiled in - AlreadyDisabled/AlreadyEnabled info - refuse operation if multiple features match

github-actions · 2024-04-03T12:45:59Z

Do you want to test this code? You can flash it directly from Betaflight Configurator:

Simply put #13494 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

src/main/cli/cli.c

blckmn · 2024-04-09T23:46:19Z

the command feature list might be worth adding some params - or simply return all, including unavailable as you indicated.

haslinghuis · 2024-04-10T00:03:23Z

In case of unavailable might have to split in all features - with a list of unavailable features on a second row.

ledvinap · 2024-04-10T09:11:54Z

the command feature list might be worth adding some params - or simply return all, including unavailable as you indicated.

Problem is that it needs more parsing of cmdline - lot of code with minimum gain.

Currently all are returned, for backward compatibility. New commit splits features on two lines, each with header (even code parsing 'Available:' shall work, with limited feature list

ledvinap · 2024-04-10T10:50:26Z

When this PR is open - is it worth supporting feature softserial telem -led_st -osd 3d ( = multiple enable.disable in single command)?

blckmn · 2024-05-07T21:58:30Z

When this PR is open - is it worth supporting feature softserial telem -led_st -osd 3d ( = multiple enable.disable in single command)?

this would make the diff and dump interesting... I like the idea, but it might be more complicated than the potential value it would deliver.

nerdCopter · 2024-05-10T19:11:13Z

a4cded977 bare-minimum testing:
https://build.betaflight.com/api/builds/9ed43bba544fdfa398274c9e207cc175/json

show features
feature list
modified features
show features again
set unavailable

# diff all

# version
# Betaflight / STM32F405 (S405) 4.5.0 May 10 2024 / 19:00:12 (a4cded977) MSP API: 1.46
# config rev: 39d434c

# start the command batch
batch start

# reset configuration to default settings
defaults nosave

board_name FOXEERF405
manufacturer_id FOXE
mcu_id 0042004f4e48500920303452
signature 

# feature
feature TELEMETRY
feature LED_STRIP
feature OSD

# master
set acc_calibration = -11,13,80,1
set osd_canvas_height = 13

profile 0

profile 1

profile 2

profile 3

# restore original profile selection
profile 0

rateprofile 0

rateprofile 1

rateprofile 2

rateprofile 3

# restore original rateprofile selection
rateprofile 0

# save configuration
save
# feature
Enabled:  RX_SERIAL TELEMETRY LED_STRIP OSD AIRMODE ANTI_GRAVITY

# feature list
Available:  INFLIGHT_ACC_CAL RX_SERIAL MOTOR_STOP SOFTSERIAL GPS TELEMETRY 3D RSSI_ADC LED_STRIP OSD AIRMODE ESC_SENSOR ANTI_GRAVITY
NotSupported: RX_PPM SERVO_TILT RANGEFINDER RX_PARALLEL_PWM DISPLAY CHANNEL_FORWARDING TRANSPONDER RX_SPI

# feature ESC_SENSOR
Enabled ESC_SENSOR

# feature -LED_STRIP
Disabled LED_STRIP

# feature
Enabled:  RX_SERIAL TELEMETRY OSD AIRMODE ESC_SENSOR ANTI_GRAVITY

# feature RANGEFINDER
Unavailable RANGEFINDER

src/main/cli/cli.c

nerdCopter · 2024-05-10T19:32:16Z

@ledvinap , i do not mind listing unsupported in feature list as you have done. if the ! is used, then such might not be human-readable.

also, maybe feature should output the same as feature list and no need for the extra word list 🤷‍♂️

SteveCEvans · 2024-05-10T21:00:42Z

The available list is in the configurator. I think it best if the firmware only reports on the features built into it.

src/main/config/config.c

haslinghuis · 2024-05-12T16:18:08Z

@ledvinap please rebase

Co-authored-by: Mark Haslinghuis <mark@numloq.nl>

ledvinap · 2024-05-17T09:06:18Z

The available list is in the configurator. I think it best if the firmware only reports on the features built into it.

I did try to follow old behavior - it did print all features, compiled-in or not.
It was meant to remind user that feature does exist, but must be enabled somehow.

I started this PR because I struggled with some feature (maybe SOFTSERIAL, nut I'm not sure) that was not enabled in cloud build. It took some time before I realized it.

nerdCopter · 2024-05-17T13:28:42Z

i like the unavailable list. i would not want to have to search for and compare against some external source.
i'd like to repeat:

also, maybe feature should output the same as feature list and no need for the extra word list 🤷‍♂️

IMHO

…vement-features

ledvinap · 2024-05-17T18:26:44Z

@nerdCopter : Enabled / Available / Unavailable ?

Maybe I can even add features that got automatically disabled (since last reboot only)

haslinghuis · 2024-05-18T20:49:59Z

src/main/config/config.c

+            FEATURE_DASHBOARD | FEATURE_LED_STRIP | FEATURE_OSD | FEATURE_RANGEFINDER
+            | FEATURE_CHANNEL_FORWARDING | FEATURE_SERVO_TILT


@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

nerdCopter · 2024-05-20T13:21:13Z

74ed6e7ab

# version
# Betaflight / STM32F405 (S405) 4.6.0 May 20 2024 / 08:17:02 (74ed6e7ab) MSP API: 1.46
# config rev: 93dfc69
# board: manufacturer_id: FOXE, board_name: FOXEERF405

# feature
Enabled:  RX_SERIAL LED_STRIP OSD AIRMODE ANTI_GRAVITY
Available:  INFLIGHT_ACC_CAL MOTOR_STOP SOFTSERIAL GPS TELEMETRY 3D RSSI_ADC ESC_SENSOR
Unavailable: RX_PPM SERVO_TILT RANGEFINDER RX_PARALLEL_PWM DISPLAY CHANNEL_FORWARDING TRANSPONDER RX_SPI

# feature -LED_STRIP
Disabled LED_STRIP

# feature ESC_SENSOR
Enabled ESC_SENSOR

# feature
Enabled:  RX_SERIAL OSD AIRMODE ESC_SENSOR ANTI_GRAVITY
Available:  INFLIGHT_ACC_CAL MOTOR_STOP SOFTSERIAL GPS TELEMETRY 3D RSSI_ADC LED_STRIP
Unavailable: RX_PPM SERVO_TILT RANGEFINDER RX_PARALLEL_PWM DISPLAY CHANNEL_FORWARDING TRANSPONDER RX_SPI

Petr Ledvina added 4 commits April 3, 2024 13:19

Features - refactor featureNames[]

49eecd4

Use designated initializers for used features. NULL values are stored in gaps. Use ARRAYLEN() for featureNames iteration Use `unsigned` for bitmasks

Features - add featuresSupportedByBuild

f78d858

bitmask of features that are supported in current build configuration. Copied from init.c sanitization

Features - simplify feature sanitization code

d51426b

featuresSupportedByBuild makes things much easier

Features - improve cli feature handling

654b658

- refuse all features that are not compiled in - AlreadyDisabled/AlreadyEnabled info - refuse operation if multiple features match

haslinghuis reviewed Apr 3, 2024

View reviewed changes

src/main/cli/cli.c Outdated Show resolved Hide resolved

fixup! Features - improve cli feature handling

10a70e4

haslinghuis assigned ledvinap Apr 3, 2024

haslinghuis added the RN: IMPROVEMENT label Apr 3, 2024

blckmn approved these changes Apr 9, 2024

View reviewed changes

Features - print available and not supported features separately

a4cded9

blckmn approved these changes May 7, 2024

View reviewed changes

haslinghuis approved these changes May 7, 2024

View reviewed changes

haslinghuis added this to the 4.6 milestone May 7, 2024

haslinghuis requested review from DieHertz, SteveCEvans, KarateBrot, klutvott123 and nerdCopter May 9, 2024 21:47

haslinghuis reviewed May 10, 2024

View reviewed changes

src/main/cli/cli.c Outdated Show resolved Hide resolved

haslinghuis reviewed May 11, 2024

View reviewed changes

src/main/config/config.c Outdated Show resolved Hide resolved

blckmn approved these changes May 11, 2024

View reviewed changes

haslinghuis added the Rebase Required label May 12, 2024

haslinghuis mentioned this pull request May 13, 2024

Do not enable features by default #13608

Merged

haslinghuis requested a review from a team May 15, 2024 23:10

Merge remote-tracking branch 'upstream/master' into improvement-features

5279f70

haslinghuis removed the Rebase Required label May 16, 2024

haslinghuis approved these changes May 16, 2024

View reviewed changes

Update src/main/cli/cli.c

3729ad1

Co-authored-by: Mark Haslinghuis <mark@numloq.nl>

ledvinap added 2 commits May 17, 2024 20:19

features - print full state without command

e4d75af

Merge remote-tracking branch 'origin/improvement-features' into impro…

74ed6e7

…vement-features

haslinghuis reviewed May 18, 2024

View reviewed changes

hydra mentioned this pull request May 19, 2024

Unnecessary features automatically enabled by default. #13602

Closed

nerdCopter approved these changes May 20, 2024

View reviewed changes

haslinghuis merged commit 286cfb5 into betaflight:master May 20, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve features (+ cli) #13494

Improve features (+ cli) #13494

ledvinap commented Apr 3, 2024

github-actions bot commented Apr 3, 2024

blckmn commented Apr 9, 2024

haslinghuis commented Apr 10, 2024

ledvinap commented Apr 10, 2024

ledvinap commented Apr 10, 2024 •

edited

blckmn commented May 7, 2024

nerdCopter commented May 10, 2024 •

edited

nerdCopter commented May 10, 2024

SteveCEvans commented May 10, 2024

haslinghuis commented May 12, 2024

ledvinap commented May 17, 2024

nerdCopter commented May 17, 2024 •

edited

ledvinap commented May 17, 2024

haslinghuis May 18, 2024

ledvinap May 19, 2024

nerdCopter commented May 20, 2024

		FEATURE_DASHBOARD \| FEATURE_LED_STRIP \| FEATURE_OSD \| FEATURE_RANGEFINDER
		\| FEATURE_CHANNEL_FORWARDING \| FEATURE_SERVO_TILT

	FEATURE_DASHBOARD \| FEATURE_LED_STRIP \| FEATURE_OSD \| FEATURE_RANGEFINDER
	\| FEATURE_CHANNEL_FORWARDING \| FEATURE_SERVO_TILT
	FEATURE_LED_STRIP \| FEATURE_OSD

Improve features (+ cli) #13494

Improve features (+ cli) #13494

Conversation

ledvinap commented Apr 3, 2024

github-actions bot commented Apr 3, 2024

blckmn commented Apr 9, 2024

haslinghuis commented Apr 10, 2024

ledvinap commented Apr 10, 2024

ledvinap commented Apr 10, 2024 • edited

blckmn commented May 7, 2024

nerdCopter commented May 10, 2024 • edited

nerdCopter commented May 10, 2024

SteveCEvans commented May 10, 2024

haslinghuis commented May 12, 2024

ledvinap commented May 17, 2024

nerdCopter commented May 17, 2024 • edited

ledvinap commented May 17, 2024

haslinghuis May 18, 2024

Choose a reason for hiding this comment

ledvinap May 19, 2024

Choose a reason for hiding this comment

nerdCopter commented May 20, 2024

ledvinap commented Apr 10, 2024 •

edited

nerdCopter commented May 10, 2024 •

edited

nerdCopter commented May 17, 2024 •

edited