Fix firmware update#179
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make firmware updates more reliable by strengthening BLE reconnection/setup behavior, and aligns the repo/tooling with the updated Flutter/Dart baseline while cleaning up some ancillary UI/export/test code.
Changes:
- Added a centralized BLE
reconnectAndSetupflow and used it during firmware-update verification / reconnect handling. - Updated sharing/export flows (Share Plus API) and made various UI/layout refactors.
- Removed the stray
windows/pubspec.yamland excludedwindows/**from analysis; pinned Dart/Flutter versions in CI and pubspec constraints.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| windows/pubspec.yaml | Deleted stray nested Flutter pubspec under windows/. |
| analysis_options.yaml | Excludes windows/** from analyzer to avoid analyzing stray subtree. |
| pubspec.yaml | Updates Dart/Flutter SDK environment constraints. |
| .github/workflows/build.yml | Pins Dart + Flutter versions used in CI builds. |
| lib/utils/bledata.dart | Introduces connection-monitor reference counting + reconnectAndSetup retry logic and callback fanout. |
| lib/widgets/device_header.dart | Routes reconnect callback via a stored function; adjusts monitor stop; removes firmware-update RSSI polling guard. |
| lib/screens/firmware_update_screen.dart | Improves firmware-update verification by actively reconnecting and re-requesting FW version; fixes listener cleanup. |
| lib/utils/workout/gpx_to_fit.dart | Refactors GPX extension parsing for HR/cad/power during GPX→FIT conversion. |
| lib/utils/workout/gpx_file_exporter.dart | Formatting + Share Plus API usage for export/share flows. |
| lib/widgets/completed_activities.dart | Formatting + Share Plus API usage; refactors list UI code. |
| lib/screens/ble_log_screen.dart | Formatting + Share Plus API usage; minor UI refactors. |
| lib/utils/preset_sharing.dart | Formatting + Share Plus API usage; minor preset import/export refactors. |
| lib/utils/presets.dart | Simplifies characteristic map checks when saving presets. |
| lib/utils/power_table_sharing.dart | Formatting + Share Plus API usage; minor refactors. |
| lib/widgets/power_table_chart.dart | Simplifies null handling when computing homing min/max. |
| lib/utils/workout/workout_controls.dart | UI formatting; replaces deprecated-ish color opacity call style. |
| lib/screens/workout_screen.dart | Formatting/layout refactors; replaces opacity call style. |
| test/widget_test.dart | Updates path_provider mocking approach; strengthens sanity assertion. |
| test/erg_workout_test.dart | Updates path_provider mocking approach; formatting improvements. |
| test/test.zwo | Text event copy tweaks (minutes/seconds wording). |
Comments suppressed due to low confidence (1)
lib/widgets/device_header.dart:148
- The periodic RSSI/firmware-version polling no longer pauses during firmware updates.
_updateRssi()triggersreadRssi()plusrequestSetting(..., fwVname)every 20s, which can interleave GATT operations with the OTA transfer and reintroduce the instability the removedisUpdatingFirmwareguard was preventing. Consider reintroducing an explicit “firmware update in progress” flag (or otherwise disabling these timers / hiding DeviceHeader on the firmware update screen) so no extra BLE reads/writes occur while updating.
Future<void> _updateRssi() async {
if (this.widget.device.isConnected) {
try {
this.bleData.rssi.value = await this.widget.device.readRssi();
bleData.requestSetting(this.widget.device, fwVname);
// No need for manual setState here anymore - the listener will handle firmware version updates
Comment on lines
196
to
+200
| await onReconnected(); | ||
| } else { | ||
| for (final callback in List<Future<void> Function()>.from( | ||
| _onReconnectedCallbacks)) { | ||
| await callback(); |
Comment on lines
91
to
113
| // Extract heart rate and cadence from TrackPointExtension | ||
| final tpExt = ext['gpxtpx:TrackPointExtension']; | ||
| if (tpExt is Map) { | ||
| final hr = tpExt['gpxtpx:hr']; | ||
| final cad = tpExt['gpxtpx:cad']; | ||
| if (hr != null) { | ||
| record.heartRate = int.tryParse(hr.toString()) ?? 0; | ||
| } | ||
| if (cad != null) { | ||
| record.cadence = int.tryParse(cad.toString()) ?? 0; | ||
| } | ||
| } else { | ||
| // Legacy/Fallback parsing logic | ||
| final tpExtStr = tpExt?.toString() ?? ''; | ||
| if (tpExtStr.isNotEmpty) { | ||
| // The TrackPointExtension string contains both HR and cadence values | ||
| // Split the string and extract the values | ||
| final values = tpExtStr.trim().split('\n').map((s) => s.trim()).where((s) => s.isNotEmpty).toList(); | ||
| if (values.length >= 2) { | ||
| record.heartRate = int.tryParse(values[0]) ?? 0; | ||
| record.cadence = int.tryParse(values[1]) ?? 0; | ||
| } | ||
| final tpExtStr = _extensionText(ext['gpxtpx:TrackPointExtension']); | ||
| final heartRate = _extractExtensionInt(tpExtStr, 'hr'); | ||
| final cadence = _extractExtensionInt(tpExtStr, 'cad'); | ||
| if (heartRate != null) { | ||
| record.heartRate = heartRate; | ||
| } | ||
| if (cadence != null) { | ||
| record.cadence = cadence; | ||
| } | ||
|
|
||
| if (tpExtStr != null && heartRate == null && cadence == null) { | ||
| final values = tpExtStr | ||
| .trim() | ||
| .split('\n') | ||
| .map((s) => s.trim()) | ||
| .where((s) => s.isNotEmpty) | ||
| .toList(); | ||
| if (values.length >= 2) { | ||
| record.heartRate = int.tryParse(values[0]) ?? 0; | ||
| record.cadence = int.tryParse(values[1]) ?? 0; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Implemented in c7f937b (with the parsing change introduced in 9a2f7db). gpx_to_fit.dart now handles map-backed gpxtpx:TrackPointExtension values again while keeping the XML/string fallback, and I added a regression test covering heart rate, cadence, and power preservation through GPX-to-FIT conversion.
Agent-Logs-Url: https://github.com/doudar/SS2kConfigApp/sessions/206f019a-7827-4538-a8ee-965f75d93964 Co-authored-by: doudar <17362216+doudar@users.noreply.github.com>
Agent-Logs-Url: https://github.com/doudar/SS2kConfigApp/sessions/206f019a-7827-4538-a8ee-965f75d93964 Co-authored-by: doudar <17362216+doudar@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
More robust re-connection