Skip to content
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

Should not stop notifications/indications if not previously started #190

Closed
twyatt opened this issue Nov 16, 2021 · 3 comments · Fixed by #193
Closed

Should not stop notifications/indications if not previously started #190

twyatt opened this issue Nov 16, 2021 · 3 comments · Fixed by #193
Labels
Milestone

Comments

@twyatt
Copy link
Member

twyatt commented Nov 16, 2021

For Peripheral.observe, when subscribers goes from 1 to 0, then underlying notifications/indications are stopped. Although this will also occur even if starting notifications/indications previously failed, or if connection drops. In both cases, stopping notifications/indications should not be attempted (the underlying BLE system will automatically clear notifications/indications).

Because these I/O operations are occurring when they shouldn't (aren't needed), the following exception is being thrown from the observe flow:

Caused by com.juul.kable.GattRequestRejectedException
       at com.juul.kable.PeripheralKt.setCharacteristicNotification(PeripheralKt.java:471)
       at com.juul.kable.PeripheralKt.access$setCharacteristicNotification(PeripheralKt.java:1)
       at com.juul.kable.AndroidPeripheral.stopObservation$core_release(AndroidPeripheral.java:391)
       at com.juul.kable.AndroidPeripheral$stopObservation$1.invokeSuspend(AndroidPeripheral.java:12)
       at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(BaseContinuationImpl.java:33)
       at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.java:104)
       at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.java:571)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.java:750)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.java:678)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.java:665)
@twyatt twyatt added bug Something isn't working android apple javascript labels Nov 16, 2021
@twyatt twyatt added this to the 0.10.4 milestone Nov 16, 2021
@twyatt twyatt linked a pull request Nov 22, 2021 that will close this issue
@twyatt twyatt modified the milestones: 0.10.4, 0.11.0 Dec 4, 2021
@twyatt
Copy link
Member Author

twyatt commented Dec 5, 2021

A potential fix is available as a SNAPSHOT build (from #193):

repositories {
    maven("https://oss.sonatype.org/content/repositories/snapshots")
}

dependencies {
    implementation("com.juul.kable:core:0.10.3-issue-190-3-SNAPSHOT")
}

@ln-12
Copy link

ln-12 commented Dec 6, 2021

Looks good in my use case, thanks :)

@twyatt
Copy link
Member Author

twyatt commented Dec 7, 2021

On an internal project, noticed that some BLE connections stalled after app was in the background for a long period of time (with device coming in and out of range).

Not sure if it is a regression due to the changes in the SNAPSHOT mentioned above, but I'm going to do more testing before merging #193.

@twyatt twyatt modified the milestones: 0.11.0, 0.12.0 Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants