feat: add badge scan mode selection and filtering support #1357
Conversation
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
Reviewer's GuideThis PR introduces a new BadgeScanProvider and BadgeScanMode enum, adds UI components for selecting scan modes and badge-name filters, and refactors BLE scan and data transfer logic to honor these user preferences throughout the app. Sequence diagram for BLE scan and data transfer with user scan preferencessequenceDiagram
actor User
participant SettingsScreen
participant BadgeScanProvider
participant HomeScreen
participant BadgeMessageProvider
participant ScanState
participant BLE
User->>SettingsScreen: Open settings
SettingsScreen->>BadgeScanProvider: setMode(mode), setBadgeNames(names)
User->>HomeScreen: Initiate data transfer
HomeScreen->>BadgeMessageProvider: checkAndTransfer(..., context)
BadgeMessageProvider->>BadgeScanProvider: get mode, get badgeNames
BadgeMessageProvider->>ScanState: new ScanState(manager, mode, allowedNames)
ScanState->>BLE: scan for devices (using mode, allowedNames)
BLE-->>ScanState: matching device found
ScanState->>BadgeMessageProvider: proceed with connection and transfer
Class diagram for BadgeScanProvider and BadgeScanModeclassDiagram
class BadgeScanProvider {
- BadgeScanMode _mode
- List<String> _badgeNames
+ BadgeScanMode get mode
+ List<String> get badgeNames
+ void setMode(BadgeScanMode mode)
+ void setBadgeNames(List<String> names)
+ void addBadgeName(String name)
+ void removeBadgeNameAt(int index)
+ void updateBadgeName(int index, String newName)
}
class BadgeScanMode {
<<enum>>
any
specific
}
BadgeScanProvider ..> BadgeScanMode : uses
Class diagram for ScanState changesclassDiagram
class ScanState {
+ DataTransferManager manager
+ BadgeScanMode mode
+ List<String> allowedNames
+ Future<BleState?> processState()
}
class BadgeScanMode {
<<enum>>
any
specific
}
ScanState ..> BadgeScanMode : uses
Class diagram for BadgeScanSettingsWidgetclassDiagram
class BadgeScanSettingsWidget {
+ Function(BadgeScanMode, List<String>) onSave
}
class _BadgeScanSettingsWidgetState {
- BadgeScanMode _mode
- List<TextEditingController> _controllers
+ void _addBadgeName()
+ void _removeBadgeName(int index)
+ void _onSave()
}
BadgeScanSettingsWidget o-- _BadgeScanSettingsWidgetState
_BadgeScanSettingsWidgetState ..> BadgeScanProvider : uses
_BadgeScanSettingsWidgetState ..> BadgeScanMode : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/16655413434/artifacts/3660498517. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
||||||||||||||||||||||||
-1_home_screen.png?raw=true)
-2_text_badge.png?raw=true){kind=icon}
-3_emoji_badge.png?raw=true){kind=icon}
-4_inverted_emoji_badge.png?raw=true){kind=icon}
-5_saved_badges.png?raw=true)
-6_saved_badges_clicked.png?raw=true)
-7_draw_badge.png?raw=true){kind=icon}
samruddhi-Rahegaonkar
force-pushed
the
issue1206
branch
from
1b2650f to
a1e36d3
Compare
samruddhi-Rahegaonkar
force-pushed
the
issue1206
branch
2 times, most recently
from
0b3bfe2 to
de1fa4f
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @samruddhi-Rahegaonkar - I've reviewed your changes - here's some feedback:
- You’re manually implementing the badge scan settings UI both in SettingsScreen and in BadgeScanSettingsWidget—extract and reuse one widget to DRY up that code.
- The new transferData/checkAndTransfer signatures scatter the BuildContext parameter—standardize its position (e.g. always first) to keep calls consistent and readable.
- In ScanState, make sure to cancel the scanResults subscription in finally (or after stopScan) to avoid any potential stream leaks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’re manually implementing the badge scan settings UI both in SettingsScreen and in BadgeScanSettingsWidget—extract and reuse one widget to DRY up that code.
- The new transferData/checkAndTransfer signatures scatter the BuildContext parameter—standardize its position (e.g. always first) to keep calls consistent and readable.
- In ScanState, make sure to cancel the scanResults subscription in finally (or after stopScan) to avoid any potential stream leaks.
## Individual Comments
### Comment 1
<location> `lib/providers/badge_message_provider.dart:97` </location>
<code_context>
- Future<void> transferData(DataTransferManager manager) async {
+ /// Transfers data to the badge via BLE using current scan settings.
+ Future<void> transferData(
+ DataTransferManager manager,
+ BuildContext context,
+ ) async {
+ final scanProvider = Provider.of<BadgeScanProvider>(context, listen: false);
</code_context>
<issue_to_address>
transferData now requires BuildContext, which may not always be available.
This dependency may limit usage in non-UI or background scenarios. Consider decoupling BLE logic from BuildContext, or clearly document this requirement.
</issue_to_address>
### Comment 2
<location> `lib/view/badgeScanSettingsWidget.dart:35` </location>
<code_context>
+ });
+ }
+
+ void _removeBadgeName(int index) {
+ setState(() {
+ _controllers.removeAt(index).dispose();
+ });
</code_context>
<issue_to_address>
Controller is disposed after removal from the list, which may cause issues.
Dispose the controller before removing it from the list to prevent potential access to a disposed controller during synchronous rebuilds.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> feat: add badge scan mode selection and filtering support fix: save and apply scan mode and badge names via provider fix: unused variables and imports fix: added comments fix: resolved conflicts
samruddhi-Rahegaonkar
force-pushed
the
issue1206
branch
from
78da202 to
f35b827
Compare
samruddhi-Rahegaonkar
requested review from
adityastic,
Jhalakupadhyay and
nope3472
nope3472
reviewed
Jul 2, 2025
|
@nope3472 I have tested it on the badge and i don't think there is an issue in both the states, have you tested it on the badge itself ? |
|
@nope3472 will you please tell me the exact issue like file or anything ? |
@nope3472 |
|
Tested. Works as expected. |
hpdang
approved these changes
Jul 15, 2025
Jhalakupadhyay
requested changes
Jul 15, 2025
| @@ -23,6 +24,7 @@ void main() { | |||
| providers: [ | |||
| ChangeNotifierProvider<InlineImageProvider>( | |||
| create: (context) => getIt<InlineImageProvider>()), | |||
| ChangeNotifierProvider(create: (_) => BadgeScanProvider()), | |||
There was a problem hiding this comment.
@samruddhi-Rahegaonkar try to avoid global providers if possible the BadgeScanProviders are only needed when there is ascanning or connecting to the badge happens rest of the files does not need it.
There was a problem hiding this comment.
However, for now I've kept it global to simplify access across the current implementation and avoid extra wiring.Since it's a lightweight provider and doesn’t hold heavy state or long-lived connections, the impact should be minimal.
| @@ -0,0 +1,36 @@ | |||
| import 'package:flutter/material.dart'; | |||
There was a problem hiding this comment.
@samruddhi-Rahegaonkar If we are saving the added badge using provider then if the app closes and starts it will be gone is this how this functionality is spossed to be ?
There was a problem hiding this comment.
Yeah! for now it should work like this if we need to maintain the persistant storage then we need to use shared preferences or any other alternative.
|
@samruddhi-Rahegaonkar update please |
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.
Fixes #1206
Changes
Screenshots / Recordings
Checklist:
constants.dartwithout hard coding any value.Summary by Sourcery
Enable configurable badge scan modes by allowing users to choose between scanning any badge or only badges with specified names, manage these preferences via a provider, and apply the filters during BLE scanning and data transfer.
New Features:
Enhancements: