-
Notifications
You must be signed in to change notification settings - Fork 582
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
Thread calling the Native API changed from main to background. #255
Conversation
Initially when working with the Android Bluetooth API (18 / Android 4.3) it was found that specific implementations (Samsung’s) cannot work correctly if called directly in callbacks from other calls. To mitigate the problem the next interaction was routed to Android main thread. Now when investigating the issue it turned out that it may be any other thread as long as it is not the callback thread.
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.
I think that this change required a brief explanation in readme/changelog about your findings.
return new ClientComponentFinalizer() { | ||
@Override | ||
public void onFinalize() { | ||
interactionExecutorService.shutdown(); |
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.
Actually this is incorrect. Dagger will provide you a new instance of the executor each time you ask for it. You seems to be closing an instance that wasn't even started and you're leaving started service as is.
Provided executors should be scoped.
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.
From what I see the executors are scoped. And .shutdown()
is executed in RxBleClient.finalize()
. Or am I missing something?
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.
You are correct, I missed the annotation @ClientScope
Naturally this change will be described in the Changelog. |
# Conflicts: # rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/RxBleConnectionImpl.java
Initially when working with the Android Bluetooth API (18 / Android 4.3) it was found that specific implementations (Samsung’s) cannot work correctly if called directly in callbacks from other calls. To mitigate the problem the next interaction was routed to Android main thread. Now when investigating the issue it turned out that it may be any other thread as long as it is not the callback thread.