Remove canvas configuration in cli #13595
Conversation
|
Do you want to test this code? You can flash it directly from Betaflight Configurator:
WARNING: It may be unstable. Use only for testing! |
|
I see the logic in this. Can’t think of a use case that would fail. |
|
@SteveCEvans the issue was with a diff imported by user. Is there any need to change this settings in CLI as firmware would set them in the correct way. |
|
This should be set by the OSD driver (PAL, NTSC or default HD) or set by the VTX using https://betaflight.com/docs/development/api/displayport#msp_set_osd_canvas |
|
i need some time for review |
|
Are there other settings that are saved, but not expected to be configured by user? Maybe we want 'value that is in diff, but not restored automatically' I can't think think about anything that is both simple and clean (not hack). |
|
The canvas size was exposed to support development and flexibility, but now that we have the build config better sorted, and all HD systems bar WTFOS (which properly handles changing it) use 52x20, I think we can hide this from view. |
| @@ -1522,8 +1522,6 @@ const clivalue_t valueTable[] = { | |||
| { "osd_aux_channel", VAR_UINT8 | MASTER_VALUE, .config.minmaxUnsigned = { 1, MAX_SUPPORTED_RC_CHANNEL_COUNT }, PG_OSD_CONFIG, offsetof(osdConfig_t, aux_channel) }, | |||
| { "osd_aux_scale", VAR_UINT16 | MASTER_VALUE, .config.minmaxUnsigned = { 1, 1000 }, PG_OSD_CONFIG, offsetof(osdConfig_t, aux_scale) }, | |||
| { "osd_aux_symbol", VAR_UINT8 | MASTER_VALUE, .config.minmaxUnsigned = { 0, 255 }, PG_OSD_CONFIG, offsetof(osdConfig_t, aux_symbol) }, | |||
| { "osd_canvas_width", VAR_UINT8 | MASTER_VALUE, .config.minmaxUnsigned = { 0, 63 }, PG_OSD_CONFIG, offsetof(osdConfig_t, canvas_cols) }, | |||
| { "osd_canvas_height", VAR_UINT8 | MASTER_VALUE, .config.minmaxUnsigned = { 0, 31 }, PG_OSD_CONFIG, offsetof(osdConfig_t, canvas_rows) }, | |||
There was a problem hiding this comment.
I suggest wrapping in #ifdef OSD_CANVAS_SIZE_DEBUG.
There was a problem hiding this comment.
@SteveCEvans
Updated with your suggestion.
haslinghuis
force-pushed
the
remove-cli-canvas-setting
branch
from
a80e9f4
to
b434506
Compare
haslinghuis
requested review from
blckmn,
ctzsnooze,
nerdCopter,
sugaarK and
SteveCEvans
haslinghuis
dismissed
SteveCEvans’s stale review
request review after suggested change
Maybe a dumb idea - but this will be set in other ways - either include
OSD_SDfor analog orOSD_HDfor digital FPV. Some digital FPV firmware will update canvas settings. But we can see this in the status command.This PR removes the configuration for canvas - so diff backup from users won't override the setting reducing support issues.