feat(device): add a Device tab with status, controls and graceful shutdown - #26
Conversation
…tdown The Device tab was a placeholder, so a headless Pi could not be inspected or stopped from the phone at all. Stopping one meant pulling power on a live SD card, which is how Pis get corrupted. Everything here consumes contracts the server already exposes: - Status: `trigger_status` (mode, radar link, port, trigger type, accept and reject counters) and `power_status` (state, charge, voltage, provider). Requested on every (re)connect, since a phone joining a session already in progress cannot rely on having seen the server's unprompted push. - Controls: `toggle_debug`, `toggle_camera` and `toggle_camera_stream`, each sent only over a live connection. Socket.IO buffers emits through a transient drop and replays them on reconnect, which would otherwise flip recording or the camera behind the user's back. - Graceful shutdown: `POST /api/shutdown` behind a two-step confirm with observable pending, success, error and retry states. Deliberately excluded: `set_radar_config`. The server refuses radar config in mock mode, so it cannot be honestly verified without hardware. Notes for review: - `radar_connected` is `monitor is not None and not mock_mode`, so it reads false in mock mode while everything works. The UI reports the mode instead of calling a working setup "offline". - A nullable measurement renders an em dash, never 0, matching the Shots screen. A mains-powered Pi reports `available: false` and is shown as "No battery" rather than an empty one. - The shutdown outcome deliberately outlives the connection: a successful shutdown drops the socket ~0.5s after the server answers, and the "wait for its lights to settle" warning has to survive that drop to be read at all. - The debug card waits for the server before offering a control. Debug mode is server-global, so a default "Start" could have stopped a capture that was already running. - `emitWhileConnected` duplicates an equivalent guard on open-flight#21 and open-flight#25; whoever merges last should fold the three into one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QvMGAzMMWRuxRht8aGciAv
johnpacino
left a comment
There was a problem hiding this comment.
A few changes on this one:
P1 — “Shut down the Pi” does not shut down the Pi. The mobile screen says to wait and then cut power, but the server’s /api/shutdown handler only cleans up OpenFlight and exits its process; it does not halt the operating system. That guidance could encourage the unsafe power removal this feature aims to prevent. Either label this “Stop OpenFlight” and remove the cut-power advice, or implement and verify an actual OS shutdown separately.
P1 — The camera controls target nonexistent current-server events. The PR requests get_camera_status and emits camera toggles, but the current server exposes get_camera_capture_settings / camera_capture_settings, not those legacy events. The Camera card will never receive a status on today’s server. I’d remove those controls from this PR or redesign the card around the current capture contract.
P2 — Shutdown can address a different server than the one connected. The screen reloads a saved URL at confirmation time. Saving the connected URL is asynchronous and explicitly allowed to fail, so after switching from Pi A to Pi B that stored value can still be A. A shutdown command must use the active connection’s URL, with a test for this switch.
Resolves conflicts with club selection (open-flight#25) in services/socket.ts and its tests by keeping both features. The two copies of emitWhileConnected are folded into one helper with an optional payload, as both PRs anticipated. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Camera card requested `get_camera_status` and emitted `toggle_camera` and `toggle_camera_stream`, but the current server has none of those handlers: its camera surface is `get_camera_capture_settings` / `camera_capture_settings`. On today's server the card could never receive a status, so it is removed along with its type, store fields and tests rather than left as dead UI. Also corrects two comments: debug status is not pushed on connect (only power and trigger status are), and the debug payload comments no longer cite server line numbers that have drifted. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`POST /api/shutdown` cleans up the OpenFlight server and exits its process (_shutdown_process_after_delay in server.py); it does not halt the operating system. The screen called this "Shut down the Pi" and told the user to wait for its lights to settle before cutting power, which invites the power pull on a live SD card this feature was meant to prevent. The flow is now labelled "Stop OpenFlight", says the Pi itself stays on, and gives no power advice. The request was also addressed to the URL reloaded from storage at confirmation time. Saving the connected URL is asynchronous and allowed to fail, so after switching from Pi A to Pi B the stored value could still be A. The screen now uses the socket's active address (`socketService.currentUrl()`), captured when the user confirms. A retry reuses that captured address: while switching servers the failure card stays up until the new server connects, and re-reading the current address there would stop a Pi nobody confirmed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks — all three confirmed against
The review changes are unit/component-tested only — not yet run on a simulator. Description updated accordingly. |
johnpacino
left a comment
There was a problem hiding this comment.
Thanks for updating, looks good!
Why was this required?
The app is meant to be a complete interface for a headless OpenFlight install, but the Device tab was a placeholder. On a screenless build there was no way to see whether the radar was connected, which port it was on, whether triggers were being rejected, or how much battery was left — those values were simply unobservable. There was also no way to stop the OpenFlight server cleanly from the phone.
What does this PR do?
Replaces the Device tab placeholder with a working screen for the hardware the phone is driving:
POST /api/shutdownbehind a two-step confirm with pending, success, error and retry states.Scope note:
/api/shutdownstops the OpenFlight server process; it does not halt the Pi's operating system. The UI says so ("The Pi itself stays on") and gives no advice about removing power. An actual OS shutdown would need server support and belongs in its own PR.Deliberately not included:
set_radar_config— the server refuses radar config in mock mode, so it cannot be verified without hardware.get_camera_capture_settings/camera_capture_settings; a card built around that contract belongs in its own PR.Automated tests
Adds 3 suites; the full suite is 17 suites / 238 tests on the branch head.
__tests__/useDeviceStore.test.ts(new) — snapshot mirroring, wholesale replacement rather than merging, malformed payloads leaving the last good reading,available: falsetreated as a real answer,debug_toggledomittinglog_pathread as "no log", andreset().__tests__/shutdown.test.ts(new) — absolute URL construction, trailing-slash normalisation, non-2xx surfaced as failure, network rejection, timeout aborting at the deadline, and a runtime withoutAbortSignal.timeout(React Native's polyfill lacks it).__tests__/DeviceScreen.test.tsx(new) — status rendering, mock mode not reported as "radar offline", em dash rather than 0 for absent measurements, waiting vs. empty states, the whole stop state machine, outcome surviving the socket drop a successful stop causes, outcome cleared on reconnect, and the debug control. Regression tests from review:__tests__/socket.test.ts(updated) — connect-time requests, listeners fortrigger_status/power_status/debug_status/debug_toggled,toggle_debugsuppressed during a transient drop, device state cleared on deliberate disconnect and on switching servers, andcurrentUrl()across switch / drop / disconnect.Each review regression test was checked by reintroducing the old behaviour: each fails against its own mutation and no other test does.
Commands run on the branch head:
Manual (human) testing
Platform: iPhone 17 Pro iOS simulator, development build (
npx expo run:ios) — not Expo Go, not a release build, no physical device.Server: a local Node stub implementing the subset of the wire contract this screen consumes, not the real Python server.
Performed on the original revision of this PR:
get_session,get_trigger_status,get_debug_status./dev/ttyUSB0/ triggeraudio/ 12 seen, 9 accepted, 3 rejected; Power On battery, 78%, 3.91 V, geekworm.toggle_debug -> ENABLED,-> DISABLED.POST /api/shutdown -> 200.Not exercised — stated so coverage is not overclaimed:
available: false) and refused-stop (500) paths on device — unit-tested only.Server/API contract impact
No server change. This consumes an existing contract:
get_trigger_status,get_debug_status(read-only, on every connect);toggle_debug(state-changing, sent only while connected).trigger_status,power_status,debug_status,debug_toggled.POST {active connection URL}/api/shutdown— stops the server process only.Units and nullability are preserved as the server states them. Two subtleties are handled explicitly:
radar_connectedismonitor is not None and not mock_mode, so it is false in mock mode while everything works. The UI reports the mode rather than calling a working setup "offline".debug_toggledomitslog_pathentirely when disabling, so an absent key is read as "no log".AI assistance
AI assistance (Claude Code) wrote the types, store, service, screen and tests, after reading
server.pyandpower/models.pyto confirm which events exist and what they carry.Earlier adversarial review rounds found five defects, each now covered by a regression test:
AbortSignal.timeout()missing in React Native (every stop would have failed on device while passing in Jest), a hook-order crash on an ordinary Wi-Fi drop, the outcome being unmounted by the disconnect a successful stop causes, a stale "Try again" able to target a different Pi after reconnecting, and the debug control rendering an unconfirmed default.Maintainer review then found three more, all confirmed against
server.pyand fixed: the stop flow described an OS shutdown the server does not perform, the camera controls targeted events the current server does not implement, and the stop request could address a stale stored URL instead of the active connection. The branch was also merged withmainto resolve conflicts with club selection (#25), folding the twoemitWhileConnectedhelpers into one.Checklist
npm cisucceedsnpx expo-doctorpasses — 21/21npx tsc --noEmitpassesnpm test -- --ci --runInBandpassesnpx expo export --platform allsucceeds when application code changed