-
Notifications
You must be signed in to change notification settings - Fork 817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Bluetooth Scan #2613
fix: Bluetooth Scan #2613
Conversation
Reviewer's Guide by SourceryThis pull request adds the necessary permissions and permission checks for Bluetooth scanning and connections. It ensures that the app requests and verifies the required Bluetooth permissions before performing any Bluetooth-related operations, enhancing the app's stability and user experience. Sequence diagram for Bluetooth scan and permission flowsequenceDiagram
participant User as User
participant App as BluetoothScanFragment
participant Permission as PSLabPermission
participant System as Android System
User->>App: Initiate Bluetooth scan
App->>Permission: checkPermissions(BLUETOOTH_PERMISSION)
Permission->>System: Check BLUETOOTH permissions
alt Permissions not granted
System-->>Permission: Permission denied
Permission->>System: Request permissions
System->>User: Show permission dialog
User->>System: Grant permissions
System-->>App: Permissions granted
else Permissions already granted
System-->>Permission: Permissions granted
Permission-->>App: Permissions OK
end
App->>System: Start Bluetooth discovery
System-->>App: Return discovered devices
App->>App: Update device list
Class diagram for PSLabPermission changesclassDiagram
class PSLabPermission {
-String[] allPermissions
-String[] csvPermissions
-String[] logPermissions
-String[] mapPermissions
+String[] bluetoothPermission
+List~String~ listPermissionsNeeded
+static final int BLUETOOTH_PERMISSION
+static PSLabPermission getInstance()
+boolean checkPermissions(Activity, int)
}
note for PSLabPermission "Added new bluetoothPermission array and BLUETOOTH_PERMISSION constant"
State diagram for Bluetooth scan processstateDiagram-v2
[*] --> CheckPermissions
CheckPermissions --> RequestPermissions: Permissions not granted
CheckPermissions --> StartScan: Permissions granted
RequestPermissions --> StartScan: User grants permissions
RequestPermissions --> [*]: User denies permissions
StartScan --> Scanning
Scanning --> DeviceFound: Device discovered
Scanning --> StopScan: User stops scan
DeviceFound --> ConnectDevice: User selects device
StopScan --> [*]
ConnectDevice --> [*]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @AsCress - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- The permission check logic appears to be inverted - devices are only added when permission is NOT granted (link)
Overall Comments:
- The Bluetooth permission check logic in BluetoothScanFragment.java is inverted - devices are only added when permission is NOT granted. This condition needs to be fixed.
- Empty else blocks with /**/ comments should properly handle permission denied cases and provide user feedback, rather than silently failing.
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 1 other issue
- 🟢 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.
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/12847981460/artifacts/2451663333 |
Fixes #2611
Changes
Screenshots / Recordings
Screen_recording_20250117_222604.webm
Checklist:
strings.xml
,dimens.xml
andcolors.xml
without hard coding any value.strings.xml
,dimens.xml
orcolors.xml
.Summary by Sourcery
Add Bluetooth permissions and checks for scanning and connections.
Bug Fixes:
Enhancements: