Add system diagnostics, power controls, and WiFi radio toggle to Tools tab#389
Conversation
…s tab Expands the web UI Tools tab with safe, purpose-built controls so users can manage the Pi without SSHing in, instead of an arbitrary-command terminal (the web UI has no auth and CSRF is disabled, so a shell would be unsafe). - System Diagnostics card: renders the existing but previously-unused GET /api/v3/system/status endpoint (CPU, memory, temp, disk, uptime), with a manual refresh and a 10s poll. - System Power section: reboot/shutdown buttons wired to the existing reboot_system / shutdown_system actions, behind a confirm step, with a dedicated powerAction() helper that treats the dropped connection as the expected "going offline" outcome rather than an error. - Network Radio section: WiFi on/off toggle backed by new GET/POST /api/v3/wifi/radio endpoints and WiFiManager.set_wifi_radio() / get_wifi_radio_state(). Disabling WiFi is refused unless a wired connection is present (reusing the existing lockout guards), with an explicit force-off confirmation for advanced users. No new privileged commands: uses nmcli radio wifi on|off (already sudo-allowlisted) and the existing reboot/poweroff grants, so the sudoers-alignment guard test stays green. Bluetooth toggle intentionally omitted since the installer removes the BlueZ stack for LED timing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
| Duplication | 2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds WiFi radio state and control methods in ChangesWiFi Radio Control Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ToolsUI as tools.html JS
participant API as api_v3 /wifi/radio
participant WiFiManager
User->>ToolsUI: click wifi-radio-toggle
ToolsUI->>API: POST /wifi/radio {enabled, force}
API->>WiFiManager: set_wifi_radio(enabled, force)
WiFiManager-->>API: (success, message, reason_code)
alt disable refused without Ethernet
API-->>ToolsUI: 400 response with reason=no_ethernet
ToolsUI->>User: show wifi-radio-force-row
User->>ToolsUI: confirm force disable
ToolsUI->>API: POST /wifi/radio {enabled:false, force:true}
API->>WiFiManager: set_wifi_radio(false, true)
WiFiManager-->>API: refreshed radio state
end
API-->>ToolsUI: updated radio state
ToolsUI->>User: update toggle and note
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
web_interface/templates/v3/partials/tools.html (1)
715-719: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDiagnostics polling isn't cleared when the user navigates away from the Tools tab.
window._diagPollIntervalis only cleared/reset when this partial is re-swapped in; if the user switches to a different tab entirely, the interval keeps firingfetch('/api/v3/system/status')every 10s in the background (theif (!panel) return;guard inloadSystemDiagnosticsprevents a JS error, but the network/server call still happens). On a resource-constrained Raspberry Pi, this is unnecessary CPU/network churn while the panel isn't even visible.Consider clearing the interval on an HTMX "leaving this view" hook (e.g.
htmx:beforeSwapon the outer container, or aMutationObserver/unloadhandler scoped to#tools-root) rather than only on re-entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web_interface/templates/v3/partials/tools.html` around lines 715 - 719, The diagnostics polling interval is only stopped when the Tools partial is reloaded, so it keeps running after the user leaves the tab. Update the polling setup around window._diagPollInterval and loadSystemDiagnostics to also clear the interval when the Tools view is being removed or swapped out, using an HTMX view-leave hook on the outer container (for example htmx:beforeSwap) or an equivalent scoped teardown tied to `#tools-root`.src/wifi_manager.py (1)
1923-1972: 🚀 Performance & Scalability | 🔵 TrivialEnable path can block the Flask worker for several seconds.
set_wifi_radio(True, ...)delegates to_ensure_wifi_radio_enabled(), whose retry loop can sleep for a total of ~10-20s across attempts (2s + 1s per retry, up to 3 retries, plus rfkill checks). Since this runs synchronously inside the/api/v3/wifi/radioPOST request handler, a slow/failing radio-enable ties up the request thread for that duration. This mirrors the existing pattern inenable_ap_mode/disable_ap_mode(also synchronous), so it's not new to this PR, but worth flagging since a new UI surface now exercises it directly from a "quick toggle" control that users will expect to be near-instant.Consider offloading long-running WiFi radio changes to a background thread with the endpoint returning immediately (with the UI polling for the new state), or documenting the expected multi-second latency in the toggle's UI copy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/wifi_manager.py` around lines 1923 - 1972, The enable path in set_wifi_radio(True, ...) can block the Flask request thread because it waits inside _ensure_wifi_radio_enabled() with retries and sleeps. Update the /api/v3/wifi/radio flow so the radio change is handled asynchronously in a background worker/thread and the endpoint returns immediately, or add UI/documentation copy that clearly sets expectations for the multi-second delay. Use the existing set_wifi_radio and _ensure_wifi_radio_enabled symbols to keep the fix aligned with the current enable/disable handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 7207-7248: The set_wifi_radio endpoint in api_v3.py parses enabled
with a plain bool() cast, which can turn string inputs like "false" into True.
Update set_wifi_radio to use the same defensive string-to-bool handling already
used for force, so enabled only accepts real booleans or known truthy/falsey
string values. Keep the behavior consistent with the existing
WiFiManager.set_wifi_radio call and preserve the current success/error response
flow.
In `@web_interface/templates/v3/partials/tools.html`:
- Around line 665-696: The disable-without-force path in setWifiRadio is
treating every failure as the ethernet-safety refusal, which hides real backend
errors. Update the API response from set_wifi_radio / the wifi radio handler to
include a distinguishing field such as reason when the no-ethernet guard
triggers, and have setWifiRadio check that field before calling
showWifiForceRow. For non-no_ethernet failures, surface d.message through
showResult instead of showing the force-off confirmation UI; keep the existing
retry/loadWifiRadio flow unchanged.
---
Nitpick comments:
In `@src/wifi_manager.py`:
- Around line 1923-1972: The enable path in set_wifi_radio(True, ...) can block
the Flask request thread because it waits inside _ensure_wifi_radio_enabled()
with retries and sleeps. Update the /api/v3/wifi/radio flow so the radio change
is handled asynchronously in a background worker/thread and the endpoint returns
immediately, or add UI/documentation copy that clearly sets expectations for the
multi-second delay. Use the existing set_wifi_radio and
_ensure_wifi_radio_enabled symbols to keep the fix aligned with the current
enable/disable handling.
In `@web_interface/templates/v3/partials/tools.html`:
- Around line 715-719: The diagnostics polling interval is only stopped when the
Tools partial is reloaded, so it keeps running after the user leaves the tab.
Update the polling setup around window._diagPollInterval and
loadSystemDiagnostics to also clear the interval when the Tools view is being
removed or swapped out, using an HTMX view-leave hook on the outer container
(for example htmx:beforeSwap) or an equivalent scoped teardown tied to
`#tools-root`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c366e02-7110-4dfb-8c1d-34db7024ff7e
📒 Files selected for processing (3)
src/wifi_manager.pyweb_interface/blueprints/api_v3.pyweb_interface/templates/v3/partials/tools.html
Addresses code review feedback on the Tools tab additions:
- api_v3.py: parse the `enabled` POST field with the same string-aware
coercion as `force`. A plain bool() cast turned {"enabled":"false"} into
True (enabling instead of disabling) for any non-UI API caller.
- wifi_manager.set_wifi_radio() now returns a reason code alongside
(success, message); the /wifi/radio error response includes it. The Tools
UI only shows the force-off confirmation when reason == 'no_ethernet', so a
genuine nmcli failure surfaces its real error instead of a misleading
"no wired connection" prompt that would just retry into the same failure.
- tools.html: gate the 10s diagnostics poll on panel visibility
(document.hidden / offsetParent), so switching to another tab stops the
recurring /api/v3/system/status calls instead of churning the Pi off-screen.
The initial load and manual Refresh remain unconditional.
Verified: {"enabled":"false"} now disables (refused w/ reason:no_ethernet),
{"enabled":"true"} enables; inline JS passes node --check; py_compile clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE
…inal-hfk9f3 # Conflicts: # web_interface/templates/v3/partials/tools.html
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/wifi_manager.py (1)
1970-1974: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd stack trace to the fallback error log and narrow the catch.
The trailing
except Exceptionswallows all remaining error types and logs without a stack trace, which hampers remote debugging on the Pi.subprocess.runhere realistically raisesOSError/SubprocessError, so catching those (plus keeping a defensive fallback withexc_info=True) preserves context.As per coding guidelines: "Catch specific exception types ... rather than generic exceptions" and "Include stack traces in error logs to provide complete error context for debugging".
♻️ Proposed change
- except subprocess.TimeoutExpired: - return False, "Command timed out while disabling WiFi radio.", 'timeout' - except Exception as e: - logger.error("Error disabling WiFi radio: %s", e) - return False, "An error occurred while disabling WiFi radio.", 'error' + except subprocess.TimeoutExpired: + return False, "Command timed out while disabling WiFi radio.", 'timeout' + except (OSError, subprocess.SubprocessError) as e: + logger.error("Error disabling WiFi radio: %s", e, exc_info=True) + return False, "An error occurred while disabling WiFi radio.", 'error'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/wifi_manager.py` around lines 1970 - 1974, The fallback error handling in the WiFi radio disable path is too broad and loses traceback context. In the code around the `subprocess.run` call and its surrounding `except` blocks, replace the generic `except Exception` with narrower handling for the expected `OSError`/`subprocess.SubprocessError` cases, and keep a defensive fallback that logs with stack trace details. Update the `logger.error` call in this branch to include exception info so remote debugging has the full context.Sources: Coding guidelines, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/wifi_manager.py`:
- Around line 1970-1974: The fallback error handling in the WiFi radio disable
path is too broad and loses traceback context. In the code around the
`subprocess.run` call and its surrounding `except` blocks, replace the generic
`except Exception` with narrower handling for the expected
`OSError`/`subprocess.SubprocessError` cases, and keep a defensive fallback that
logs with stack trace details. Update the `logger.error` call in this branch to
include exception info so remote debugging has the full context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1a4e020-01b4-44c2-af31-14c66a95a62b
📒 Files selected for processing (3)
src/wifi_manager.pyweb_interface/blueprints/api_v3.pyweb_interface/templates/v3/partials/tools.html
Addresses a CodeRabbit nitpick: the fallback handler in set_wifi_radio()'s disable path caught bare Exception and logged without a traceback. Narrow it to (OSError, subprocess.SubprocessError) — the errors subprocess.run realistically raises — and log with exc_info=True for full context on the Pi. Anything genuinely unexpected now propagates to the endpoint's outer handler (500), matching the codebase's specific-exception convention. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE
2 participants
Summary
Expands the web UI Tools tab with safe, purpose-built controls so users can manage the Pi without SSHing in. This grew out of a request for an in-browser terminal, which was ruled out as too risky (the web UI has no auth and CSRF is disabled, so an arbitrary-command shell would be a remote-code-execution surface). Instead, this wires up a curated set of controls — most of which lean on capabilities that already exist in the backend.
GET /api/v3/system/statusendpoint (CPU, memory, temperature, disk, uptime), with a manual Refresh button and a lightweight 10s poll.reboot_system/shutdown_systemactions, behind a confirm step. A dedicatedpowerAction()helper treats the dropped connection as the expected "going offline" outcome instead of a red error.GET/POST /api/v3/wifi/radioendpoints andWiFiManager.set_wifi_radio()/get_wifi_radio_state(). Turning WiFi off is refused unless a wired (Ethernet) connection is present (reusing the existing lockout guards), with an explicit "force off anyway" confirmation for advanced users.Bluetooth toggle was intentionally omitted: the installer removes the BlueZ stack because it interferes with LED matrix timing, so there'd be nothing to toggle on a standard install.
Type of change
Related issues
Test plan
EMULATOR=true python3 run.py)Verified against a locally-running web server (off-Pi, so
nmcli/vcgencmdabsent — confirms graceful degradation):GET /api/v3/system/statusreturns the full diagnostics payload consumed by the new card.GET /api/v3/wifi/radio→{available:false, enabled:null, ethernet_connected:false}.POST /api/v3/wifi/radio {"enabled":false}is refused with the anti-lockout message (HTTP 400);POST {}→enabled is required(400).GET /v3/partials/toolsreturns HTTP 200 with all new sections; the inline JS passesnode --check; the Python modules compile.test/web_interface/test_systemctl_sudoers_alignment.pypasses (2/2) — no new privileged commands were introduced (uses the already-allowlistednmcli radio wifi on|offand the existing reboot/poweroff grants).Documentation
Plugin compatibility
Checklist
Notes for reviewer
_has_connectivity_safety()/_is_ethernet_connected(); on a Pi reachable only via WiFi, disabling is blocked unless the user explicitly forces it (which will drop their connection — that's the documented trade-off).🤖 Generated with Claude Code
https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE
Generated by Claude Code
Summary by CodeRabbit