feat(ble): Handle invalid BLE attributes#4485
Merged
Merged
Conversation
This change introduces handling for `InvalidAttributeException` which can occur when GATT handles become stale, for instance, after a service change or an unexpected disconnect. When an `InvalidAttributeException` is caught during a read or write operation: - The existing BLE characteristics are cleared. - The service is notified of the disconnection with an `InvalidAttribute` error. - The `shouldReconnect` flag for this error is now set to `true`, allowing the connection to be re-established. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4485 +/- ##
=========================================
+ Coverage 9.22% 10.57% +1.34%
=========================================
Files 427 427
Lines 14332 14356 +24
Branches 2385 2386 +1
=========================================
+ Hits 1322 1518 +196
+ Misses 12757 12542 -215
- Partials 253 296 +43 ☔ View full report in Codecov by Sentry. |
This commit adjusts the log levels for several common and often transient error conditions to reduce noise in the logs. - Change many common BLE and TCP connection errors from `Error` to `Warning`. These failures are frequently due to environmental factors (e.g., device out of range) and don't necessarily indicate a bug. - Similarly, bonding failures are now logged as `Warning` because they can be caused by user actions or temporary connectivity issues. - Modify analytics to only record exceptions to Crashlytics for severities of `Error` or higher, preventing lower-severity warnings from cluttering crash reports. - Add a retry mechanism when starting the foreground service. If it fails with a `SecurityException` (possible on Android 14+ when requesting location from the background), it will attempt to restart without the location service type. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit updates the Nordic BLE library dependencies and refactors their usage.
Key changes include:
- Renaming `libs.nordic` to `libs.nordic.client.android` for clarity.
- Adding a new `nordic-ble` version variable in `libs.versions.toml`.
- Introducing mock dependencies for the Nordic BLE library (`client-android-mock`, `client-mock`, `core-android-mock`, `core-mock`).
- Implementing new test classes using these mocks:
- `BleOtaTransportNordicMockTest` to simulate and verify the OTA update flow.
- `NordicBleInterfaceTest` to test the connection and notification lifecycle.
- Modifying `RadioInterfaceService` and its `handleFromRadio` method to be `open` to allow for mocking in tests.
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit introduces a suite of new tests for the `NordicBleInterface` to improve test coverage and ensure its robustness. The new tests verify several key behaviors: - Writing data to the `toRadioCharacteristic` is successful. - The `onDisconnect` callback is correctly triggered when a peripheral disconnects. - Service discovery fails as expected if a required characteristic is missing. - An exception during a write operation correctly triggers a disconnect event. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Adds a new test suite, `NordicBleInterfaceRetryTest`, to verify the retry mechanism for BLE write operations in the `NordicBleInterface`. The tests cover two main scenarios: - A write operation that succeeds after one retry. - A write operation that ultimately fails and triggers a disconnect after exhausting the maximum number of retries. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Adds new test suites for BLE functionality using the Nordic BLE mock library. - **OTA Error Handling**: Introduces `BleOtaTransportErrorTest` to verify the correct handling of various Over-the-Air update failures, including hash rejection, connection loss during transfer, and verification mismatches. - **Packet Drain Logic**: Implements `NordicBleInterfaceDrainTest` to ensure that when a notification indicates multiple packets are available, the interface correctly reads and processes all of them until the queue is empty. - **Code Formatting**: Minor code style adjustment in `NordicBleInterfaceRetryTest`. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This pull request focuses on improving BLE error handling and logging, enhancing robustness for Bluetooth operations, and making the codebase more testable. The most significant changes include switching many BLE error logs from error to warning level, updating the BLE error recovery strategy for invalid attributes, handling Android foreground service startup failures more gracefully, and improving dependency management for testing.
Bluetooth error handling and robustness:
Logger.e(error) toLogger.w(warning) in multiple locations, reflecting that many BLE failures are transient and not always critical. This applies to both the Nordic and TCP interfaces. [1] [2] [3] [4] [5] [6] [7] [8] [9]InvalidAttributeerrors are now treated as recoverable by attempting a fresh connection and discovery, and characteristics are cleared when such errors occur. Added a dedicatedhandleInvalidAttributemethod for this logic. [1] [2] [3] [4]clearCharacteristics()method, which is called on disconnect or when attributes are invalidated. [1] [2]Foreground service startup improvements:
SecurityExceptionwhen starting a foreground service with location type on Android 14+: now retries with a reduced service type and logs appropriately, preventing unnecessary app crashes. [1] [2]Testability and dependency management:
RadioInterfaceServicefrom@Singletontoopen classand madehandleFromRadioopen, allowing easier mocking and subclassing in tests. [1] [2]app/build.gradle.kts.Other minor improvements: