betaflight / betaflight-tx-lua-scripts

Collection of scripts to configure Betaflight from your TX (currently only supported in OpenTx)
GNU General Public License v3.0
590 stars 142 forks source link

Hide GPS settings if unavailable #495

Closed atomgomba closed 7 months ago

atomgomba commented 7 months ago

This is an improvement over #480 to also check for availability of GPS, not only the VTX. The current mechanism used for checking availability of features has been refactored so that the implementation is more straightforward and easier to extend to support checking for more features in the future (immediately comes to mind OSD and Blackbox; e.g. some AIO boards do not have flash chip). This use-case was mentioned in this comment on the past PR.

However there is an apparent bottleneck when we try to extend this mechanism for showing supported settings pages only. With this we want to avoid confusion and save time for the user; less scrolling and no need to select a page just to find it's not available. Currently we send one MSP command to check availability of each feature, thus increasing the initialization time significantly which ultimately defeats the purpose.

My proposal for a more robust solution is to implement a new MSP command in firmware (say called MSP2_BUILD_CONFIG) which would return the supported features as bit flags (1 or 2 bytes) in a single roundtrip. This would also allow us to postpone VTX Table downloading as well, and do it when the user selects the VTX page (only when it's available). At the end of the day the user may just want to change PIDs and so no point in waiting for VTX Table download on init unconditionally.

For these reasons I'm making this PR a draft and open for discussion.

TheIsotopes commented 7 months ago

I would say we should edit the current scripts first before adding new things (e.g. Blackbox). imo that currently means first checking vtx, gps and osd for presence and if necessary hiding if not present, also preventing the vtx table download.

atomgomba commented 7 months ago

I would say we should edit the current scripts first before adding new things (e.g. Blackbox). imo that currently means first checking vtx, gps and osd for presence and if necessary hiding if not present, also preventing the vtx table download.

You're right, I completely agree this makes sense from a development point of view ("make it work first and optimize after"). The problem is that three roundtrips (that is request-response for OSD, VTX, GPS, already 6 data transfers) take a considerable amount of time, partly because these commands do not just return a status ("on" or "off") but also a list of values which makes the transferred data bigger and the transfer longer.

EDIT: Sorry, I forgot to mention my whole point, which is it makes no sense to release a version with such slow loading times into production, the optimization part has to be done before that happens.

atomgomba commented 7 months ago

Another advantage of adding a dedicated MSP command for this is that we could get not only if a function is built-in (compiled in firmware) but also whether if it is active (say GPS is compiled in firmware, but not available due to device is not powered) - all in a single request

TheIsotopes commented 7 months ago

I tested it and hiding of VTX and GPS works. All that's missing is hiding the OSD and suppressing the download of the VTX tables.

atomgomba commented 7 months ago

So I made some updates (still not checking for OSD). Now features are checked on script startup to decide which menu items we need to show. In case the VTX menu is available and selected then the user is presented with the VTX download dialog and they can decide on downloading the VTX table. Upon successful VTX table download (and on subsequent occasions) the VTX options are presented as normal.

Please note to fully test this one must remove the VTX table from the SD card.

Given this works, checking for OSD will follow in separate PR, but ultimately the build options check will be done through a single dedicated MSP command, which is already in the making, but not yet published.

TheIsotopes commented 7 months ago

Currently everything is working as it should. 👍 VTX and GPS pages are hidden if not available and the VTX tables are no longer downloaded if it is not integrated in the firmware.