betaflight / betaflight

Open Source Flight Controller Firmware
GNU General Public License v3.0
8k stars 2.86k forks source link

Remove canvas configuration in cli #13595

Closed haslinghuis closed 1 week ago

haslinghuis commented 3 weeks ago

Maybe a dumb idea - but this will be set in other ways - either include OSD_SD for analog or OSD_HD for 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.

github-actions[bot] commented 3 weeks ago

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

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

SteveCEvans commented 2 weeks ago

I see the logic in this. Can’t think of a use case that would fail.

haslinghuis commented 2 weeks ago

@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.

SteveCEvans commented 2 weeks ago

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

ledvinap commented 2 weeks ago

i need some time for review

ledvinap commented 2 weeks ago

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).

SteveCEvans commented 2 weeks ago

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.