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

Separated command queues between connections #260

Merged
merged 9 commits into from
Aug 30, 2017

Conversation

dariuszseweryn
Copy link
Owner

(#250) It was found that commands from different BluetoothGatt’s are synchronized so it is possible to decouple synchronization of commands for different connections. Thanks to that if there are two or more connections opened and one connection would get stalled — other connections may continue to work without stalling — with a single command queue shared between connections it was not possible.

(#250) It was found that commands from different `BluetoothGatt`’s are synchronized so it is possible to decouple synchronization of commands for different connections. Thanks to that if there are two or more connections opened and one connection would get stalled — other connections may continue to work without stalling — with a single command queue shared between connections it was not possible.
@dariuszseweryn dariuszseweryn added this to the 1.4.0 milestone Aug 3, 2017
@dariuszseweryn dariuszseweryn self-assigned this Aug 3, 2017
@dariuszseweryn dariuszseweryn requested a review from uKL August 3, 2017 15:24
@osmer
Copy link

osmer commented Aug 14, 2017

Can someone merge this pull request?

@uKL
Copy link
Collaborator

uKL commented Aug 15, 2017

It still requires some slight changes but you can expect it will arrive soon.

@osmer
Copy link

osmer commented Aug 15, 2017 via email

uKL added 3 commits August 21, 2017 15:26
… from leaking out of the ConnectionComponent.
…operations should not mention "radio" from now on.
Copy link
Collaborator

@uKL uKL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all "radio" mentions in favour of queue. There is one comment still to be addressed.

@@ -52,7 +51,7 @@ public void run() {

@Override
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
public <T> Observable<T> queue(final Operation<T> operation) {
public synchronized <T> Observable<T> queue(final Operation<T> operation) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if synchronization is required here. The only thing this method does it creating an Observable instance. It doesn't have to be synchronized (no shared resources)

@osmer
Copy link

osmer commented Aug 29, 2017

were the code changes requested approved?

Copy link
Collaborator

@uKL uKL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job :)

@dariuszseweryn dariuszseweryn merged commit b02645c into develop Aug 30, 2017
@dariuszseweryn dariuszseweryn deleted the feature/decouple_connections branch August 30, 2017 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants