Add warning when bluetooth is off#161
Conversation
There was a problem hiding this comment.
Pull request overview
Adds user-facing UI copy and a scanner-screen banner to warn when Bluetooth is not powered on, aligning with Issue #156 (“If bluetooth is off when pairing warn user”).
Changes:
- Track
FlutterBluePlus.adapterStateinScannerScreenand show a “Bluetooth is off” warning banner when noton. - Add new English localization strings for the Bluetooth-off banner.
- Update
untranslated.jsonto reflect missing translations for the new keys across non-English locales.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| untranslated.json | Records untranslated localization keys for multiple locales after adding new scanner strings. |
| lib/screens/scanner_screen.dart | Subscribes to adapter state and conditionally renders a Bluetooth-off warning banner. |
| lib/l10n/app_en.arb | Adds English strings for the new Bluetooth-off banner (and an “Enable Bluetooth” label). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _bluetoothStateSubscription = FlutterBluePlus.adapterState.listen((state) { | ||
| if (mounted) { | ||
| setState(() { | ||
| _bluetoothState = state; | ||
| }); | ||
| // Cancel scan if Bluetooth turns off while scanning | ||
| if (state != BluetoothAdapterState.on && | ||
| connector.state == MeshCoreConnectionState.scanning) { |
There was a problem hiding this comment.
The adapter state listener only updates _bluetoothState. Per the PR description, turning Bluetooth off while scanning should cancel the scan, but no call to connector.stopScan() (or equivalent) happens when the adapter transitions away from on. Add logic here to stop an active scan when Bluetooth turns off and ensure connector state/UI stay consistent.
There was a problem hiding this comment.
This is an extreme edge case, probably not a high priority. I did find it myself when testing but I don't think the average user is going to turn off ble after opening the app and then wonder what happened.
But it would be a nice to have safety.
There was a problem hiding this comment.
Might be fixed? Not totally sure what the problem is if I didnt fix it but I think I got it
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I'll look closer and test on android tomorrow |
446564
left a comment
There was a problem hiding this comment.
The biggest thing I can see is the translations issues and the missing use of unawaited, the other things are nice to have though to prevent edge cases.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
How do we want to do translations? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Bluetooth off warning | ||
| if (_bluetoothState != BluetoothAdapterState.on) | ||
| _bluetoothOffWarning(context), |
There was a problem hiding this comment.
The warning banner currently shows for any adapter state other than on (e.g., unknown, turningOn, turningOff), which can incorrectly tell users “Bluetooth is off” during initialization or transitions. Restrict the banner to states that definitively indicate Bluetooth is unavailable (e.g., off/unavailable/unauthorized), or vary the text based on the specific state.
| Text( | ||
| context.l10n.scanner_bluetoothOff, | ||
| style: const TextStyle( | ||
| color: Colors.red, | ||
| fontWeight: FontWeight.w600, | ||
| fontSize: 14, | ||
| ), | ||
| ), | ||
| const SizedBox(height: 4), | ||
| Text( | ||
| context.l10n.scanner_bluetoothOffMessage, | ||
| style: TextStyle( |
There was a problem hiding this comment.
These new scanner_bluetoothOff* strings are referenced from code, but the checked-in generated localization outputs (lib/l10n/app_localizations*.dart) don’t contain these getters yet. Either regenerate and commit the updated localization Dart files (per the current repo layout), or stop checking the generated localization files into source control so they can’t drift out of sync.
…haoticLeah/meshcore-open into enhancement/bluetooth-disabled-warning
If you run |
…-disabled-warning
…app theme colors removed FAB overrides because material 3 does this for us, fixed missing translations.
|
@codex review |
|
I fixed a few things in this PR. The way you do translations is using the translate.py script in the tools folder. It uses a local AI agent to do the translations for us. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37db955ab2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Tested on linux and android
Linux:
Android:
What it does:
Someone should probably test IOS
Closes: #156