JuulLabs / able

Able: Android Bluetooth Low Energy library
Apache License 2.0
160 stars 11 forks source link

Retry not working #58

Closed andybdahl closed 4 years ago

andybdahl commented 4 years ago

It seems the checkConnection() function in Retry never completes, the line Able.verbose { "checkConnection → End" } never runs.

If I use Able without the Retry layer, everything works as expected.

twyatt commented 4 years ago

Could you provide logs leading up to where the retry stalls?

andybdahl commented 4 years ago

Sure, here they are

2020-04-30 09:34:10.743 28277-28399/com.metricorr.app V/Retry: checkConnection → Begin
2020-04-30 09:34:10.744 28277-28399/com.metricorr.app V/Retry: checkConnection → consumeEach → newState = 2
2020-04-30 09:34:10.744 28277-28399/com.metricorr.app D/Retry: checkConnection → consumeEach → STATE_CONNECTED
2020-04-30 09:34:10.744 28277-28399/com.metricorr.app V/Retry: checkConnection → consumeEach → Repeat
2020-04-30 09:34:10.746 28277-28401/com.metricorr.app V/Retry: checkConnection → Begin
2020-04-30 09:34:10.747 28277-28401/com.metricorr.app V/Retry: checkConnection → consumeEach → newState = 2
2020-04-30 09:34:10.747 28277-28401/com.metricorr.app D/Retry: checkConnection → consumeEach → STATE_CONNECTED
2020-04-30 09:34:10.747 28277-28401/com.metricorr.app V/Retry: checkConnection → consumeEach → Repeat
andybdahl commented 4 years ago

What is happening is when I'm already connected, then a call to Retry.writeDescriptor for example causes a checkConnection() to never finish, as the device is already connected at the time of calling.

twyatt commented 4 years ago

It appears that subscription is not honoring the cancel request:

https://github.com/JuulLabs-OSS/able/blob/03b83fcd5cec77eb717144f49922e1bc3e596ed4/retry/src/main/java/Retry.kt#L98-L101

It's possible it's a bug in the Coroutines library, as the version of Able that you're using depends on an old (experimental) version of Coroutines.

https://github.com/JuulLabs-OSS/able/blob/03b83fcd5cec77eb717144f49922e1bc3e596ed4/build.gradle#L10

We're going to release Able 1.0.0 soon which removes the retry module and replaces it with keep-alive module that keeps the connection alive for you (reconnects on disconnect). It will allow users of Able to decide how to handle I/O retries.

I'll try to publish a 1.0.0-SNAPSHOT soon so you can experiment with it. In the mean time, you can look through the develop branch (and it's commits) to see how the API has changed.

twyatt commented 4 years ago

1.0.0-SNAPSHOT has been published, can you please try the new keep-alive module and let me know if that fits your use-case, or if you find any bugs. Thank you!

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

dependencies {
    implementation "com.juul.able:keep-alive:1.0.0-SNAPSHOT"
}
twyatt commented 4 years ago

This should be fixed in 0.8.0, please re-open if you still see the issue. Thanks for reporting.