fix(color): Ghost module config page cannot be exited with no module attached - #7599
Open
juricabi wants to merge 3 commits into
Open
fix(color): Ghost module config page cannot be exited with no module attached#7599juricabi wants to merge 3 commits into
juricabi wants to merge 3 commits into
Conversation
The page only closed itself once the module replied with GHST_MENU_STATUS_CLOSING, which never arrives when no module is attached, leaving the radio stuck in the Ghost menu. Close the page directly from onLongPressRTN() instead. Also forward checkEvents() to the base Window so the menu line widgets are refreshed. Original change by philmoz (EdgeTX#7598).
SYS, MDL and TELE (short and long) all funnel through NavWindow::doKeyShortcut(). Page::doKeyShortcut() calls onCancel() and then forwards the event to navWindow(), relying on onCancel() having popped the page off the layer stack. This page overrides onCancel() to navigate the Ghost menu rather than close, so Window::topWindow() still returns this page and doKeyShortcut() recurses into itself until the stack overflows. With the default shortcuts 5 of the 6 bindings reach that path; only a short SYS press is safe, as it maps to QM_OPEN_QUICK_MENU and takes the other branch. Ignore the shortcut while this page is open, as radio_diagkeys.h already does and as Lua tools do by not being NavWindows. Long RTN still leaves.
The page was blank until a module answered, with no indication that it was waiting for one. Show STR_WAITING_FOR_MODULE as the B&W menu already does in gui/common/stdlcd/radio_ghost_menu.cpp.
juricabi
force-pushed
the
fix/ghost-menu-hang-colorlcd
branch
from
July 31, 2026 08:22
5b87f53 to
ff34341
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7597.
This picks up where #7598 left off. The first commit is philmoz's change from that PR, which fixes the "can't exit" part of the issue. While testing it on my radio I found the page still locks up on the navigation keys, so the second commit deals with that, and the third adds the waiting message the issue mentions.
The remaining lockup
With #7598 applied long RTN gets you out, but pressing SYS, MDL or TELE still freezes the radio.
Those keys, short and long, all end up in
NavWindow::doKeyShortcut().Page::doKeyShortcut()callsonCancel()and then forwards the event tonavWindow(), which works because normallyonCancel()has popped the page off the layer stack andnavWindow()resolves to whatever is underneath. This page overridesonCancel()to navigate the Ghost menu instead of closing, soWindow::topWindow()still returns the Ghost page anddoKeyShortcut()ends up calling itself until the stack runs out.With the default shortcuts that is 5 of the 6 bindings. Short SYS is the exception, since it maps to
QM_OPEN_QUICK_MENUand takes the other branch.The fix is to ignore the shortcut while this page is open. That matches what the rest of the tools already do:
radio_diagkeys.h,widgets_setup.handtopbar.hall override these keys with empty bodies, and Lua tool pages ignore them too becauseStandaloneLuaWindowderives fromWindowrather thanNavWindowand so has nodoKeyShortcut()at all. Long RTN still leaves the page.This does mean a short SYS press no longer opens the quick menu from this page, where previously it did — it was the one binding that did not hang, since
QM_OPEN_QUICK_MENUtakes the other branch. That path did not really work anyway:PagecallsonCancel()when a quick menu item is picked, and this page'sonCancel()navigates the Ghost menu instead of closing, so choosing an entry left you still in the Ghost menu having sent a strayGHST_BTN_JOYLEFTto the module.Waiting message
The body was blank until a module answered, so nothing on screen said what it was waiting for. This uses
STR_WAITING_FOR_MODULE, the same as the B&W menu ingui/common/stdlcd/radio_ghost_menu.cpp.About the timeout
The issue also asks for the page to close itself after a while. I left that out on purpose: the B&W menu has no timeout either, and it seems better to keep the two consistent than to add one only on colour. With these changes there is no longer a state you cannot get out of.
Testing
On a HelloRadioSky V12 (colour 320x240, keys and rotary encoder).
With a Ghost module connected the menu opens, navigates and closes normally, as before.
With no module attached:
I also unplugged the module while still in the menu. The screen stops updating and keeps showing the last state, but the keys keep working and the page can be left normally, so nothing locks up there either.
NV14 / PL18
Not affected.
targets/pl18/CMakeLists.txtdefines-DPCBPL18for all of its revisions, NV14 included, and bothonLongPressRTN()and the newdoKeyShortcut()sit inside the#if defined(HARDWARE_KEYS) && !defined(PCBPL18)block. I builtPCB=PL18 PCBREV=NV14to check and neither symbol ends up in the binary, so those targets keep their current behaviour.