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.

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

Hide GPS settings if unavailable #495

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

atomgomba
Copy link
Contributor

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
Copy link

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
Copy link
Contributor Author

atomgomba commented Jan 14, 2024

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
Copy link
Contributor Author

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
Copy link

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 atomgomba force-pushed the hide-gps-if-unavailable branch from 94680e3 to 571dfd5 Compare January 23, 2024 17:05
@atomgomba
Copy link
Contributor Author

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.

@atomgomba atomgomba marked this pull request as ready for review January 23, 2024 17:11
@TheIsotopes
Copy link

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.

Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

LGTM !!

@atomgomba atomgomba mentioned this pull request Jan 24, 2024
@haslinghuis haslinghuis merged commit 569e409 into betaflight:master Jan 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants