Feat/proxy device details page#354
Closed
jonasbark wants to merge 324 commits into
Closed
Conversation
…tion Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-trip test updated to use a full 24-entry ratio list; added a separate test asserting that wrong-length lists are dropped during deserialisation so downstream callers (setGearRatios) never see malformed data.
- drop unused dart:async import - setActive early-returns when target name doesn't exist (avoids spurious persist/notify) - remove throws a descriptive StateError when the target name doesn't exist - hydrateFromSync re-applies single-active invariant per trainer in case the synced payload has duplicates
All call sites now go through core.shiftingConfigs and the active ShiftingConfig. Remove the global getProxy*/setProxy*/clear helpers and the associated clamp constants; these lived at lib/utils/settings/settings.dart:488-568. The unused fitness_bike_definition import it pulled in drops with them.
- rename now refuses to overwrite an existing config with the same target name (previously produced two entries sharing name) - hydrateFromSync now promotes the first config to active when a synced payload has zero actives for a trainer (previously the list got stuck all-inactive and re-synced that way) - trainer feedback suppresses virtualShiftingMode/gradeSmoothing/ gearRatios when the proxy device has no FitnessBikeDefinition so the payload no longer contradicts trainerSupportsVirtualShifting=null
The file exercised getProxy*/setProxy*/clearProxyGearRatios accessors deleted alongside the ShiftingConfig migration. Equivalent coverage now lives in test/models/shifting_config_test.dart (defaults, clamping, 24-entry gear-ratios guard) and test/services/shifting_configs_controller_test.dart (persistence round-trip, active invariant).
Two bugs meant that switching between ShiftingConfigs looked like a no-op in the UI: 1. The picker's "New" button upserted a copy of the currently-active config under a different name, so switching back and forth between two configs with identical values showed no change. Create new configs from ShiftingConfig.defaults(...) instead — the user can still duplicate an existing config from the Manage dialog. 2. TrainerSettingsSection's listener re-seeded bike/rider weight, VS mode, and grade smoothing, but not gear ratios. Switching between configs with different ratio tables silently left the old table in place. Fold the seeding logic into a single _applyActiveConfigToDefinition helper that also calls def.setGearRatios(cfg.gearRatios ?? defaults).
…tic refs Task 2's promotion of FitnessBikeDefinition.neutralGear to an instance field broke three remaining static references in the gear-ratios editor. Route _gearRow's isNeutral check through def.neutralGear, and thread the value into _hintFor as a parameter so the top-level helper stays pure.
Adds SupportMessageGroup widget + groupConsecutiveBySender helper that splits a flat timeline into runs of same-sender messages, then renders each run as one shadcn ChatGroup. The avatar prefix shows once per support-side group instead of next to every reply, matching the expected chat-thread visual. SupportMessageBubble is now stateless about its surroundings (returns just a ChatBubble), with the reply CTA folded into the bubble content and a showSenderLabel flag so only the first bubble in a run repeats "You" / "Support". Both SupportChatPage and SupportThreadPage now build groups before rendering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The button is shown only when no replies exist yet (replyCount == 0), i.e. it actually starts a new thread rather than viewing an existing one. Also updates DE/ES/FR/IT/PL with the corresponding 'create' phrasing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the support chat has no messages yet, the empty state now leads
with a Row that pairs the jonas.jpg Avatar (52 px, primary-tinted
circle backdrop matching the in-thread avatar) with the new
supportChatIntro string ("You'll be talking to Jonas from
BikeControl"). The existing supportChatEmpty CTA stays below as the
secondary call-to-action.
i18n: adds supportChatIntro to all six ARBs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a small inline row at the bottom of the ProxyPage column with a SmallProgressIndicator and a muted-text label. Visible only while core.connection.isScanning is true via a ValueListenableBuilder, so the chip self-hides between scan windows. i18n: new lookingForSmartTrainers key in EN/DE/ES/FR/IT/PL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a ScrollController on the message-list ListView and scrolls to maxScrollExtent on every event that adds or refreshes content: - After _bootstrap loads the initial chat history (jump, no animation, so the page opens already pinned to the bottom). - After _refresh fetches new messages on resume / pull-to-refresh (animated, only when the message count changed). - After enqueuing the optimistic placeholder in _send (animated). - After replacing the placeholder with the server-confirmed message (animated). The helper guards on hasClients so calls before the ListView is built are safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches the message ListView to reverse: true with reversed children, which makes the newest message the natural visual anchor at first build instead of scrolling there after layout. The previous ScrollController + post-frame jumpTo(maxScrollExtent) approach raced the layout: bubble heights (especially with attachments) finalised later than the post-frame callback, leaving the list short of the true bottom. Removes the now-unused ScrollController, _scrollToBottom helper, and all the manual scroll calls. Removes the RefreshIndicator wrapper too; pull-to-refresh on a reversed list would trigger from the bottom edge which is awkward UX, and the page already auto-refreshes on lifecycle resume via didChangeAppLifecycleState. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now reads as a heading describing what the diagnostic payload is, e.g. "The following data will be attached to the message for diagnostic purposes" — fits the new bottom-sheet layout where the string sits above a monospace dump of the payload. DE/ES/FR/IT/PL updated with the equivalent sentence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now reads as "Help & Support" (DE: Hilfe & Support, ES: Ayuda y soporte, FR: Aide et support, IT: Aiuto e supporto, PL: Pomoc i wsparcie). The link entry continues to point at the existing troubleshooting markdown but the label shifts to a broader umbrella term that also covers the new chat-with-support flow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The key was only present in EN, leaving every other locale to fall back to the English label. Adds the matching translation in each ARB. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vice-details-page # Conflicts: # lib/i10n/intl_de.arb # lib/i10n/intl_en.arb # lib/i10n/intl_es.arb # lib/i10n/intl_fr.arb # lib/i10n/intl_it.arb # lib/i10n/intl_pl.arb # lib/pages/proxy_device_details.dart # lib/pages/trainer_feedback.dart
Extends KeyPair.encode/decode to round-trip the optional ControllerButton
icon alongside the existing name + sourceDeviceId fields. Encoded as a
{codePoint, fontFamily, fontPackage} sub-map only when present, so a
button without an icon still serialises as the legacy plain-string
shape — backward compatible with profiles already saved to
SharedPreferences.
Decoding rebuilds the IconData via its codePoint + font family/package,
then applies it via copyWith on the matched ControllerButton or as a
constructor arg for unknown-button fallback. Buttons that previously
had a default icon from their static definition are preserved on
decode (the decoded value overrides only when serialised).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 'Connect your <name> for:' header and its 5 feature bullets in
showInformation() now route through new ARB keys (proxyConnectFor,
proxyFeatureAddVirtualShifting, proxyFeatureAdjustGears,
proxyFeatureDirectControl{controller}, proxyFeatureMiniWorkout,
proxyFeatureWifiProxy).
handleTrainerAction's user-visible Success / Ignored messages
(ERG target updates, gear shift confirmations, mode switches,
intensity ±5%) are now localised via AppLocalizations.current —
matching the pattern in base_actions.dart since the dispatcher has
no BuildContext. The 'No active FitnessBikeDefinition' NotHandled
message stays English; it's an internal diagnostic, never surfaced
to the user.
DE/ES/FR/IT/PL translations added (15 keys each, tone in line with
the rest of the app — du / tú / tu / tu / ty).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a sticky 'support_chat_active' bool to Settings, flipped to true the first time SupportChatService.openChat succeeds. HelpButton becomes a StatefulWidget; if the flag is set in initState it kicks off a background fetchChat() and counts admin-side messages whose createdAt is later than the chat's lastSeenAt. When at least one is found, a small destructive-coloured dot appears both on the button's leading icon (top-right corner) and as the trailing slot of the inline 'Chat with Support' menu entry. Opening the chat clears the dot locally and re-runs the unread check on return so the indicator stays in sync with what the user has now seen. All checks are best-effort: missing auth, network errors, and edge-function failures are swallowed and the dot just stays off. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When an admin-side support-chat message arrives with body
'success_rating', the bubble swaps the raw token for a localised
thank-you sentence ('Thank you for using BikeControl! Please consider
rating BikeControl in the {store} — it helps a lot!') and renders an
inline 'Rate BikeControl' primary button below it.
The {store} placeholder picks the platform-native store name —
App Store on iOS / macOS, Play Store on Android, Microsoft Store on
Windows, with App Store as the web/Linux fallback. The button launches
InAppReview.requestReview() when supported and falls back to
openStoreListing(appStoreId: 'id6753721284', microsoftStoreId:
'9NP42GS03Z26') otherwise — same configuration the existing menu
'Leave a review' entry already uses.
i18n: new successRatingMessage{store} + rateBikeControl keys
translated across en/de/es/fr/it/pl.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The rating prompt already has its own primary 'Rate BikeControl' button; the secondary 'Create Thread' / 'Replies (N)' link below made the bubble look noisy and invited the user to start a thread on a non-conversational nudge. Suppress it when isRatingPrompt is true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The IconData(...) call in KeyPair.decodeButton — added to round-trip
ControllerButton.icon through SharedPreferences — is a non-const
constructor that defeats Flutter's icon-font tree-shaker, breaking
the release build:
This application cannot tree shake icons fonts. It has non-constant
instances of IconData at the following locations:
- lib/utils/keymap/keymap.dart:511:18
In practice the round-trip was redundant: known buttons in
ControllerButton.values already carry their const icon in the static
definition, so they get the same icon back via copyWith on decode
just from the matched name. Unknown buttons never had a custom icon
in the first place, so dropping the encode side too is a no-op for
saved profiles.
Encode now emits only name (+ deviceId when set); decode never
constructs an IconData. Comment in both halves explains the
constraint so this doesn't regress.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
debugText() — used by the support-chat diagnostic preview, the login
screen feedback fallback, and the global crash log — now lists every
ProxyDevice currently in core.connection.proxyDevices on its own
line. Each entry summarises the bits that matter for diagnosing a
Bridge / proxy issue:
<name> · state=<disconnected|starting|started|bridged> ·
mode=<proxy|wifi|bluetooth> · def=<RuntimeType> ·
fw=<firmware> · mfg=<manufacturer> ·
[gear=<n>/<max> · trainerMode=<erg|sim|...> ·
power=<W> · cadence=<rpm> · speed=<km/h> · hr=<bpm>]
Telemetry fields only appear when the active definition is a
FitnessBikeDefinition and the corresponding ValueNotifier has a
non-null reading.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The power / cadence / speed / hr fields aren't useful for diagnosing a stuck Bridge or proxy issue — they're just current-rider state. The BLE service & characteristic UUIDs are the actually-diagnostic bits, and TelemetrySnapshot already builds that block as the chat freetext. Promotes _buildServicesFreetext in telemetry_snapshot.dart to a public buildProxyServicesFreetext, then has _describeProxyDevice in menu.dart drop the live-metric fields and append the services block indented under each proxy entry. Same exact text the support chat freetext already includes, so report and freetext stay in lockstep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “proxy smart trainer” details experience and related infrastructure to support Virtual Shifting, local workout recording/export, and improved controller/config UX across the app.
Changes:
- Add Proxy Device Details UI (connect mode picker, live metrics, pro notice) and integrate proxy devices into navigation/debug output.
- Add workout recording stack (recorder, summary, FIT writer, repository) plus related action handling and tests.
- Add review prompt banner/service, support chat models/widgets, controller contour visualization, and multiple settings/keymap enhancements; remove
ios_receiptand add new dependencies/plugins.
Reviewed changes
Copilot reviewed 183 out of 231 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| windows_iap/analysis_options.yaml | Set Dart formatter options (page width, trailing commas). |
| windows/flutter/generated_plugins.cmake | Register share_plus for Windows. |
| windows/flutter/generated_plugin_registrant.cc | Register share_plus plugin for Windows. |
| pubspec.yaml | Bump version; add deps (fit_tool, share_plus, flutter_svg, http_parser, fake_async); remove ios_receipt; add assets. |
| macos/Podfile.lock | Remove ios_receipt; add share_plus. |
| macos/Flutter/GeneratedPluginRegistrant.swift | Remove ios_receipt; register share_plus. |
| .github/workflows/patch.yml | Make Shorebird patch release version configurable via workflow input. |
| windows_iap/analysis_options.yaml | Formatter config for windows_iap package. |
| prop_public/analysis_options.yaml | Formatter config for prop_public package. |
| accessibility/analysis_options.yaml | Formatter config for accessibility package. |
| README.md | Update feature list to describe Virtual Shifting customization and trainer proxying. |
| CHANGELOG.md | Add unreleased 5.4.0 notes describing new proxy trainer features and other additions. |
| test/widgets/ui/stepper_control_test.dart | Widget test for new StepperControl. |
| test/utils/settings/settings_review_prompt_test.dart | Unit tests for review prompt settings persistence. |
| test/utils/settings/settings_retrofit_mode_test.dart | Unit tests for retrofit mode persistence. |
| test/utils/keymap/supported_app_virtual_gear_amount_test.dart | Tests for per-app virtual gear count (MyWhoosh vs default). |
| test/utils/keymap/controller_button_initials_test.dart | Tests for new ControllerButton.initials. |
| test/utils/core_logic_preferred_bridge_transport_test.dart | Tests for bridge transport selection logic. |
| test/services/workout/workout_summary_test.dart | Tests for WorkoutSummary aggregation logic. |
| test/services/workout/workout_repository_test.dart | Tests for FIT file persistence/listing/deletion. |
| test/services/workout/workout_recorder_test.dart | Tests for recorder state transitions and timing behavior. |
| test/services/workout/trainer_metrics_test.dart | Tests for TrainerMetrics factory behavior on unknown definitions. |
| test/services/workout/fit_writer_test.dart | Tests for FIT encoding and round-trip parsing. |
| test/services/telemetry_snapshot_test.dart | Tests for TelemetrySnapshot JSON shaping/truncation behavior. |
| test/services/shifting_configs_controller_test.dart | Tests for shifting config persistence/selection invariants. |
| test/pages/proxy_device_details_test.dart | Widget test for Proxy Device Details header/actions. |
| test/pages/proxy_device_details_pro_notice_test.dart | Widget tests for Pro notice copy and remaining-time formatting. |
| test/pages/proxy_device_details/connection_card_trainer_support_test.dart | Widget test for ConnectionCard rows and missing-transport hint. |
| test/models/user_settings_shifting_test.dart | Tests for syncing shifting configs via UserSettings JSON. |
| test/bluetooth/proxy/handle_trainer_action_consolidated_test.dart | Tests for trainer-control actions mapped to the proxy definition. |
| lib/widgets/ui/wifi_animation.dart | Adjust default WiFi animation size. |
| lib/widgets/ui/stepper_control.dart | New stepper control widget. |
| lib/widgets/ui/setting_tile.dart | New reusable setting tile/card widget. |
| lib/widgets/ui/connection_method.dart | Add small variant + localization improvements + instruction link handling. |
| lib/widgets/ui/button_widget.dart | Redesign controller button rendering; add keymap-based badge icon. |
| lib/widgets/ui/animated_button_widget.dart | Add hover/tap behaviors and trigger assignment popup integration. |
| lib/widgets/trainer_features.dart | Show mini workout card for in-app BikeControl trainer app; simplify manual control tile. |
| lib/widgets/status_icon.dart | Wrap progress indicator in RepaintBoundary. |
| lib/widgets/scan.dart | Remove unused SizedBox() from scan UI. |
| lib/widgets/review_banner.dart | New review prompt banner widget. |
| lib/widgets/mouse_pair_widget.dart | Add small parameter passthrough to ConnectionMethod. |
| lib/widgets/keyboard_pair_widget.dart | Add small parameter passthrough to ConnectionMethod. |
| lib/widgets/menu.dart | Expand debug payload (proxy devices summary + services); add _describeProxyDevice. |
| lib/widgets/keymap_explanation.dart | Refactor trigger-conflict logic into shared dialog/helper. |
| lib/widgets/controller/trigger_conflict_dialog.dart | New shared trigger conflict dialog + helper utilities. |
| lib/widgets/controller/controller_layout.dart | New controller silhouette/layout model (SVG + procedural shapes). |
| lib/widgets/controller/controller_canvas.dart | New canvas renderer for controller contours + positioned buttons. |
| lib/widgets/apps/zwift_tile.dart | Add small parameter. |
| lib/widgets/apps/zwift_mdns_tile.dart | Add small parameter. |
| lib/widgets/apps/openbikecontrol_mdns_tile.dart | Add small parameter; update instruction link; include error details in notification. |
| lib/widgets/apps/openbikecontrol_ble_tile.dart | Add small parameter. |
| lib/widgets/apps/mywhoosh_link_tile.dart | Add small parameter. |
| lib/widgets/apps/local_tile.dart | Add small parameter. |
| lib/widgets/unlock_confirm.dart | New unlock confirmation countdown widget for Click V2 unlock flow. |
| lib/utils/settings/settings.dart | Add retrofit mode + auto-connect + support chat + review prompt persistence; notify ProxyDevices on trainer app change. |
| lib/utils/media_key_handler.dart | Add icons for media keys when creating ControllerButtons. |
| lib/utils/keymap/buttons.dart | Add trainer-control actions + isOutsideTrainerApp flag + trainerActions constant; add ControllerButton.initials. |
| lib/utils/keymap/apps/supported_app.dart | Add TrainerConnectionType enum, virtualGearAmount default, and register BikeControl as supported app. |
| lib/utils/keymap/apps/my_whoosh.dart | Override virtual gear amount to 30. |
| lib/utils/keymap/apps/bike_control.dart | New SupportedApp for “BikeControl” itself. |
| lib/utils/iap/iap_manager.dart | Guard Pro entitlement getters when not initialized. |
| lib/utils/core.dart | Add shifting configs controller, workout recorder/repo, bridge usage tracker, review prompt service, preferred bridge transport helper, and small tiles. |
| lib/utils/actions/base_actions.dart | Add workout pause/resume and trainer-control action handling; allow actions that don’t require trainer connection. |
| lib/utils/actions/android.dart | Add support for broadcasting a custom Android intent action from keymaps. |
| lib/services/workout/workout_sample.dart | New telemetry sample model. |
| lib/services/workout/workout_summary.dart | New workout aggregation model + JSON serialization. |
| lib/services/workout/workout_recorder.dart | New workout recorder with pause/resume and active duration tracking. |
| lib/services/workout/trainer_metrics.dart | Unify metric sources across bike definition types. |
| lib/services/workout/fit_writer.dart | FIT activity encoding via fit_tool. |
| lib/services/workout/past_workout.dart | New listing model for stored workout files + optional summary sidecar. |
| lib/services/workout/workout_repository.dart | Store/list/delete FIT workouts + summary sidecar JSON. |
| lib/services/review_prompt_service.dart | New service to decide when to show review banner based on session count & snooze. |
| lib/services/support_chat_models.dart | New support chat/message/attachment models + JSON parsing. |
| lib/repositories/user_settings_repository.dart | Include shifting configs in sync payload; apply shifting configs on sync. |
| lib/pages/trainer.dart | Pass small to tiles; only show sections when trainer app + target selected. |
| lib/pages/proxy.dart | Navigate to new ProxyDeviceDetails; auto-start proxy on tap; show “scanning for trainers” empty state. |
| lib/pages/device.dart | Refactor footer builder to single widget; pass footer into BaseDevice.showInformation; tweak divider styling. |
| lib/pages/customize.dart | Show keymap explanation when proxy trainer connected; localize “Open connection settings” copy; add “no trainer selected” warning path. |
| lib/pages/configuration.dart | Treat BikeControl trainer app as self-hosted target; hide target picker; set last target to this device. |
| lib/pages/subscriptions/login.dart | Add mail fallback CTA with debug payload via mailto: link. |
| lib/pages/proxy_device_details/virtual_shifting_pro_notice.dart | New pro notice widget for virtual shifting budget. |
| lib/pages/proxy_device_details/metric_card.dart | New metric card UI component. |
| lib/pages/proxy_device_details/live_metrics_section.dart | New live metrics section binding to active bike definition. |
| lib/pages/proxy_device_details/gear_ratio_curve.dart | New UI visualization for gear ratio curve. |
| lib/pages/support_chat/widgets/support_message_group.dart | New grouping widget for consecutive sender messages. |
| lib/pages/support_chat/widgets/support_attachment_view.dart | New attachment renderer with signed URL caching and image previews. |
| lib/models/user_settings.dart | Add shifting configs column to synced settings model. |
| lib/main.dart | Initialize shifting configs; start review prompt service; add divider theming wrapper. |
| lib/bluetooth/messages/notification.dart | Remove log-entry accumulation side effect from LogNotification constructor. |
| lib/bluetooth/remote_pairing.dart | Update getTile signature to accept small; adjust imports. |
| lib/bluetooth/remote_keyboard_pairing.dart | Update getTile signature to accept small; adjust imports. |
| lib/bluetooth/devices/trainer_connection.dart | Add virtualShiftingTransport mapping and getTile({small}); export ConnectionMethodType. |
| lib/bluetooth/devices/bluetooth_device.dart | Add device info fields (fw/hw/mfg/name); handle MTU request errors; always allow ProxyDevice; improve device-info reads. |
| lib/bluetooth/devices/zwift/zwift_device.dart | Add semantic firmware comparison via version package. |
| lib/bluetooth/devices/zwift/zwift_click.dart | Add controller layout positions. |
| lib/bluetooth/devices/zwift/zwift_play.dart | Add controller layout + SVG contour support. |
| lib/bluetooth/devices/zwift/zwift_ride.dart | Add controller layout for combined Play silhouette; track initialization time. |
| lib/bluetooth/devices/zwift/zwift_emulator.dart | Update getTile({small}) signature. |
| lib/bluetooth/devices/zwift/ftms_mdns_emulator.dart | Update getTile({small}) signature. |
| lib/bluetooth/devices/openbikecontrol/obc_mdns_emulator.dart | Switch to new transporter/definition; define transport type; getTile({small}). |
| lib/bluetooth/devices/openbikecontrol/obc_ble_emulator.dart | Define transport type; getTile({small}). |
| lib/bluetooth/devices/openbikecontrol/obc_bike_definition.dart | Replace DirCon dependency with BleDefinition implementation. |
| lib/bluetooth/devices/mywhoosh/link.dart | Update getTile({small}) signature. |
| lib/bluetooth/devices/hid/hid_device.dart | Ensure HID devices mark connected; refactor info/additional info separation; allow footer injection. |
| lib/bluetooth/devices/sram/sram_axs.dart | Move long explanatory text into showAdditionalInformation. |
| lib/bluetooth/devices/shimano/shimano_di2.dart | Move hint text into showAdditionalInformation. |
| lib/bluetooth/devices/gyroscope/gyroscope_steering.dart | Add controller layout; move meta info into showMetaInformation. |
| lib/bluetooth/devices/elite/elite_sterzo.dart | Add controller layout. |
| lib/bluetooth/devices/cycplus/cycplus_bc2.dart | Add controller layout + button colors. |
| lib/bluetooth/devices/wahoo/wahoo_kickr_bike_shift.dart | Add controller layout. |
| lib/bluetooth/devices/thinkrider/thinkrider_vs200.dart | Add controller layout + button colors. |
| lib/bluetooth/ble.dart | Add Generic Access + additional Device Info UUID constants. |
| lib/bluetooth/devices/base_device.dart | Support controller layouts + unified info rendering with footer injection and StatusIcon. |
| lib/pages/unlock.dart | Persist “likely unlocked” flag when marking unlocked. |
| ios/ExportOptions.plist | Remove manageAppVersionAndBuildNumber key. |
| assets/contours/zwift_play.svg | Add controller contour SVG asset. |
| prop_public/lib/prop.dart | Export new dircon_emulator.dart. |
| prop_public/lib/emulators/ble_definition.dart | Add new BleDefinition abstraction. |
| prop_public/lib/emulators/dircon_emulator.dart | Add DirconEmulator + RetrofitMode API surface (currently stubbed). |
| prop_public/lib/emulators/definitions/proxy_bike_definition.dart | Add ProxyBikeDefinition abstraction (currently incomplete). |
| prop_public/lib/emulators/transporter/transporter.dart | Add Transporter base class. |
| prop_public/lib/emulators/transporter/network_transporter.dart | Add NetworkTransporter (currently incomplete). |
| prop_public/lib/emulators/transporter/bluetooth_transporter.dart | Add BluetoothTransporter (currently incomplete). |
| prop_public/lib/services/bridge_usage_tracker.dart | Add BridgeUsageTracker API (currently stubbed). |
| prop_public/lib/emulators/prefs.dart | Add “not sure if unlocked” preference hooks (currently stubbed). |
| ios_receipt/* | Remove vendored ios_receipt plugin package and associated platform code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+18
| BridgeUsageTracker({required this.prefs, required this.dailyLimit}); | ||
|
|
||
| ValueListenable<Duration> get usedTodayListenable => ValueNotifier(dailyLimit); | ||
|
|
||
| bool get isExhausted => false; | ||
|
|
||
| get onBudgetExhausted => null; | ||
|
|
||
| void startSession({required bool Function() isActive}) {} | ||
|
|
||
| void stopSession() {} |
There was a problem hiding this comment.
BridgeUsageTracker is currently a stub: usedTodayListenable creates a fresh ValueNotifier each call, onBudgetExhausted returns null (so .listen(...) in ProxyDevice will crash), and isExhausted/startSession/stopSession never enforce the daily limit. This needs a real implementation with a stable ValueNotifier<Duration>, a typed Stream<void> for budget exhaustion, and persistence keyed by date in SharedPreferences.
Comment on lines
+1
to
+2
| import 'package:flutter/src/foundation/change_notifier.dart'; | ||
| import 'package:shared_preferences/shared_preferences.dart'; |
There was a problem hiding this comment.
Avoid importing Flutter internals (package:flutter/src/...). Use the public API (package:flutter/foundation.dart) for ValueListenable/ValueNotifier/ChangeNotifier types to prevent breakage across Flutter upgrades.
Comment on lines
+3
to
+4
| import 'package:universal_ble/src/models/ble_device.dart'; | ||
| import 'package:universal_ble/src/models/ble_service.dart'; |
There was a problem hiding this comment.
This file imports universal_ble via package:universal_ble/src/..., which is a private path and can break without notice. Import the public library (package:universal_ble/universal_ble.dart) and use its exported model types instead.
Comment on lines
+9
to
+12
| @override | ||
| void sendCharacteristicNotification(String characteristicUUID, List<int> data, {int responseCode = 1}) { | ||
| // TODO: implement sendCharacteristicNotification | ||
| } |
There was a problem hiding this comment.
sendCharacteristicNotification is left unimplemented. OpenBikeControlMdnsEmulator now constructs a NetworkTransporter and will need this to send notifications/responses back to the client; leaving this as a TODO will cause runtime failures when bridging. Implement the DirCon wire framing + socket writes (and handle socket disposal/errors).
Comment on lines
+11
to
+14
| @override | ||
| void sendCharacteristicNotification(String characteristicUUID, List<int> data, {int responseCode = 1}) { | ||
| // TODO: implement sendCharacteristicNotification | ||
| } |
There was a problem hiding this comment.
sendCharacteristicNotification is left unimplemented. If BluetoothTransporter is used for BLE-peripheral mode, notifications will never be delivered to connected clients. Implement the platform peripheral notification path (or remove this transporter until implemented) to avoid silently broken virtual shifting.
Comment on lines
+199
to
+205
| if (!_isSmartTrainer) return RetrofitMode.proxy; | ||
| final transport = core.logic.preferredBridgeTransport(core.logic.enabledTrainerConnections); | ||
| return switch (transport) { | ||
| TrainerConnectionType.bluetooth => RetrofitMode.bluetooth, | ||
| TrainerConnectionType.wifi => RetrofitMode.wifi, | ||
| null => RetrofitMode.wifi, | ||
| }; |
There was a problem hiding this comment.
defaultRetrofitMode’s doc says it should fall back to Proxy when no Bridge transport is enabled, but the null transport case currently returns RetrofitMode.wifi. This will default users into a mode that may not work (and bypass the intended fallback). Update the null branch to match the documented behavior (likely RetrofitMode.proxy).
Comment on lines
+21
to
+66
| class DirconEmulator { | ||
| final ValueNotifier<bool> isStarted = ValueNotifier(false); | ||
| final ValueNotifier<bool> isConnected = ValueNotifier(false); | ||
| final ValueNotifier<bool> isUnlocked = ValueNotifier(false); | ||
| final ValueNotifier<bool> alreadyUnlocked = ValueNotifier(false); | ||
| final ValueNotifier<bool> waiting = ValueNotifier(false); | ||
| final ValueNotifier<String> data = ValueNotifier(''); | ||
|
|
||
| final ValueNotifier<RetrofitMode> retrofitMode = ValueNotifier(RetrofitMode.proxy); | ||
|
|
||
| DateTime? connectionDate; | ||
|
|
||
| BleDefinition? get activeDefinition => null; | ||
|
|
||
| String get advertisementName => 'null'; | ||
|
|
||
| List<BleService>? get services => []; | ||
|
|
||
| set trainerName(String Function() trainerName) {} | ||
|
|
||
| set shouldAdvertise(bool Function() shouldAdvertise) {} | ||
|
|
||
| set isTrial(bool Function() isTrial) {} | ||
|
|
||
| set onFitnessBikeDefinitionCreated(void Function(FitnessBikeDefinition def) onFitnessBikeDefinitionCreated) {} | ||
|
|
||
| Future<void>? pauseAdvertising() async {} | ||
|
|
||
| void setScanResult(BleDevice scanResult) {} | ||
|
|
||
| void handleServices(List<BleService> services) {} | ||
|
|
||
| Future<void> startServer() async {} | ||
|
|
||
| bool processCharacteristic(String characteristic, Uint8List bytes) { | ||
| return false; | ||
| } | ||
|
|
||
| void stop() {} | ||
|
|
||
| void setRetrofitMode(RetrofitMode savedMode) {} | ||
|
|
||
| Future<void> switchRetrofitMode(RetrofitMode next) async {} | ||
|
|
||
| void debugSetActiveDefinition(FitnessBikeDefinition def) {} | ||
| } |
There was a problem hiding this comment.
DirconEmulator is a placeholder (methods are empty and activeDefinition is always null), but it’s instantiated and used by ProxyDevice and ZwiftClickV2 (e.g. startServer(), processCharacteristic(), retrofitMode, isConnected/isStarted). In this state, proxy/virtual-shifting flows and the new tests will not work. Please replace this stub with a functional implementation (or remove it from the app build) before merging.
Comment on lines
+16
to
+46
| @override | ||
| // TODO: implement advertiseServiceUUIDs | ||
| List<String> get advertiseServiceUUIDs => throw UnimplementedError(); | ||
|
|
||
| @override | ||
| List<BleCharacteristic> getCharacteristics(String serviceUUID) { | ||
| // TODO: implement getCharacteristics | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| void onNotification(String characteristic, Uint8List bytes) { | ||
| // TODO: implement onNotification | ||
| } | ||
|
|
||
| @override | ||
| void onWriteRequest(String characteristicUUID, List<int> characteristicData) { | ||
| // TODO: implement onWriteRequest | ||
| } | ||
|
|
||
| @override | ||
| // TODO: implement serviceUUIDs | ||
| List<String> get serviceUUIDs => throw UnimplementedError(); | ||
|
|
||
| get powerW => null; | ||
|
|
||
| get heartRateBpm => null; | ||
|
|
||
| get cadenceRpm => null; | ||
|
|
||
| get speedKph => null; |
There was a problem hiding this comment.
ProxyBikeDefinition currently throws UnimplementedError for required BLE definition APIs (serviceUUIDs, advertiseServiceUUIDs, getCharacteristics) and returns null for metric getters without types. Since TrainerMetrics.fromDefinition expects ValueListenable fields from ProxyBikeDefinition, this will either crash or break typing at runtime. Please implement these members (or remove ProxyBikeDefinition from supported definitions until it’s complete).
Comment on lines
+26
to
+30
| void setNotSureIfUnlocked(String deviceId, bool bool) {} | ||
|
|
||
| bool notSureIfUnlocked(String deviceId) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
setNotSureIfUnlocked is a no-op and notSureIfUnlocked always returns false, but ZwiftClickV2.isLikelyUnlocked and the unlock UX rely on this flag. Persist this value in _prefs (e.g. clickV2_<id>_likely_unlocked) and rename the parameter from bool to something like value to avoid shadowing/confusion.
Comment on lines
+65
to
+76
| /// Resolves which concrete [RetrofitMode] the Virtual Shifting radio will | ||
| /// switch into when picked. Mirrors the active Trainer Connections — BT wins | ||
| /// over WiFi. Returns `null` when neither transport is enabled, in which | ||
| /// case the VS radio renders disabled and the missing-transport hint shows. | ||
| RetrofitMode get _resolvedVirtualShiftingMode { | ||
| final transport = core.logic.preferredBridgeTransport(core.logic.enabledTrainerConnections); | ||
| return switch (transport) { | ||
| TrainerConnectionType.bluetooth => RetrofitMode.bluetooth, | ||
| TrainerConnectionType.wifi => RetrofitMode.wifi, | ||
| null => RetrofitMode.wifi, | ||
| }; | ||
| } |
There was a problem hiding this comment.
_resolvedVirtualShiftingMode is documented/used as if it can be null when no Bluetooth/WiFi trainer connection is enabled, but it’s declared non-nullable and returns RetrofitMode.wifi for the null transport case. This makes the Virtual Shifting option appear available even when no transport is enabled and contradicts the hint text. Change this getter to return RetrofitMode? and propagate the null case to disable the VS radio / show the missing-transport hint.
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.
No description provided.