Implement readRSSI on Android and Darwin #203
Conversation
Summary of ChangesHello @the-eric-kwok, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully implements the readRssi functionality for Android and Darwin platforms, while correctly providing 'not implemented' stubs for Windows, Linux, and Web. The changes are comprehensive, including updates to the public API, documentation, example application, and tests. The implementation across different platforms is consistent and well-structured. I have one suggestion for improvement in the Android implementation to enhance code clarity and efficiency.
android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt
Show resolved
Hide resolved
|
relation: |
fotiDim
left a comment
fotiDim
left a comment
There was a problem hiding this comment.
Thanks for the PR. Added some first comments. This won't probably make it for the 1.0 release which is due soon if it doesn't support all possible platforms.
| | enable/disable Bluetooth | ✔️ | ❌ | ❌ | ✔️ | ✔️ | ❌ | | ||
| | onAvailabilityChange | ✔️ | ✔️ | ✔️ | ✔️ | ✔️ | ✔️ | | ||
| | requestMtu | ✔️ | ✔️ | ✔️ | ✔️ | ✔️ | ❌ | | ||
| | readRssi | ✔️ | ✔️ | ✔️ | ❌ | ❌ | ❌ | |
There was a problem hiding this comment.
@the-eric-kwok the platforms that are marked as ❌ do they not support reading RSSI at all or are they just not implemented? If they can support it please mark them as 🚧
There was a problem hiding this comment.
@fotiDim As far as I know, they are not supportable. On these platform you can only read RSSI in device discovery phase. Not after they are connected.
See:
There was a problem hiding this comment.
@the-eric-kwok bluez seems to support it https://github.com/canonical/bluez.dart/blob/579c153de7692e7940b158cb2d3ecfe3bb4a3975/lib/src/bluez_device.dart#L156. I haven't tested it myself. Could you give it a try?
There was a problem hiding this comment.
Linux can be done in a separate PR. Merging...
There was a problem hiding this comment.
Pull request overview
This PR implements the readRssi functionality for Android and Darwin (iOS/macOS) platforms to read the signal strength (RSSI) of connected BLE devices, resolving issue #195.
- Adds platform-specific implementations for Android (Kotlin) and Darwin (Swift) to read RSSI values
- Implements proper error handling and future-based callbacks for asynchronous RSSI reading
- Adds stub implementations for unsupported platforms (Windows, Linux, Web) that return "not implemented" errors
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pigeon/universal_ble.dart | Adds readRssi method definition to platform channel interface |
| lib/src/universal_ble_pigeon/universal_ble.g.dart | Generated Dart platform channel code for readRssi method |
| lib/src/universal_ble_pigeon/universal_ble_pigeon_channel.dart | Implements readRssi in pigeon channel with error handling wrapper |
| lib/src/universal_ble_platform_interface.dart | Adds abstract readRssi method to platform interface |
| lib/src/universal_ble.dart | Adds public API method with comprehensive documentation for reading RSSI |
| lib/src/models/ble_device.dart | Adds convenience readRssi method to BleDevice class |
| lib/src/universal_ble_web/universal_ble_web.dart | Implements readRssi stub that throws "not implemented" exception for Web |
| lib/src/universal_ble_linux/universal_ble_linux.dart | Implements readRssi stub that throws "not implemented" exception for Linux |
| android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt | Implements readRssi using BluetoothGatt.readRemoteRssi() with proper callback handling and cleanup |
| android/src/main/kotlin/com/navideck/universal_ble/UniversalBleHelper.kt | Adds RssiResultFuture data class for managing async RSSI read operations |
| android/src/main/kotlin/com/navideck/universal_ble/UniversalBle.g.kt | Generated Kotlin platform channel setup code for readRssi |
| darwin/Classes/UniversalBlePlugin.swift | Implements readRssi using CBPeripheral.readRSSI() with proper future handling and cleanup |
| darwin/Classes/UniversalBleHelper.swift | Adds RssiReadFuture class for managing async RSSI read operations |
| darwin/Classes/UniversalBle.g.swift | Generated Swift platform channel setup code for readRssi |
| windows/src/universal_ble_plugin.h | Adds readRssi method declaration to Windows plugin |
| windows/src/universal_ble_plugin.cpp | Implements readRssi stub that returns "not implemented" error for Windows |
| windows/src/generated/universal_ble.g.h | Generated C++ platform channel interface for readRssi |
| windows/src/generated/universal_ble.g.cpp | Generated C++ platform channel implementation for readRssi |
| test/ble_characteristic_test.dart | Adds mock readRssi implementation returning -50 dBm |
| example/lib/data/mock_universal_ble.dart | Adds mock readRssi implementation for example app |
| example/lib/peripheral_details/peripheral_detail_page.dart | Adds "Get RSSI" button to example app UI |
| README.md | Adds "Reading RSSI" section with usage example, platform limitations, and fixes code block formatting |
| README.low_level.md | Adds "Read RSSI" section with usage example and platform limitations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| override fun readRssi(deviceId: String, callback: (Result<Long>) -> Unit) { | ||
| try { | ||
| val gatt = deviceId.toBluetoothGatt() | ||
| if (gatt.readRemoteRssi()) { | ||
| rssiResultFutureList.add(RssiResultFuture(deviceId, callback)) | ||
| } else { | ||
| callback( | ||
| Result.failure( | ||
| createFlutterError( | ||
| UniversalBleErrorCode.FAILED, | ||
| "Failed to read RSSI" | ||
| ) | ||
| ) | ||
| ) | ||
| } | ||
| } catch (e: FlutterError) { | ||
| callback(Result.failure(e)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing debug logging for readRssi operation. Other similar operations (requestMtu, readValue, writeValue, setNotifiable) include UniversalBleLogger.logDebug calls for consistency and debugging purposes. Consider adding a log statement like "READ_RSSI -> $deviceId" at the start of the function to match the logging pattern used in other operations.
| @override | ||
| Future<int> readRssi(String deviceId) async { | ||
| return -50; // Mock RSSI value | ||
| } |
There was a problem hiding this comment.
The readRssi functionality lacks test coverage. While mock implementations have been added to _UniversalBleMock and MockUniversalBle, there are no actual test cases that invoke and verify the readRssi behavior. Consider adding a test case in the test suite that calls UniversalBle.readRssi or bleDevice.readRssi and verifies the expected mock return value of -50.
| override fun onReadRemoteRssi(gatt: BluetoothGatt?, rssi: Int, status: Int) { | ||
| val deviceId = gatt?.device?.address ?: return | ||
| rssiResultFutureList.removeAll { | ||
| if (it.deviceId == deviceId) { | ||
| if (status == BluetoothGatt.GATT_SUCCESS) { | ||
| it.result(Result.success(rssi.toLong())) | ||
| } else { | ||
| it.result( | ||
| Result.failure( | ||
| createFlutterError( | ||
| UniversalBleErrorCode.FAILED, | ||
| "Failed to read RSSI" | ||
| ) | ||
| ) | ||
| ) | ||
| } | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing error logging in onReadRemoteRssi failure case. Other similar callbacks (onCharacteristicRead, onCharacteristicWrite) log errors using UniversalBleLogger.logError when operations fail. Consider adding error logging when status is not GATT_SUCCESS to maintain consistency with the codebase's error reporting pattern.
3 participants
This PR resolves issue #195 .
Note
Adds cross-platform
readRssisurface with native support on Android and iOS/macOS.readRssi(deviceId)to platform interface and generated Pigeon channels (UniversalBlePlatformChannel) across Android/Darwin/Windows gluereadRssiviaBluetoothGatt.readRemoteRssi()with result tracking and error handling; cleans up pending futures on disconnectreadRssiusingCBPeripheral.readRSSI()anddidReadRSSIdelegate; integrates with existing futures cleanupUniversalBleExceptionUniversalBle.readRssi(deviceId)andBleDevice.readRssi(); updates platform interface, pigeon channel, and mock/test implementationsperipheral_detail_page.dartWritten by Cursor Bugbot for commit cd051db. This will update automatically on new commits. Configure here.