Skip to content

feat(profiles): mirror the server's profile roster - #21

Merged
johnpacino merged 1 commit into
open-flight:mainfrom
btripp:feat/profiles
Sep 23, 2026
Merged

johnpacino merged 1 commit into
open-flight:mainfrom
btripp:feat/profiles

Conversation

@btripp

@btripp btripp commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds the profile data layer: Profile/ProfilesSnapshot wire types, a useProfileStore mirroring the server's roster, and the socket mapping (get_profiles on connect, a profiles listener, and four mutation emitters). No UI.

Why was this required?

Shots are attributed to a profile — the server stamps every shot with profile_id and profile_name, and two people sharing a bay produce a single session. Mobile already stores that attribution and displays the name on each row, but has no way to read the roster or change who is active. On the phone a shared session is therefore indistinguishable from one player's, and there is no way to switch players without walking to the kiosk.

The server already owns the entire surface (get_profiles, set_active_profile, add_profile, rename_profile, remove_profile, each answered with one authoritative profiles snapshot). Nothing server-side is missing; mobile simply never wired it up.

The data layer lands first, without UI, so screens can be built against a settled contract rather than reshaping it underneath them.

Automated tests

__tests__/useProfileStore.test.ts — 8 tests: initial not-loaded state, roster and selection mirrored, roster replaced wholesale rather than merged, malformed snapshot leaving the last good roster, missing active_profile_id treated as no selection, empty roster accepted as a real answer, the open settings dict round-tripped untouched, and reset clearing state.

__tests__/socket.test.ts — 12 added: get_profiles emitted on connect and again on reconnect, snapshot applied on a profiles event, roster kept through a transient drop, cleared on deliberate disconnect(), malformed snapshot ignored, repeated snapshot not accumulating, all four emitters' payload shapes, and nothing emitted while disconnected.

Commands run: npm ci, npx tsc --noEmit, npx jest --ci --runInBand (11 suites / 121 tests). All pass.

Coverage gap, stated plainly: socket.test.ts's fake stores handlers in a flat Record, so re-registration silently overwrites. The reconnect test proves the emit repeats — it does not prove handlers are not duplicated. AGENTS.md asks for duplicate-handler coverage and this harness structurally cannot provide it; fixing that is its own change.

Manual (human) testing

Platforms: iOS 26.5 simulator (iPhone 17 Pro) and Android 17 / API 37 emulator (Pixel_10_Pro_XL).
Build: development build in both cases — npx expo prebuild followed by npx expo run:ios / npx expo run:android. Not Expo Go, not a release build.
Server: OpenFlight Python server in --mock mode on localhost:8080 (Android reached it at 10.0.2.2:8080).

Important caveat: this was exercised on a scratch branch merging all five open PRs (#17–#21), not on this branch in isolation. It shows the five work together and do not conflict; it is not evidence for what this PR ships alone.

Steps performed and observed result:

  1. Native build from clean — iOS compiled and signed in Xcode (0 errors); Android assembleDebug reported BUILD SUCCESSFUL. Both installed and launched.
  2. App opened on the Live tab, status pill "Disconnected", server field pre-filled from persisted storage, all four tabs (Live / Shots / Stats / Device) rendered with icons.
  3. Entered the mock server address and tapped Connect — pill turned green "Connected", and the Connect row was replaced by Simulate Shot / Disconnect.
  4. Tapped Simulate Shot — a full shot rendered identically on both platforms: DRIVER, 161.2 mph ball speed with the gauge filled, and eight metric tiles — 288 yds est. carry (273–302 range), 110.0 mph club speed / 1.47 smash, 10.5° V-launch and +2.1° H-launch both sourced mock with three-dot HIGH confidence, −4.3° club AoA and −4.8° club path from radar, +2.4° spin axis reading "fade", 2,286 rpm spin at LOW confidence.
  5. Theme rendered correctly per device setting — light on the iOS simulator, dark on the Android emulator.

Not exercised — stated so the coverage is not overclaimed:

  • The Shots, Stats and Device tabs were not opened. Programmatic navigation raised an iOS system dialog rather than routing, and UI automation was unavailable, so no tab switch was performed.
  • Disconnect / reconnect cycle.
  • Swing-speed mode (--mock-swing-speed), which is the payload shape that omits shot_number.
  • No physical device; no release build.

Server/API contract impact

No server change. This PR consumes an existing contract:

  • Emits get_profiles, set_active_profile {profile_id}, add_profile {name}, rename_profile {profile_id, name}, remove_profile {profile_id}
  • Consumes profiles {profiles, active_profile_id}

Profile mirrors profiles.py:67-78: id (string), name (string), created_at (ISO-8601 UTC, seconds precision, Z-suffixed), settings (open dict, round-tripped untouched and deliberately not narrowed client-side).

Removes PlayerChangedPayload, which typed a player_changed event that does not exist in the server — verified against server.py. Profiles are what player selection actually needs. ROADMAP.md still specifies set_player/player_changed for Phase 1 item 4, so anyone implementing from the doc would build against a phantom API; that correction belongs in a separate docs change.

AI assistance

AI assistance (Claude Code) wrote the types, store, socket mapping and tests, having first inventoried the server's Socket.IO surface to confirm which events exist.

Two design decisions were taken from the kiosk's implementation rather than invented: the store is deliberately not persisted (the web UI found a second copy of the selection raced the connect-time snapshot, and a phone reconnects far more often than a kiosk); and a transient drop keeps the roster while a deliberate disconnect clears it.

One error caught during the work: a club_changed handler was drafted calling a setClub action that does not exist on the store. It was removed rather than papered over, and club selection is left to its own PR on top of #19.

Checklist

  • This PR has one coherent objective and contains no unrelated changes
  • New or changed behavior has automated tests, or I explained why none apply
  • I documented manual human testing with the actual platform/build used
  • I documented any OpenFlight server contract impact
  • I disclosed substantive AI assistance and personally reviewed every change
  • npm ci succeeds
  • npx expo-doctor passes — fails pre-existing on main, not introduced here: 20/21 checks passed. 1 checks failed — expo@57.0.22 installed, SDK expects ~57.0.23. Present identically on every branch cut from main; worth its own dependency PR.
  • npx tsc --noEmit passes
  • npm test -- --ci --runInBand passes
  • npx expo export --platform all succeeds when application code changed
  • Documentation was updated where required
  • No policy documents are mixed into a product-feature PR
  • No unrelated generated files, formatting, or dependency updates are included

@johnpacino johnpacino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The profile roster looks aligned with the server contract, but I’d like one reconnect behavior fixed before approval. The new mutation methods call socket.emit() whenever a socket object exists. After a temporary Wi‑Fi drop, Socket.IO keeps that object and buffers emitted events, then sends them on reconnect. That could apply an old profile selection or edit later than the user intended; a delayed selection could affect which profile receives subsequent shots.

Please send profile mutations only while the socket is connected, and add a test that simulates a transient disconnect (without calling the service’s deliberate disconnect() method). The current “sends nothing” test does not cover that case.

Non-blocking follow-up: switching directly to a different server leaves the previous server’s roster visible until a new snapshot arrives. It would be good to clear the roster when the server URL changes.

@btripp
btripp requested a review from johnpacino September 22, 2026 05:25
johnpacino pushed a commit that referenced this pull request Sep 23, 2026
…tdown (#26)

* feat(device): add a Device tab with status, controls and graceful shutdown

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 #21 and #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

* fix(device): drop camera controls the server does not implement

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>

* fix(device): stop OpenFlight, not "the Pi", at the connected server

`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>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

@johnpacino johnpacino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Updates look good but can you rebase with main and update? I'll approve and merge after. Thanks!

Shots are attributed to a profile — a person or a place — and the server
already owns the whole surface: get_profiles, set_active_profile,
add_profile, rename_profile, remove_profile, each answered with one
authoritative `profiles` snapshot. Mobile had no way to read or change it,
so a shared bay could not be told apart on the phone.

Adds the wire types, a store that mirrors the roster, and the socket
mapping. No UI: the data layer lands first so screens can be built against
it without reshaping the contract underneath them.

The store is deliberately not persisted. The web UI found that a second
copy of the selection raced the snapshot that arrives on connect, and a
phone reconnects far more often than a kiosk does. The roster survives a
transient drop — Socket.IO reconnects on its own and blanking the picker
on every wifi hiccup would be worse — but a deliberate disconnect or a
switch to a different server clears it, alongside the device status and
club, so another server's roster cannot linger as though current.

Profile mutations go through the shared emitWhileConnected helper, so a
selection made during a transient drop is not buffered by Socket.IO and
replayed on reconnect, where it could decide who later shots are filed
under.

Also removes PlayerChangedPayload. It typed a `player_changed` event that
does not exist in the server; profiles are what player selection needs.
ROADMAP.md still specifies set_player/player_changed for Phase 1 item 4
and wants correcting in a docs change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@johnpacino johnpacino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for updating!

@johnpacino
johnpacino merged commit 9b5de01 into open-flight:main Sep 23, 2026
6 checks passed
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.

2 participants