Conversation
… continuous gear shifting or volume adjustments
Updated version number and release date for 5.2.0, removed unreleased features section.
There was a problem hiding this comment.
Pull request overview
Release-oriented update for BikeControl 5.2.0, including MyWhoosh connectivity adjustments and new long-press behavior in controller handling.
Changes:
- Bump app version to 5.2.0+118 and add 5.2.0 release notes to the changelog.
- Add implicit “repeat single click on long press” behavior (Pro-only) in
BaseDevice, plus a corresponding long-press editor affordance. - Adjust MyWhoosh supported connection methods and reorder some connection/tile lists.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pubspec.yaml | Version bump to 5.2.0+118. |
| lib/utils/requirements/platform.dart | Initialize status to false instead of late. |
| lib/utils/keymap/apps/my_whoosh.dart | Add OBP mDNS as supported for MyWhoosh; toggle starring. |
| lib/utils/core.dart | Reorder trainer connection lists (enabled/connected/visible). |
| lib/pages/trainer.dart | Reorder “other tiles” rendering. |
| lib/pages/overview.dart | Add “more” menu with a close action (uses exit(0)). |
| lib/pages/device.dart | Layout tweak: spaceBetween on device header row. |
| lib/pages/button_edit.dart | Add “Repeat single click action” card for long-press trigger editing. |
| lib/bluetooth/devices/base_device.dart | Implement repeat timer + implicit repeat-on-long-press logic and cleanup hooks. |
| CHANGELOG.md | Add 5.2.0 entry with MyWhoosh partnership messaging. |
Comments suppressed due to low confidence (1)
lib/bluetooth/devices/base_device.dart:389
- In repeat-on-long-press mode,
performClick()will call_showCommandLimitAlert()whenever the daily limit is reached. With_repeatIntervalat 150ms this can spam notifications/toasts rapidly until the button is released. Consider stopping the repeat timer (or throttling alerts) as soon as_canExecuteCommand()becomes false during repeat.
void _startRepeatingSingleClick(ControllerButton button) {
_repeatTimer?.cancel();
// Fire immediately, then repeat periodically
unawaited(performClick([button], trigger: ButtonTrigger.singleClick));
_repeatTimer = Timer.periodic(_repeatInterval, (_) {
unawaited(performClick([button], trigger: ButtonTrigger.singleClick));
});
}
void _stopRepeatingSingleClick() {
_repeatTimer?.cancel();
_repeatTimer = null;
}
Future<void> performClick(
List<ControllerButton> buttonsClicked, {
ButtonTrigger trigger = ButtonTrigger.singleClick,
}) async {
for (final action in buttonsClicked) {
// Check IAP status before executing command
if (!_canExecuteCommand()) {
_showCommandLimitAlert();
continue;
}
final result = await core.actionHandler.performAction(
action,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,5 @@ | |||
| import 'dart:async'; | |||
| import 'dart:io'; | |||
There was a problem hiding this comment.
Importing dart:io in this page will break Flutter web builds (and the repo has a web build workflow). Consider moving the exit/Platform-specific logic behind a conditional import or a small platform-abstraction so the web target can compile.
| import 'dart:io'; |
| MenuButton( | ||
| leading: const Icon(Icons.power_settings_new_rounded), | ||
| onPressed: (c) { | ||
| core.connection.disconnectAll(); | ||
| core.connection.stop(); | ||
| exit(0); | ||
| }, | ||
| child: Text('Close BikeControl'), |
There was a problem hiding this comment.
disconnectAll() and stop() both return Future<void>, but they’re called without await and then exit(0) is executed immediately. This can prevent proper cleanup and may leave BLE state/resources in an inconsistent state. Make the handler async, await shutdown, and use a platform-appropriate close (avoid exit(0) on iOS/web).
| core.connection.stop(); | ||
| exit(0); | ||
| }, | ||
| child: Text('Close BikeControl'), |
There was a problem hiding this comment.
The menu label Close BikeControl is a new user-facing string but isn’t localized (most nearby text uses i18n). Please add this to the localization files and use context.i18n/AppLocalizations here for consistency.
| child: Text('Close BikeControl'), | |
| child: Text('${AppLocalizations.of(context).close} BikeControl'), |
| void _startRepeatingSingleClick(ControllerButton button) { | ||
| _repeatTimer?.cancel(); | ||
| // Fire immediately, then repeat periodically | ||
| unawaited(performClick([button], trigger: ButtonTrigger.singleClick)); | ||
| _repeatTimer = Timer.periodic(_repeatInterval, (_) { | ||
| unawaited(performClick([button], trigger: ButtonTrigger.singleClick)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Timer.periodic schedules performClick(...) via unawaited, so repeat ticks can overlap if performAction takes longer than _repeatInterval. This can queue up concurrent actions and cause out-of-order execution. Add a simple in-flight guard (skip ticks while a repeat click is running) or await the previous invocation before scheduling the next.
| Future<void> performDown( | ||
| List<ControllerButton> buttonsClicked, { | ||
| ButtonTrigger trigger = ButtonTrigger.longPress, | ||
| }) async { | ||
| for (final action in buttonsClicked) { | ||
| // Check IAP status before executing command | ||
| if (!_canExecuteCommand()) { | ||
| //actionStreamInternal.add(AlertNotification(LogLevel.LOGLEVEL_ERROR, _getCommandLimitMessage())); | ||
| continue; | ||
| } | ||
|
|
||
| // Check for implicit repeat single click mode | ||
| if (_shouldRepeatSingleClick(action)) { | ||
| _startRepeatingSingleClick(action); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
performDown() can receive multiple buttons (e.g., explicit long-press combos), but _startRepeatingSingleClick uses a single _repeatTimer and cancels any existing timer each time it’s called. If more than one button qualifies for repeat, only the last one will actually repeat. Either restrict implicit repeat to single-button long presses or track repeat timers per button.
| /// Whether a long press should implicitly repeat the single click action. | ||
| /// Active by default for Pro users when no explicit long press action is set. | ||
| bool _shouldRepeatSingleClick(ControllerButton button) { | ||
| if (!supportsLongPress) return false; | ||
| try { | ||
| if (!IAPManager.instance.isProEnabledForCurrentDevice) return false; | ||
| } catch (_) { | ||
| return false; | ||
| } | ||
| final longPressPair = core.actionHandler.supportedApp?.keymap.getKeyPair( | ||
| button, | ||
| trigger: ButtonTrigger.longPress, | ||
| ); | ||
| if (longPressPair != null && !longPressPair.hasNoAction) return false; | ||
| final singleClickPair = core.actionHandler.supportedApp?.keymap.getKeyPair( | ||
| button, | ||
| trigger: ButtonTrigger.singleClick, | ||
| ); | ||
| return singleClickPair != null && !singleClickPair.hasNoAction; | ||
| } |
There was a problem hiding this comment.
This PR introduces implicit repeat-on-long-press behavior for Pro users (_shouldRepeatSingleClick + repeat timer), but there are existing tests for long-press timing in test/base_device_long_press_behavior_test.dart and none cover the new repeat mode. Please add tests to validate: (1) repeat starts only when long-press has no explicit mapping and single-click does, (2) repeat stops on release, and (3) no repeat occurs for non‑Pro users.
| if (widget.trigger == ButtonTrigger.longPress) ...[ | ||
| Builder( | ||
| builder: (context) { | ||
| final singleClickPair = widget.keymap.getKeyPair( | ||
| _keyPair.buttons.first, | ||
| trigger: ButtonTrigger.singleClick, | ||
| ); | ||
| final singleClickLabel = singleClickPair != null && !singleClickPair.hasNoAction | ||
| ? singleClickPair.toString() | ||
| : null; | ||
| return SelectableCard( | ||
| icon: Icons.repeat, | ||
| title: Text('Repeat single click action'), | ||
| isActive: _keyPair.hasNoAction, | ||
| value: _keyPair.hasNoAction ? singleClickLabel : null, | ||
| onPressed: () { | ||
| if (!_keyPair.hasNoAction) { | ||
| _keyPair.physicalKey = null; | ||
| _keyPair.logicalKey = null; | ||
| _keyPair.modifiers = []; | ||
| _keyPair.touchPosition = Offset.zero; | ||
| _keyPair.inGameAction = null; | ||
| _keyPair.inGameActionValue = null; | ||
| _keyPair.androidAction = null; | ||
| _keyPair.command = null; | ||
| _keyPair.screenshotPath = null; | ||
| setState(() {}); | ||
| widget.onUpdate(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
The UI exposes “Repeat single click action” for long-press editing, but the underlying behavior is Pro-only (_shouldRepeatSingleClick checks Pro entitlement). As-is, non‑Pro users can enable this UI state but it won’t have any effect. Consider marking this SelectableCard as isProOnly: true (so it shows the Pro badge / paywall) or otherwise gating it based on subscription status.
| ### 5.2.0 (06-04-2026) | ||
| Great news for MyWhoosh users: MyWhoosh has partnered with BikeControl to provide official support for controller hardware. | ||
|
|
||
| - A new network-based connection method is now available on all platforms, replacing the previous “Link” connection method |
There was a problem hiding this comment.
Changelog says the new network-based method is “replacing the previous ‘Link’ connection method”, but the codebase still includes and exposes the MyWhoosh Link connection path (e.g., AppConnectionMethod.myWhooshLink, MyWhooshLinkTile, and core.whooshLink). Please clarify the wording (deprecated vs. replaced) to match actual behavior in 5.2.0.
| - A new network-based connection method is now available on all platforms, replacing the previous “Link” connection method | |
| - A new network-based connection method is now available on all platforms, deprecating the previous “Link” connection method |
No description provided.