feat: added usb transfer support (Android only) #1473
Conversation
Reviewer's GuideThis PR integrates Android-only USB CDC/ACM serial communication alongside existing BLE transfers by reusing the BadgeMagic data protocol (but with 64-byte chunks), adds a transfer method UI with a provider, updates Android manifest and resources for USB host support, and switches to a modern usb_serial SDK-34+ fork. Sequence diagram for USB transfer processsequenceDiagram
actor User
participant App
participant TransferProvider
participant UsbCdc
User->>App: Tap "Transfer" button
App->>TransferProvider: openTray()
User->>App: Select "USB" in tray
App->>TransferProvider: selectMethod(ConnectionType.usb)
App->>UsbCdc: openDevice()
UsbCdc-->>App: Device opened
App->>UsbCdc: write(data in 64-byte chunks)
UsbCdc-->>App: Write success/failure
App->>User: Show transfer result
Class diagram for new and updated USB transfer classesclassDiagram
class TransferProvider {
- ConnectionType? _selectedMethod
- bool _showTray
+ openTray()
+ closeTray()
+ selectMethod(ConnectionType)
+ reset()
+ selectedMethod
+ showTray
}
class PayloadBuilder {
- DataTransferManager manager
+ buildPayloads()
}
class UsbCdc {
- UsbPort? _port
+ openDevice()
+ write(List<int>)
+ close()
+ listDevices()
+ isFossasiaDeviceConnected()
}
class UsbWriteState {
- PayloadBuilder builder
+ processState()
}
class UsbScanState {
- PayloadBuilder builder
+ processState()
}
TransferProvider <|.. AnimationBadgeProvider
PayloadBuilder --> DataTransferManager
UsbWriteState --> PayloadBuilder
UsbScanState --> PayloadBuilder
UsbWriteState --> UsbCdc
UsbScanState --> UsbCdc
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The HomeScreen widget is becoming very large and complex with USB transfer logic and UI baked in—extract the transfer tray and handleTransfer flow into smaller widgets or a dedicated controller/service to simplify maintenance.
- The handleAnimationTransfer method in AnimationBadgeProvider now has a big BLE vs. USB conditional; refactor by pulling out the USB-specific and BLE-specific flows into separate helper methods to clean up the logic.
- TransferMethodTray references colorPrimaryDark which isn’t defined in constants.dart—either define this color or switch to an existing constant like colorPrimary to fix the undefined reference.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The HomeScreen widget is becoming very large and complex with USB transfer logic and UI baked in—extract the transfer tray and handleTransfer flow into smaller widgets or a dedicated controller/service to simplify maintenance.
- The handleAnimationTransfer method in AnimationBadgeProvider now has a big BLE vs. USB conditional; refactor by pulling out the USB-specific and BLE-specific flows into separate helper methods to clean up the logic.
- TransferMethodTray references colorPrimaryDark which isn’t defined in constants.dart—either define this color or switch to an existing constant like colorPrimary to fix the undefined reference.
## Individual Comments
### Comment 1
<location> `lib/view/homescreen.dart:622` </location>
<code_context>
+ ),
+ ),
+ // Transfer Method Tray Overlay
+ Consumer<TransferProvider>(
+ builder: (context, transferProvider, _) {
+ if (transferProvider.showTray) {
</code_context>
<issue_to_address>
**issue (bug_risk):** The overlay for the transfer tray may block all interactions, including the tray itself.
The overlay's full-screen coverage may block tray interactions. Adjust the Stack order or hitTestBehavior to keep the tray accessible.
</issue_to_address>
### Comment 2
<location> `lib/providers/animation_badge_provider.dart:285-294` </location>
<code_context>
+ if (connectionType == ConnectionType.bluetooth) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The USB transfer path does not reset animation state for special animations.
The BLE path resets animation mode and index for certain animations, but the USB path does not. This may cause inconsistent UI states. Please align the USB logic with BLE for animation state resets.
</issue_to_address>
### Comment 3
<location> `lib/providers/animation_badge_provider.dart:335-344` </location>
<code_context>
+ Future<void> write(List<int> data) async {
+ if (_port == null) throw Exception("USB port not open");
+
+ try {
+ await _port!.write(Uint8List.fromList(data));
+ logger.d("USB chunk written: ${data.length} bytes");
+ } catch (e) {
+ logger.e("Failed to write USB chunk: $e");
+ rethrow;
</code_context>
<issue_to_address>
**suggestion:** Error handling for USB transfer is limited to logging.
Please add user-facing error notifications for USB transfer failures to match the BLE error handling approach.
</issue_to_address>
### Comment 4
<location> `lib/bademagic_module/usb/usb_cdc.dart:35-37` </location>
<code_context>
+ logger.d("Found FOSSASIA device: ${device.vid}:${device.pid}");
+
+ // ✅ ADD THIS BOOTLOADER DETECTION
+ if (device.vid == bootloaderVendorId && device.pid == bootloaderProductId) {
+ logger.e("Device is in bootloader mode - cannot transfer data");
+ throw Exception("Device is in bootloader mode. Please disconnect, then connect without holding any buttons.");
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Throwing an exception for bootloader mode may not be user-friendly.
Display a user-facing message explaining the bootloader mode before throwing the exception to clarify why the transfer is blocked.
Suggested implementation:
```
if (device.vid == bootloaderVendorId && device.pid == bootloaderProductId) {
logger.e("Device is in bootloader mode - cannot transfer data");
// Show user-facing message before throwing exception
// NOTE: Replace `context` with your actual BuildContext variable if needed
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
"Device is in bootloader mode. Please disconnect, then connect without holding any buttons. Data transfer is blocked in bootloader mode.",
),
backgroundColor: Colors.red,
duration: Duration(seconds: 5),
),
);
throw Exception("Device is in bootloader mode. Please disconnect, then connect without holding any buttons.");
}
```
- You must ensure that a valid `BuildContext` named `context` is available at this point in your code. If this is not inside a widget or you do not have access to `context`, you will need to refactor the code to pass it in, or use another user-facing notification mechanism appropriate for your app.
- Import `package:flutter/material.dart` if not already imported.
- If you use another UI framework, replace the `ScaffoldMessenger`/`SnackBar` code with the appropriate user notification method.
</issue_to_address>
### Comment 5
<location> `lib/view/widgets/transfer_method_tray.dart:17` </location>
<code_context>
+ @override
+ Widget build(BuildContext context) {
+ return Container(
+ height: MediaQuery.of(context).size.height / 3.8,
+ decoration: BoxDecoration(
+ color: Theme.of(context).dialogBackgroundColor,
</code_context>
<issue_to_address>
**suggestion:** Tray height is hardcoded as a fraction of screen height.
This approach may cause layout issues on devices with atypical screen sizes. Consider setting min/max height constraints or making the tray height responsive to its content.
Suggested implementation:
```
return Container(
constraints: BoxConstraints(
minHeight: 200,
maxHeight: MediaQuery.of(context).size.height * 0.6,
),
decoration: BoxDecoration(
color: Theme.of(context).dialogBackgroundColor,
borderRadius: const BorderRadius.only(
topLeft: Radius.circular(16),
topRight: Radius.circular(16),
),
boxShadow: [
BoxShadow(
color: Colors.black.withOpacity(0.2),
blurRadius: 8,
offset: const Offset(0, -2),
),
```
If the tray's child widgets are not already responsive to their content, consider wrapping the main content inside the tray with an `IntrinsicHeight` or using a `Column` with `MainAxisSize.min` to ensure the tray adapts to its content. Adjust the minHeight and maxHeight values as appropriate for your design.
</issue_to_address>
### Comment 6
<location> `lib/providers/transfer_provider.dart:22-14` </location>
<code_context>
+ notifyListeners();
+ }
+
+ void selectMethod(ConnectionType method) {
+ _selectedMethod = method;
+ _showTray = false;
+ notifyListeners();
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Selecting a method closes the tray immediately.
If transfer initiation is delayed, users may not receive immediate feedback. Consider keeping the tray open until the transfer begins or add a loading indicator.
Suggested implementation:
```
ConnectionType? _selectedMethod;
bool _showTray = false;
bool _isLoading = false;
ConnectionType? get selectedMethod => _selectedMethod;
bool get showTray => _showTray;
bool get isLoading => _isLoading;
void openTray() {
_showTray = true;
notifyListeners();
```
```
void selectMethod(ConnectionType method) {
_selectedMethod = method;
notifyListeners();
}
void startTransfer() {
_isLoading = true;
_showTray = false;
notifyListeners();
}
```
You will need to update the UI code to:
- Call `startTransfer()` when the transfer is actually initiated.
- Show a loading indicator when `isLoading` is true.
- Ensure the tray remains open after method selection, and only closes when transfer starts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
for all other platforms, app will work, but when you press transfer via usb, it will say Platform unsupported. |
|
@tushar11kh Please address all the suggestions given by the sourcey AI, and implement wherever necessary. |
mariobehling
changed the title
|
I believe I implemented all suggestions given by the sourcey AI. |
|
@mariobehling @samruddhi-Rahegaonkar |
|
Hey @tushar11kh, please remove the unrelated file changes like |
tushar11kh
force-pushed
the
android_usb_transfer
branch
from
26d2b83 to
5ed638a
Compare
tushar11kh
force-pushed
the
android_usb_transfer
branch
from
8e296d2 to
092ddb4
Compare
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/18465162558/artifacts/4254303608. ScreenshotsAndroid Screenshots
iPhone Screenshots
iPad Screenshots
|
||||||||||||||||||||||||
-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}
|
@mariobehling @samruddhi-Rahegaonkar @nope3472
|
tushar11kh
force-pushed
the
android_usb_transfer
branch
from
1f6a915 to
9e3f079
Compare
tushar11kh
force-pushed
the
android_usb_transfer
branch
from
9e3f079 to
29dc788
Compare
There was a problem hiding this comment.
Hi @tushar11kh Thanks for the updates and follow-up commits! I rechecked the PR. Please also note that this review was AI-assisted — I recommend you also use AI tools to double-check your next revision for potential Android USB permission or manifest issues.
🔴 Needs Fixing Before Merge
-
Manifest intent placement:
The<intent-filter>forandroid.hardware.usb.action.USB_DEVICE_ATTACHEDand<meta-data android:resource="@xml/device_filter">are still declared under<application>instead of the activity handling the intent.
→ Move both to the receiving<activity>. -
Play Store visibility:
<uses-feature android:name="android.hardware.usb.host" />is missingandroid:required="false".
→ Add this attribute so the app remains installable on devices without USB host support. -
Missing USB permission request:
The USB open flow still doesn’t explicitly request user permission (e.g.,UsbManager.requestPermission(...)).
→ Add the permission prompt and handle “permission denied” gracefully. -
Hard-coded IDs and baud rate:
The VID/PID and baud rate are still hard-coded inusb_cdc.dart.
→ Move these intoconstants.dartto centralize configuration. -
Typo in module folder name:
The directorylib/bademagic_module/usb/...is still missing the “g”.
→ Rename it tolib/badgemagic_module/...and fix imports. -
Platform guard for
UsbScanState:
UsbScanStatestill lacks a guard — it should not be created or called on non-Android builds.
→ Wrap inPlatform.isAndroidchecks or skip initialization. -
Intent handling UX:
The manifest declares USB attach handling, but no corresponding activity or user prompt flow is implemented.
→ Implement or document how attach/detach intents are handled. -
Dependency pinning:
The forkedusb_serialdependency should be pinned to a specific commit or tag.
→ Add a fixed reference and rationale comment inpubspec.yaml.
👍 Other-to-have Improvements
- Add a user-friendly UI message for bootloader mode instead of throwing directly.
- Add a short “USB Transfer (Android only)” section to the README and CHANGELOG.
🧪 Recommended Minimal Tests
- Chunking test: Ensure a 130-byte payload splits correctly into
[64, 64, 2]chunks.
|
@mariobehling thank you for the review. I appreciate the feedback, but I need to clarify several points where your AI assistant continues to misunderstand my implementation here. 1. Manifest Structure is CorrectThe USB intent filter and meta-data are properly placed inside 3. USB Permissions are HandledThe 5. Typo in module folder nameThe 6. Platform guard for UsbScanStateThe transfer method tray only appears on Android. On other platforms, the app automatically uses Bluetooth transfer, so USB code paths are never executed. This is cleaner than scattering platform checks throughout the codebase. I'll still add defensive checks in USB classes as belt-and-suspenders protection. 7. Intent handling UXThe USB attach intent is for automatic permission granting to recognized FOSSASIA devices, not for custom UI. The user prompt flow is handled by our Actual problems which are fixed now: 2. Play Store visibilityadded android:required="false" to USB host feature 4. Hard-coded IDs and baud rate:Hard-coded IDs: Moved to constants.dart 8. Dependency pinningPinned I need some manual checks as well , as AI is not covering all things here. |
4 participants
Fixes #986
Overview
This PR adds USB transfer capability to the Badge Magic app, allowing users to send animations and text to FOSSASIA badges via USB connection alongside the existing Bluetooth functionality.
What's Implemented
USB CDC/ACM Communication
badgemagic-firmwarereference.VID=0x10FC, PID=0x55E0VID=0x0416, PID=0x5020(taken frombadgemagic-rs)User Interface
Android Integration
AndroidManifest.xml.device_filter.xmlfor FOSSASIA device detection.Package Dependency Update
usb_serial: ^0.3.0(outdated, SDK 33)Technical Details
UsbScanStateandUsbWriteState.Testing Needed
Since I don’t have access to physical hardware, I need help with testing:
How to Test
Screenshots / Recordings
WhatsApp.Video.2025-09-15.at.04.08.05.mp4
Note
This implements only data transfer, not firmware flashing.
Checklist:
constants.dartwithout hard coding any value.Summary by Sourcery
Add USB transfer capability to the Badge Magic app by implementing USB CDC/ACM serial communication, a transfer method selection UI, and corresponding Android configuration and dependency updates
New Features:
Summary by Sourcery
Add USB transfer support on Android by implementing USB CDC/ACM communication, integrating it into the animation transfer flow, adding a transfer method UI and provider, and updating Android configurations and dependencies
New Features:
Build: