Skip to content

Setup wizard overhaul with Wi-Fi, location and mesh steps, App Hub install queueing, and install robustness fixes - #731

Merged
KenVanHoeylandt merged 5 commits into
mainfrom
develop
Nov 2, 2026
Merged

KenVanHoeylandt merged 5 commits into
mainfrom
develop

Conversation

@KenVanHoeylandt

@KenVanHoeylandt KenVanHoeylandt commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Setup app

  • Split Setup.cpp into Setup.cpp, SetupLayout.cpp, SetupWifi.cpp and SetupCompletion.cpp. Shared types (Phase, StepConfiguration, Context) live in SetupPrivate.h.
  • New Wi-Fi page after the Wi-Fi intro page. It shows a plain network list, "Connecting [spinner]" while connecting, and "Failed to connect" with a Retry button. The connected network is marked in the list. Credentials entry still goes through WifiConnect, which now takes --hide-statusbar so the statusbar stays hidden while Setup is on screen.
  • Location Services is now two pages: an intro with the description, then an "App location access:" page with the pills.
  • New Mesh step ("Enable Mesh?" with Disabled / MeshCore / Meshtastic, default Disabled). It is only added when a LoRa radio is present and, on ESP32, PSRAM is available.
  • Back now returns to the page a step ended on (Wi-Fi list, choice page, ...) instead of resetting to its intro page.
  • The time zone page hides the detect button when location services are disabled.
  • SETUP_VERSION is 3, so existing users see Setup again.

Wi-Fi

  • WifiManage and Setup no longer use getMainDispatcher(). Blocking work (radio on, connect, disconnect, loading saved credentials) is signalled to the app's own task through an event group bit. Bindings got a userData pointer for this.
  • Loading saved credentials no longer happens on the LVGL thread.

App Hub

  • The module can now queue installs and removes for different apps and runs them in order. Per app there is still one pending op, and "update all" excludes installs/removes (returns ERROR_RESOURCE_BUSY). Header docs, CLI message and tests are updated, with a new test for ordering.
  • App details only shows the spinner when that app's own op is queued or running. While another app is busy the buttons stay available so you can queue.
  • The main list shows a "Loading..." status label instead of a second spinner. Loading, empty filter and error messages share showStatusLabel() and are centered.

Install

  • untar() rejects files that aren't a multiple of 512 bytes. minitar calls abort() on a short trailing block, so a truncated or non-tar .app from the Files app crashed the device.
  • Added app_install_is_active(). The statusbar install icon now also shows for installs started outside App Hub (Files app, HTTP upload).

GPS

  • GpsEvent supports GSV and GSA sentences (GPS_EVENT_MESSAGE_GSV, GPS_EVENT_MESSAGE_GSA). The meshtastic GPS driver parses and emits them, and the location provider ignores them.
  • Subscriptions hold a single latest event, so a GSV/GSA arriving right after an unread RMC/GGA overwrites it. I haven't seen it cause lost fixes, but it's the first suspect if they show up.

Misc

  • Exported gmtime, gmtime_r and localtime_r from c-symbols-module.

Summary by CodeRabbit

  • New Features

    • Setup now includes guided Wi-Fi and mesh configuration, with available mesh options presented for selection.
    • GPS updates now include GSV and GSA messages.
    • Wi-Fi Connect can hide the status bar while its window is open.
    • The install status icon appears during active installations.
  • Improvements

    • App Hub can queue installs and removals for different apps together while preventing conflicting operations on the same app.
    • App details remain actionable while operations on other apps are running.
    • Wi-Fi settings handle connection and radio actions more reliably, and location detection controls reflect when detection is unavailable.
    • App Hub status screens and spinner display are improved, with better keyboard navigation in dialogs.
  • Bug Fixes

    • Invalid archive sizes are rejected before installation.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c2dbece-59f1-432a-908c-a0fc5564ff7d
Commits

Reviewing files that changed from the base of the PR and between c0073fb and c6964c4.

Files selected for processing: 4
  • Modules/lvgl-module/source/widgets/spinner.cpp
  • Tactility/Private/Tactility/app/wifimanage/State.h
  • Tactility/Source/app/setup/SetupWifi.cpp
  • Tactility/Source/app/wifimanage/State.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Walkthrough

This PR updates App Hub operation queuing and status views, adds active-install tracking and archive-size validation, and extends GPS sentence events and C time-symbol exports. It restructures setup into phases, adds Mesh selection and Wi-Fi setup, and updates Wi-Fi connection and management behavior. It also changes when the time-zone detection button is shown.

Merge Risk: Low · up to c6964

The changes retain bounded GPS and Wi-Fi issues, including skipped position updates, conflicting connection actions, stale network lists, and no Setup recovery for a rejected saved password. Merge is reasonable with explicit owner awareness and follow-up; no broad outage or data-loss failure is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 32.20% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title names several changes that appear in the changeset: the setup wizard overhaul (SetupPrivate.h, Setup.cpp, SetupLayout.cpp, SetupWifi.cpp, SetupCompletion.cpp), App Hub install queueing (app_…
Full details: Title check

Explanation

The title names several changes that appear in the changeset: the setup wizard overhaul (SetupPrivate.h, Setup.cpp, SetupLayout.cpp, SetupWifi.cpp, SetupCompletion.cpp), App Hub install queueing (app_hub.h, app_hub_service.cpp, and related test), and install robustness fixes (install.cpp untar size checks and app_install_is_active counters). The title is long at 113 characters and 17 words, and it lists several items instead of one main change. It does not mention the GPS GSV/GSA additions, the Wi-Fi dispatcher refactor, or the statusbar and mesh changes. These omissions are acceptable because the title need not cover every detail, and the title is related to the changeset's main work.

Full details: Docstring Coverage

Explanation

Docstring coverage is 32.20% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 59 functions across 29 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 10

Note

Follow-up findings are listed below so you can review them separately from inline comments.

Follow-up (1)
Minor (1)
Minor · Remove the stale gmtime TODO from Documentation/ideas.md. · ideas.md:23

Documentation/ideas.md:23
Minor · Maintainability & Code Quality · Low value

Remove the stale gmtime TODO from Documentation/ideas.md.

This PR adds - Export \gmtime` symbolto the Medium Priority list.gmtimeis already exported inSYMBOLS`, so the list shows finished work as open. Delete that line.

Suggested fix
 ## Medium Priority
 
-- Export `gmtime` symbol
 - Improve watch interface. Consider adding some sort of device classification so UI in apps can choose to optimize for the watch.
Nitpick comments: 2
Modules/app-module/tests/source/install_test.cpp: 1

7-9: Trivial · Maintainability & Code Quality · Poor tradeoff

Add a test that reads app_install_is_active() while an install runs.

The test reads the flag only before and after app_install(). An implementation that never sets the counter still passes. A check during the call would cover the increment in app_install(). The repository does not currently provide a way to hold an install open mid-call. A test hook or fixture would be needed first.

Suggested change

Add a case that calls app_install_is_active() from inside the install path and asserts true. The test needs a blocking or hookable install step to make that observation possible.

Modules/app-hub-module/tests/source/app_hub_test.cpp: 1

234-245: Trivial · Maintainability & Code Quality · Quick win

Check the completion results in the queue-order test.

The test queues remove("first") and install("second") before it reads any event. It then checks only each event's type and operation_id. The finished event takes its result from execute(), so a change that skips execute() for queued operations would still pass. The sibling test checks ERROR_NOT_FOUND, but it runs each operation alone. Both ids are absent from the catalog, so the expected result for each is ERROR_NOT_FOUND.

🧪 Suggested test assertions
     event = waitForEvent(sub);
     CHECK_EQ(event.type, APP_HUB_EVENT_REMOVED);
     CHECK_EQ(event.operation_id, firstId);
+    CHECK_EQ(event.result, ERROR_NOT_FOUND);
     event = waitForEvent(sub);
     CHECK_EQ(event.type, APP_HUB_EVENT_INSTALLING);
     CHECK_EQ(event.operation_id, secondId);
     event = waitForEvent(sub);
     CHECK_EQ(event.type, APP_HUB_EVENT_INSTALLED);
     CHECK_EQ(event.operation_id, secondId);
+    CHECK_EQ(event.result, ERROR_NOT_FOUND);

Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 592d21a8-a96c-4c58-9492-6c44946ad74e
Commits

Reviewing files that changed from the base of the PR and between d72bddf and 477758f.

Files selected for processing: 26
  • Documentation/ideas.md
  • Drivers/gps-meshtastic-module/source/gps_meshtastic.cpp
  • Modules/app-hub-module/include/app_hub/app_hub.h
  • Modules/app-hub-module/source/app_hub_command.cpp
  • Modules/app-hub-module/source/app_hub_service.cpp
  • Modules/app-hub-module/tests/source/app_hub_test.cpp
  • Modules/app-module/include/app/install.h
  • Modules/app-module/source/install.cpp
  • Modules/app-module/tests/source/install_test.cpp
  • Modules/c-symbols-module/source/module.cpp
  • Modules/gps-module/include/gps/gps.h
  • Modules/location-module/source/gps_provider.cpp
  • Tactility/Private/Tactility/app/setup/SetupPrivate.h
  • Tactility/Private/Tactility/app/wificonnect/WifiConnect.h
  • Tactility/Private/Tactility/app/wifimanage/Bindings.h
  • Tactility/Source/app/apphub/AppHubApp.cpp
  • Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp
  • Tactility/Source/app/setup/Setup.cpp
  • Tactility/Source/app/setup/SetupCompletion.cpp
  • Tactility/Source/app/setup/SetupLayout.cpp
  • Tactility/Source/app/setup/SetupWifi.cpp
  • Tactility/Source/app/timezone/TimeZoneView.cpp
  • Tactility/Source/app/wificonnect/WifiConnect.cpp
  • Tactility/Source/app/wifimanage/View.cpp
  • Tactility/Source/app/wifimanage/WifiManage.cpp
  • Tactility/Source/service/statusbar/Statusbar.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Comment thread Tactility/Source/app/setup/Setup.cpp Resolved
Comment thread Tactility/Source/app/setup/Setup.cpp Resolved
Comment thread Tactility/Source/app/setup/Setup.cpp Resolved
Comment thread Tactility/Source/app/setup/Setup.cpp Resolved

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Follow-up Comments

Minor follow-up Keep the Mesh enabled setting when the saved backend is not registered.

408-414: Minor · Functional Correctness · Poor tradeoff

Keep the Mesh enabled setting when the saved backend is not registered.

When the saved backend ID is not registered, the Mesh step keeps the Disabled default. Accepting that default calls mesh_set_enabled(false) and clears the persisted enabled flag. The saved backend ID remains. After that backend registers again, Mesh does not start unless the user re-enables it.

This case is newly reachable in this PR. SETUP_VERSION increases from 2 to 3 in SetupCompletion.cpp, so users with a completed marker from the previous version see Setup again and can reach this step.

The fix requires a decision on how the step presents an unmatched enabled backend. Pre-selecting a backend would silently change the saved backend, so no single-line change is proposed.

Open the source location

Nitpick Keep the removal pending until the install is submitted.

242-247: Trivial · Maintainability & Code Quality · Poor tradeoff

Keep the removal pending until the install is submitted.

The test calls app_hub_remove("first") and then app_hub_install("second") with no wait between them. The worker can finish the removal first. The install then runs without overlapping the removal, and the assertions still pass. In that run, a regression that rejects a different-app install while another operation is pending goes undetected.

Keep the removal queued or running while the install is submitted. A blocking test hook is one option. With the removal still pending, a correct implementation accepts the install and queues it behind the removal. The test then checks the event order. A rejecting implementation fails the install result check.

Open the source location

Nitpick Assert that a successful install also increments the started count.

11-11: Trivial · Maintainability & Code Quality · Poor tradeoff

Assert that a successful install also increments the started count.

The new count check runs only after a failing app_install() call. The current code increments the count before install_package(), so successful calls are counted too. The test would still pass if the increment moved into the failure path. Add a case that installs a valid package and checks that app_install_get_started_count() increases by one. Building a valid package fixture is needed for this case.

Suggested fix
     CHECK_EQ(app_install_get_started_count(), before + 1);
+    // Add: install a valid package fixture and assert the count increases by one.

Open the source location


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ec4c392b-2ce7-4ebd-b96c-65b38d855045
Commits

Reviewing files that changed from the base of the PR and between 477758f and b5e46b2.

Files selected for processing: 13
  • Documentation/ideas.md
  • Modules/app-hub-module/tests/source/app_hub_test.cpp
  • Modules/app-module/include/app/install.h
  • Modules/app-module/source/install.cpp
  • Modules/app-module/tests/source/install_test.cpp
  • Tactility/Private/Tactility/app/setup/SetupPrivate.h
  • Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp
  • Tactility/Source/app/setup/Setup.cpp
  • Tactility/Source/app/setup/SetupCompletion.cpp
  • Tactility/Source/app/setup/SetupLayout.cpp
  • Tactility/Source/app/setup/SetupWifi.cpp
  • Tactility/Source/app/wifimanage/WifiManage.cpp
  • Tactility/Source/service/statusbar/Statusbar.cpp
Files skipped from review as they are similar to previous changes: 2
  • Documentation/ideas.md
  • Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Comment thread Tactility/Source/app/setup/Setup.cpp Resolved

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 568f3e5f-46c9-41ce-be7c-c61f42149e80
Commits

Reviewing files that changed from the base of the PR and between b5e46b2 and c0073fb.

Files selected for processing: 6
  • Modules/lvgl-module/source/widgets/spinner.cpp
  • Tactility/Source/app/alertdialog/AlertDialog.cpp
  • Tactility/Source/app/setup/Setup.cpp
  • Tactility/Source/app/setup/SetupCompletion.cpp
  • Tactility/Source/app/wificonnect/WifiConnect.cpp
  • Tactility/Source/app/wifimanage/WifiManage.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c2dbece-59f1-432a-908c-a0fc5564ff7d
Commits

Reviewing files that changed from the base of the PR and between c0073fb and c6964c4.

Files selected for processing: 4
  • Modules/lvgl-module/source/widgets/spinner.cpp
  • Tactility/Private/Tactility/app/wifimanage/State.h
  • Tactility/Source/app/setup/SetupWifi.cpp
  • Tactility/Source/app/wifimanage/State.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

@KenVanHoeylandt KenVanHoeylandt changed the title Develop Setup wizard overhaul with Wi-Fi, location and mesh steps, App Hub install queueing, and install robustness fixes Nov 2, 2026
@KenVanHoeylandt
KenVanHoeylandt merged commit a8824e9 into main Nov 2, 2026
78 checks passed
@KenVanHoeylandt
KenVanHoeylandt deleted the develop branch November 2, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant