Skip to content

feat(clubs): pick the club shots are filed under - #25

Merged
johnpacino merged 1 commit into
open-flight:mainfrom
btripp:feat/club-selection
Sep 22, 2026
Merged

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

Conversation

@btripp

@btripp btripp commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Why was this required?

Every shot the server records is filed under a club (club on each shot), but the phone had no way to see or change which one. The only way to switch clubs was walking back to the kiosk between shots. On a headless Pi there was no way at all, and every shot was filed under whatever club was last set. That breaks the per-club numbers the kiosk and the phone both show.

The server already owns the whole surface: set_club, a club_changed broadcast to every client, and the current club in session_state. Mobile already had the club list mirrored (#19) and ClubChangedPayload typed. Nothing was wired to them.

What does this PR do?

Adds a Club row to the Live screen. Tapping it opens a sheet listing every club, grouped as Irons / Hybrids / Woods like the kiosk's picker, with the current one highlighted. Picking one sends set_club.

  • Shows what the server confirmed, not the last tap. The server ignores an unknown club with no reply, so the store only changes on club_changed (or the club in session_state on connect). There's no optimistic update to roll back.
  • Sends only over a live connection. Socket.IO keeps the socket through a transient drop and buffers emits, replaying them on reconnect. A pick made during a Wi-Fi hiccup would otherwise land later and silently re-file the shots that follow. This is the same issue raised on feat(profiles): mirror the server's profile roster #21.
  • Disabled unless connected. A tap can't look like it worked when nothing was sent. If the connection drops while the sheet is open, the sheet closes.
  • Cleared on a deliberate disconnect or a switch to a different server; kept through a transient drop.
  • Follows changes from other clients, such as the kiosk or a connected simulator.

Not included: the roadmap's "prompt for a club on first connect", swing-speed training implements (set_training_implement), and profile selection. The profile picker follows once #21 lands.

Automated tests

  • __tests__/socket.test.ts, 11 added: club seeded from session_state; kept when a snapshot has no club (older servers); club_changed applied; malformed club_changed ignored (missing, non-string, null); set_club payload while connected; store unchanged until the server confirms; nothing sent before connecting; nothing sent during a transient drop, even after reconnecting; club kept through a drop; cleared on deliberate disconnect(); cleared when connecting to a different server. The socket fake now tracks connected, the same way feat(profiles): mirror the server's profile roster #21's does.
  • __tests__/ClubPicker.test.tsx, 10 added (RNTL interactions): shows the server's club / "not set"; opening lists all three groups with the current club marked selected; picking calls setClub and closes; the row keeps the old club until confirmation; Done closes without sending; disabled while disconnected, connecting and error; the sheet closes if the connection drops while it's open.
  • __tests__/LiveScreen.test.tsx: the Live screen renders the picker once connected.

I checked that the component tests catch real breakage: removing the disabled guard and the close-on-drop guard makes 4 tests fail.

All run on the head commit: npm ci, npx expo-doctor (21/21), npx tsc --noEmit, npm test -- --ci --runInBand (14 suites / 171 tests), npm run format:check, npx expo lint, npx expo export --platform all.

Manual (human) testing

Platform: iOS 26.5 simulator, iPhone 17 Pro. Build: Expo Go 57.0.9, not a development or release build. Server: the OpenFlight Python server in --mock mode from open-flight/openflight main.

  1. By hand: connected, opened the Club row and picked seven clubs in a row (3-iron, 9-iron, PW, GW, 5-wood, 2-iron, 9-iron) while simulating shots. The row followed each pick, and the server logged a club_changed broadcast for every one, in order.
  2. Scripted with XCUITest, driving Expo Go by accessibility label. It typed the server address, tapped Connect, tapped the Club row (the sheet opened with Driver highlighted), tapped Pitching Wedge, and the row updated. The server broadcast club_changed {"club": "pw"}.
  3. Reconnect behavior (a temporary in-app probe, not committed):
    • the club was seeded from session_state on connect;
    • a change made by a second Socket.IO client appeared on the phone;
    • the server was stopped mid-session, the row dimmed and kept its club, and a pick made while disconnected was not delivered after the server came back (zero club_changed broadcasts after the restart).
  4. Checked dark and light appearance.
Club list After picking

The blue gear is Expo Go's developer button, not part of the app. In Expo Go it overlaps the sheet's Done button; it isn't there in a real build.

Not tested: Android (no emulator run for this branch), a development or release build, a physical device, and swing-speed mode.

Server/API contract impact

No server change. Consumes the existing contract:

  • Emits set_club {club}, where club is one of the ClubType values mirrored in data/clubs.ts.
  • Consumes club_changed {club}, and the club field of session_state.

Adds optional club?: string to SessionStatePayload. It's optional because older servers don't send it; when it's absent the current club is kept, not cleared.

Overlap with #21: both PRs add the same "emit only while connected" guard (emitWhileConnected here, emitProfileMutation there). Whichever merges second should switch to the other's helper so there's one copy. I'll do that when rebasing.

AI assistance

AI assistance (Claude Code) wrote the store and socket changes, the picker component and the tests, and ran the checks and simulator verification above. The design choices (confirmed-only display, the connected-only guard, clearing on a server switch) follow the review feedback on #21 and the kiosk's own set_club / club_changed handling. I tested the picker by hand on the simulator.

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

Adds a club picker to the Live screen, wired to the server's set_club /
club_changed events and to the club session_state restores on connect.

The picker shows the club the server reports, not the last tap: the
server ignores an unknown club without replying, so only its
club_changed confirmation updates the store. set_club is sent only over
a live connection, because Socket.IO buffers emits during a transient
drop and would replay a stale pick on reconnect. The picker is disabled
unless connected, and the club is cleared on a deliberate disconnect or
a switch to a different server.

Co-Authored-By: Claude Opus 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.

Looks good

@johnpacino
johnpacino merged commit 49ac7a4 into open-flight:main Sep 22, 2026
10 checks passed
btripp added a commit to btripp/openflight-mobile that referenced this pull request Sep 22, 2026
Club selection (open-flight#25) and the profile roster touch the same places in
services/socket.ts. Both are kept: switching servers or disconnecting
deliberately now clears the roster and the club, and the profile mutations
use main's emitWhileConnected helper instead of this branch's identical
emitProfileMutation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
btripp added a commit to btripp/openflight-mobile that referenced this pull request Sep 22, 2026
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>
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>
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