Build the simulated gears the way a real groupset shifts, and ask which gear the bike is parked in - #45
Open
sbroenne wants to merge 14 commits into
Open
Build the simulated gears the way a real groupset shifts, and ask which gear the bike is parked in#45sbroenne wants to merge 14 commits into
sbroenne wants to merge 14 commits into
Conversation
… is parked in The bike never shifts. It sits in one fixed gear, and that gear's ratio is what every virtual gear is scaled from - but the app never asked what it was. A quiet, straight chain line is satisfied by lots of very different gears, so a rider parked in 50/11 was riding gears 90% harder than the screen said, and the easy half of the ladder did not exist. Ask for the parked gear, recommend the quietest one that still encodes, and scale every gear from it. Also replace sort-prune-dedupe in Drivetrain.build with a synchro walk modelled on Shimano Synchronized Shift and SRAM AXS Sequential. Measured over the 72 shipped groupset builds this takes shifts too small to feel from 12 to 0, holes above 25% from 5 to 0, and the smallest step anywhere from 0.4% to 5.9%. The starting gear is now declared rather than derived from TrainerSafety, so editing an unrelated safety number no longer moves the rider's gears. The 616-combination parts matrix is replaced by 17 named real groupsets, and the hardcoded ladder becomes selectable named ladders. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
…name Setup now has a required 'gear the bike is in' step. It names the gear to park in, pre-selects it, and lets the rider confirm or correct it in one tap. When a confirmed gear puts part of the ladder out of reach it says so in plain words rather than failing mid-ride. The Start button now names the real blocker. A rider whose trainer is connected but who has not confirmed a gear used to be told to wait for the trainer, which gave them nothing to act on. Gears can be picked by groupset rather than part by part, and the virtual ladder is now a choice between named ladders, with the trademark disclaimer stated on the screen that names the brands. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Says plainly what the app does: the bike stays in one gear and never shifts, and the app changes gear by changing the wheel size the trainer works from. Explains why the parked gear has to be asked for, with the table showing that a straight chain line still leaves a two-to-one spread in what the gears feel like. Records the measurement behind the algorithm change, the encoding floor under the parked ratio, and the honest note that Campagnolo has no synchronised mode so the app models its gearing rather than an algorithm. Adds the trademark disclaimer to the pages that name brands, and the 'gears all feel too hard' answer to support. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Adds the -shotUnparked fixture and two UI tests: that the parked-gear screen recommends a gear and lets it be confirmed, and that Start names the parked gear when that is the missing step. The existing startup fixtures now park the bike, because every screen downstream of setup assumes it. The startup chain reminder names the confirmed gear rather than giving generic advice, and the ladder notes were shortened so the Gears row reads as one line. Groupset lookup now prefers the default when several groupsets share the same parts. 50/34 with an 11-34 is sold on everything up to Dura-Ace, and naming the most expensive one a rider might own is a worse guess than the most common one they probably do. Screenshots regenerated for the new wording and gear counts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
There was a problem hiding this comment.
Pull request overview
This PR fixes the core “bike never shifts” scaling bug by explicitly modeling (and requiring) the gear the bike is physically parked in, and it replaces the drivetrain build algorithm with a sequential (walked) shift sequence that matches how electronic groupsets behave. It also adds selectable virtual gear ladders, a curated catalog of real buyable groupsets, expanded safety/docs surfaces, and extensive new test coverage to lock the behavior down.
Changes:
- Introduce parked gear modeling (UI + configuration + engine scaling) and gate setup/start on explicit parked-gear confirmation.
- Replace
Drivetrain.build’s cross-product/sort/prune approach with a walked “sequential” gear sequence; add real-groupset catalog and virtual ladder selection. - Add broad new unit/UI tests plus documentation and App Store copy updates to reflect the new setup requirements and behavior.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| VirtualGearsUITests/VirtualGearsUITests.swift | Adds UI coverage for parked-gear selection/confirmation and startup messaging. |
| VirtualGearsProduct/VirtualGearsHomeView.swift | Updates startup reminder + button title/hints to name “parked gear missing” explicitly. |
| VirtualGearsProduct/VirtualGearsApp.swift | Adds screenshot fixture scenario for “trainer ready but parked gear not confirmed”. |
| VirtualGearsProduct/SetupView.swift | Replaces chain-line section with parked-gear flow; adds ladder/groupset selection UI and new views. |
| VirtualGearsProduct/AppConfiguration.swift | Adds groupset/ladder setters and physical setup + parked gear helpers in the store. |
| Tests/VirtualGearsCoreTests/RideabilityTests.swift | New rideability regression suite across all shipped groupset builds. |
| Tests/VirtualGearsCoreTests/ProxyCoordinatorTests.swift | Updates coordinator tests to include parked gear selection. |
| Tests/VirtualGearsCoreTests/ParkedGearTests.swift | New tests pinning parked-gear scaling math, floors/ceilings, and persistence. |
| Tests/VirtualGearsCoreTests/DrivetrainTests.swift | Updates drivetrain invariants to match walked sequencing and declared starting gear. |
| Tests/VirtualGearsCoreTests/DemoRideStateTests.swift | Ensures demo uses parked-gear scaling rather than drivetrain reference gear. |
| Tests/VirtualGearsCoreTests/AppConfigurationTests.swift | Gates setup completion on parked gear; updates ladder naming/summary expectations. |
| Sources/VirtualGearsCore/ProxyCoordinator.swift | Threads parked gear through safety checks and ConfirmedGearEngine construction. |
| Sources/VirtualGearsCore/ParkedGear.swift | New parked-gear + physical-setup model and recommendation/workability logic. |
| Sources/VirtualGearsCore/GroupsetCatalog.swift | New curated catalog of real buyable groupsets + lookup helpers. |
| Sources/VirtualGearsCore/GearLadder.swift | New named/selectable virtual ladders with declared starting gear. |
| Sources/VirtualGearsCore/DrivetrainCatalog.swift | Aligns parts catalog with shipped groupsets and updates cassette definitions. |
| Sources/VirtualGearsCore/Drivetrain.swift | Implements walked “synchronised” sequence and declared starting-gear selection. |
| Sources/VirtualGearsCore/DemoRideState.swift | Passes parked gear into the engine; stages demo configuration with suggestion. |
| Sources/VirtualGearsCore/ConfirmedGearEngine.swift | Scales commands from parked gear ratio (when provided) instead of reference gear. |
| Sources/VirtualGearsCore/AppConfiguration.swift | Adds gear ladder + physical setup fields, forgiving decode, parked-gear gating, and safety checks. |
| README.md | Updates quickstart/feature list for parked-gear requirement and new gearing options. |
| docs/support.md | Adds troubleshooting guidance for wrong parked gear / startup blocker messaging. |
| docs/safety.md | Documents the parked-gear floor/ceiling constraints and why setup enforces them. |
| docs/requirements.md | Updates gearing selection guidance + sequential shifting description and disclaimers. |
| docs/index.md | Adds “one thing it asks for” + sequential ladder explanation to the landing page. |
| docs/how-it-works.md | Documents parked-gear scaling and declared starting gear behavior. |
| docs/APP_STORE.md | Updates App Store copy for parked-gear requirement and real-groupset support. |
| DEVELOPMENT.md | Adds measured rationale for walked ladder + parked-gear scaling explanation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+78
to
+82
| private func clearParkedGear() { | ||
| configuration.physical.parkedChainringTeeth = nil | ||
| configuration.physical.parkedCogTeeth = nil | ||
| configuration.parkInSuggestion() | ||
| } |
Root cause: the five new setup rows (gear ladder, parked gear, physical chainring, physical cassette, groupset) were hand-rolled Button views that never had .buttonStyle(.plain) applied, unlike the pre-existing ChoiceRow component used elsewhere in setup. An earlier attempted fix using .tint(.primary) didn't address the actual cause and was reverted. Fix: extend ChoiceRow (already correct, already shared by the chainring and cassette 'copy a real bike' pickers) with optional note/noteColor/ fits parameters, then migrate all five affected rows onto it. This makes correct rendering structural instead of something each row has to remember to opt into. Add UI test coverage for all five affected rows to guard against this regressing: - testGroupsetChoiceMovesTheCheckmarkOnSelection - testGearLadderChoiceMovesTheCheckmarkOnSelection - testPhysicalChainringAndCassetteChoicesUpdateTheSummary Validated: 302 Swift unit tests pass, all 35 UI tests pass serially, generic iOS Simulator build succeeds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Previously the Start button, when the only thing missing was the parked gear, showed 'Set the gear you are in' but was disabled - naming an action it would not perform when tapped. The only way to actually set the parked gear was to separately notice the unrelated gear-icon Settings button in the toolbar. Now tapping it in that state opens Settings directly, so the instruction the button gives is also what tapping it does. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
…ect safety net - Setup guide: add a "Setup guide" row in Settings so a rider can re-run the groupset/parked-gear/wheel-size wizard later, not just on first launch. Covered by three new UI tests exercising the full step flow, the Skip action, and re-entry from Settings. - Custom gear ladder: let a rider define their own gear count and ratio range instead of choosing between fixed tables, alongside the one built-in ladder. Covered by new GearLadderCatalogTests and UI test coverage for switching to/from Custom and editing its gear count. - FTMSPeripheral: re-issue advertising if the last central disconnects while a ride still wants advertising, as a defensive safety net (BLE advertising should already survive a disconnect, but there was no explicit guarantee). Investigated a live Real Velo disconnect with real device logs; root cause was a Windows-side BLE supervision timeout with no reconnect attempt, not a regression of the previously-fixed control-permission lockout. Documented as a known limitation in README and docs/support.md. - AppConfigurationTests: cover the setupWizardCompleted field's default, mutation, and decode-fallback behavior. Fix a pre-existing UI test bug along the way: the parked-gear Settings row now carries its own accessibility identifier and tests scroll to it, since it fell below the fold once the setup guide row was added above it. Validated: 315/315 Swift tests, 39/39 UI tests (serial, iPhone 17 Pro). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Riders who run a Zwift Cog or other single-speed cog on the trainer had no way in the wizard to keep their real groupset's gearing simulated while telling the app the physical back cog isn't the cassette. The groupset step now asks that once, up front, with a tooth-count stepper (default 14T, matching a Zwift Cog) since some single-speed cogs are a different size. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
The single-sprocket toggle always reset to off/14T on Back, even after a rider had already applied a different choice, because it was plain @State with no seed from the store. It now initializes from the current physical setup so re-visiting the step shows the truth. Also renamed the bottom "Single sprocket, or my bike isn't listed" button, which now collides in wording with the new toggle above it — that button skips groupset choice entirely (virtual ladder or custom parts), not single sprocket with real groupset gearing, so it now reads "None of these, or my bike isn't listed". Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
Three follow-ups from a review of the new single-sprocket wizard step:
1. The single-sprocket toggle sat above a long, scrollable groupset
list, so its state was easy to forget by the time a rider tapped a
row. Each row's note now says so directly ("paired with a 14T
sprocket") when the toggle is on, travelling with the row instead of
living only in a header that scrolls out of view.
2. Settings' standalone Groupset picker only ever changed the
simulated gearing, by design, but left no way to also bring the
physical bike in line with a newly chosen groupset short of
re-running the whole wizard. It now offers that as an explicit,
optional button when the physical setup does not already match —
never automatic, so a rider deliberately simulating a different bike
than the one on the trainer is not silently overwritten.
3. The wizard's wheel-size step only had a Stepper, 1 mm at a time
across a 600 mm range. Added the same typed entry field Settings
already has, so an exact known value does not require dozens of
taps.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2c3d711c-cb97-4bfa-a302-2d01fe5d2f1f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1
The bug worth reading first
The bike never shifts. It sits on the trainer in one fixed gear, and what the rider feels is:
We only ever controlled the second term, and silently assumed the first was 2.40. The README already says "leave the bike in a quiet, straight chain line" — correct advice, but a quiet chain line is satisfied by lots of very different gears:
Step sizes were always right, because the scaling is relative. Only the absolute position of the whole ladder was wrong, which is exactly why it would never have been reported as a bug — shifting feels fine, the gears just are not the gears on screen.
Setup now recommends the quietest gear that works, computed from the groupset, and asks the rider to confirm or correct it. It is required, because a wrong guess corrupts every gear.
There is a hard floor at 2.011 (
2400 × 5.49 / 6553.5): park below it and the hardest gear stops encoding once a riding app asks for a 2400 mm wheel. Naive "small ring, middle cog" advice breaks 105 (34/17 = 2.00), GRX (31/17 = 1.82) and SRAM Wide (30/17 = 1.76), so the suggestion is computed rather than written down. There is also a ceiling that is a product limit, not an encoding one — parked in 50/11 every gear still encodes, but the easiest asks for a 238 mm wheel.isSafetherefore checks workability explicitly; encoding checks alone are not enough.Gears are now walked, not sorted
Drivetrain.buildused to pair every ring with every cog, sort by ratio, prune cross-chained pairs and drop exact duplicates. It now walks the drivetrain the way Shimano Synchro and SRAM AXS Sequential do: one cog per press, a ring change at the shift point paired with a compensating rear jump, never big-big or small-small.Measured across the 72 builds of the groupsets actually shipped here:
Both defects disappear rather than getting patched — a walked drivetrain cannot invent a hole or produce a sub-perception step. Gear counts land at cassette speeds + 3…6 (11-speed → 14–16, matching real Di2).
The starting gear is now declared rather than derived from
TrainerSafety. Changing the riding-app wheel range from 2400 to 2600 used to move a compact rider 10% harder — and that range has already been changed once, to fix FulGaz.Real parts only
The 22 × 28 = 616-combination matrix is replaced by 17 named groupsets across Shimano, SRAM and Campagnolo. Every entry is something you can buy, so every entry can be checked by hand. Custom chainrings and cassettes remain for unlisted bikes. Default is 105 R7100 50/34 with 11-34.
Virtual ladders are now selectable and named, each declaring its own starting gear.
One honest note: there is one sequential concept, not three. Shimano Synchro and SRAM Sequential are the same behaviour under different names, and Campagnolo has no synchro mode at all. So this ships one engine with per-groupset shift points rather than three pretend algorithms.
Validation
Worth flagging
A hardware retest on the KICKR is genuinely warranted. The ring transitions are new behaviour, and the parked-gear question changes what every gear feels like.
Two smaller notes: exact cog-by-cog teeth for Campagnolo 13s and SRAM 10-30 come from general knowledge rather than spec sheets, and on the parked-gear screen the candidates are listed in mechanical order, so on a compact the first few rows all read "Puts some gears out of reach" — the recommendation sits at the top, so this was judged acceptable.
No migration anywhere: the app has not shipped, so there are no saved setups to preserve.