feat: Enhance Badge with Adjustable Brightness Support#1271
feat: Enhance Badge with Adjustable Brightness Support#1271samruddhi-Rahegaonkar wants to merge 2 commits intofossasia:developmentfrom
Conversation
Reviewer's Guide by SourceryThis pull request introduces brightness control functionality for both virtual and connected badges. It includes a toggle to enable/disable brightness control, a slider to set the LED intensity, and integration of brightness rendering on the virtual badge. The brightness level is managed using a provider, and the badge data transfer process has been updated to include brightness information. Sequence diagram for setting brightnesssequenceDiagram
participant User
participant HomeScreen
participant BadgeBrightnessProvider
participant BleBrightnessService
participant BluetoothDevice
User->HomeScreen: Adjusts brightness slider
HomeScreen->BadgeBrightnessProvider: setBrightness(brightnessLevel)
activate BadgeBrightnessProvider
BadgeBrightnessProvider->BleBrightnessService: setBrightness(brightnessLevel)
activate BleBrightnessService
alt isConnected == true
BleBrightnessService->BluetoothDevice: Writes brightness value
activate BluetoothDevice
BluetoothDevice-->>BleBrightnessService: Acknowledgment
deactivate BluetoothDevice
else isConnected == false
BleBrightnessService-->>BadgeBrightnessProvider: Returns false
end
BleBrightnessService-->>BadgeBrightnessProvider: Returns success
deactivate BleBrightnessService
BadgeBrightnessProvider-->>HomeScreen: Notifies listeners
deactivate BadgeBrightnessProvider
HomeScreen->HomeScreen: Rebuilds UI with new brightness
Updated class diagram for BadgeBrightnessProviderclassDiagram
class BadgeBrightnessProvider {
-BleBrightnessService _bleBrightnessService
-double _brightness
-bool _isConnected
-String _errorMessage
-bool _isBrightnessVisible
+double brightness
+bool isConnected
+String errorMessage
+bool isBrightnessVisible
+BadgeBrightnessProvider(BleBrightnessService bleBrightnessService)
-Future<void> _initialize()
+void toggleBrightnessVisibility(bool value)
+Future<bool> connectToDevice(String deviceId)
+Future<bool> setBrightness(double brightnessLevel)
+Future<void> getCurrentBrightness()
+Future<void> disconnect()
+void dispose()
}
class BleBrightnessService {
+Future<bool> connectToDevice(String deviceId)
+Future<bool> setBrightness(int brightnessLevel)
+Future<int?> getCurrentBrightness()
+Future<void> disconnect()
}
BadgeBrightnessProvider -- BleBrightnessService : depends on
Updated class diagram for BadgePaintclassDiagram
class BadgePaint {
-BadgeUtils badgeUtils
-List~List~bool~~ grid
-double brightness
+BadgePaint(List~List~bool~~ grid, double brightness)
+void paint(Canvas canvas, Size size)
+bool shouldRepaint(BadgePaint oldDelegate)
}
File-Level Changes
Assessment against 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 @samruddhi-Rahegaonkar - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a loading state to the UI while connecting to the badge, to improve user experience.
- The
YOUR_DEVICE_IDstring should be replaced with a persistent storage lookup.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| .getAnimationIndex()], | ||
| null, | ||
| false); | ||
| brightnessProvider.brightness.round() |
There was a problem hiding this comment.
issue (bug_risk): Suspicious cast from int to Map<String, dynamic>.
The call to badgeData.checkAndTransfer is passing brightnessProvider.brightness.round() casted to Map<String, dynamic>? even though brightness.round() produces an int. If the intent is to pass brightness as a parameter, consider wrapping it in a map (e.g., {"brightness": brightnessProvider.brightness.round()}) rather than casting an int. Ensure the parameter type expected by checkAndTransfer is correctly respected.
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/14384460232/artifacts/2920793486. 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
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@Jhalakupadhyay should i close PR then ? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@samruddhi-Rahegaonkar if we can add brightnesslevel in the json we transfer if possible will be good because making livesync with the firmware at the moment is not implemented and it will take time |
|
@nope3472 adding brightnessLevel to the JSON only sets the brightness once at the time of message transfer. |
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
So, it seems you understand it at the end. The brightness only needs to be set once. You do not need to connect to the app continuously. Please update this PR accordingly and share a short video showcasing the feature.
@mariobehling I will take this PR shortly after completing the open ones. |
|
This feature is still nice to have. Please update the PR or close it if you are no longer working on it. |
5 participants
Fixes #866
Changes
Screenshots / Recordings
video.mp4
Checklist:
constants.dartwithout hard coding any value.Summary by Sourcery
Add brightness control support for virtual and connected badges, enabling users to adjust LED intensity through a slider and toggle
New Features:
Enhancements: