fix(cmake): key the GUI merge on the GUI target, not on TouchGFX - #283
fix(cmake): key the GUI merge on the GUI target, not on TouchGFX#283tobymurray wants to merge 1 commit into
Conversation
una_app_build_app() lists what the merge step must wait for, and adds the GUI ELF only when TOUCHGFX_PATH is defined. That asks whether the app uses TouchGFX. What it needs to know is whether una_app_build_gui() built a GUI ELF. The two matched while TouchGFX was the only way to draw one. Since cb1ae1f (2025-09-29) the SDK also ships the CustomGUI entry point, which needs only a Gui class taking the kernel and a run(), and leaves the app to own the message loop. An app using it builds a GUI ELF and sets no TOUCHGFX_PATH, so the merge target never depends on that ELF: the generated makefile lists Service.elf alone. The ELF is still built, because add_executable puts it in all. What is missing is the ordering, so app_merging.py races the post-build step that packs Tmp/*.gui. "make <App>App" fails outright with "Missing .gui file", which is required for every type but Glance, and a full parallel make succeeds or fails depending on scheduling order. Both outcomes were observed on one tree. Glances are why that file is optional, and they are unaffected: a glance app has no GUI ELF by design. Its service asks the kernel for the glance geometry, sends a list of SDK::Glance controls with RequestGlanceUpdate, and the kernel draws them, so there is no app framebuffer and no GUI process to wait for. if(TARGET) asks CMake the question directly. TOUCHGFX_PATH stays in the condition for apps that call una_app_build_app() before una_app_build_gui(), where the target does not exist yet and add_custom_target resolves DEPENDS at generate time. Verified in the CI image with a CustomGUI app that names the SDK entry point by path and sets none of TOUCHGFX_PATH, TOUCHGFX_LIBS or GUI_*: the merge target gains GUI.elf as a prerequisite and "make <App>App" merges, where on upstream/main the prerequisite is absent and the same command fails. Workout's .uapp is byte-identical across the change (404484 bytes).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFinal app merging now detects an existing GUI ELF target or a defined ChangesGUI dependency handling
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This change corrects GUI build ordering for CustomGUI applications without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Rationale: moving forward with
CustomGUI, which is not represented in this repo presentlyuna_app_build_app()lists what the merge step must wait for, and adds the GUI ELF only whenTOUCHGFX_PATHis defined. That asks whether the app uses TouchGFX. What it needs to know is whetheruna_app_build_gui()built a GUI ELF.The two matched while TouchGFX was the only way to draw one. Since cb1ae1f the SDK also ships the
CustomGUIentry point, which needs only a Gui class taking the kernel and a run(), and leaves the app to own the message loop. An app using it builds a GUI ELF and sets noTOUCHGFX_PATH, so the merge target never depends on that ELF: the generated makefile lists Service.elf alone.The ELF is still built, because
add_executableputs it in all. What is missing is the ordering, soapp_merging.pyraces the post-build step that packs Tmp/*.gui. "make App" fails outright with "Missing .gui file", which is required for every type but Glance, and a full parallel make succeeds or fails depending on scheduling order. Both outcomes were observed on one tree.Glances are why that file is optional, and they are unaffected: a glance app has no GUI ELF by design. Its service asks the kernel for the glance geometry, sends a list of SDK::Glance controls with RequestGlanceUpdate, and the kernel draws them, so there is no app framebuffer and no GUI process to wait for.
if(TARGET) asks CMake the question directly.
TOUCHGFX_PATHstays in the condition for apps that calluna_app_build_app()beforeuna_app_build_gui(), where the target does not exist yet and add_custom_target resolves DEPENDS at generate time.Summary by CodeRabbit