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

Run custom operation (or at least get access to BluetoothGatt and RxBleGattCallback) #137

Closed
maoueh opened this issue Feb 16, 2017 · 4 comments
Assignees

Comments

@maoueh
Copy link
Contributor

maoueh commented Feb 16, 2017

Summary

For speed optimization purpose, it would be nice to have a way to access BluetoothGatt and RxBleGattCallback classes directly to avoid overhead of entering/exiting the RxBleRadio queue.

This is particularly useful to improve speed of rather big transfer like when uploading a firmware to a device. In our case, with a custom fork allowing me to queue up operation to be executed, I was able to go from around 85s to 39s when using a single operation to make the whole transfer.

What I envision would be a queue method on the RxBleConnection interface so that user would be able to queue their own operation for execution.

Even if there would be other method to allow access BluetoothGatt and RxBleGattCallback, I think that a queue that receives a builder method returning an RxBleRadioOperation instance seems a pretty good solution.

For me, this fits nicely in the architecture of the library and most important, it leverages the queue directly so implementor would not need to care about interfering with other operations performed by RxAndroidBLE. Indeed, they would run when queue is ready to process the operation which removes the need for some user code to prevent concurrent invocations (even if I think it usually quite easy to ensure no concurrent executions).

The interface method could look this:

    /**
     * <b>This method requires deep knowledge of RxAndroidBLE internals. Use it only as a last resort if you know
     * what your are doing.</b>
     * <p>
     * Queue an operation for future execution. The method accepts a builder parameter. This builder parameters
     * receives three instances and return the a non-null instance of {@link RxBleRadioOperation}. The three
     * arguments the builder receives are:
     * <ol>
     * <li>{@link Scheduler} The RxBleRadio scheduler used to execute operation</li>
     * <li>{@link BluetoothGatt} The Android API GATT instance</li>
     * <li>{@link RxBleGattCallback} The internal Rx ready bluetooth gatt callback to be notified of GATT operations</li>
     * </ol>
     *
     * @param builder The builder used to create the {@link RxBleRadioOperation} instance
     * @param <T>     The type returned by the  {@link RxBleRadioOperation} instance
     * @return Observable emitting the value after execution or an error in case of failure.
     */
    <T> Observable<T> queue(Func3<Scheduler, BluetoothGatt, RxBleGattCallback, RxBleRadioOperation<T>> builder);

Of course, the main goal is to have access to BluetoothGatt and RxBleGattCallback, so if you don't feel queue and custom operation is the way to go, as long as I can have access somehow to those classes, I would be happy.

@maoueh
Copy link
Contributor Author

maoueh commented Feb 16, 2017

@dariuszseweryn Copying your answer from another thread (see here):

I see no particular reason to do it like this. I was thinking about just exposing the RxBleGattCallback and BluetoothGatt for direct usage for optimisation purposes.
Could you elaborate more on your use-case? We would then also have material for internal discussions.

@dariuszseweryn
Copy link
Owner

Hello @maoueh
Sorry for a late response but I was on vacation since Thursday.
I have thought about your case and I indeed agree that your proposal would fit nicely in the library. At the moment I do have some more important things on my plate (i.e. API 21 scans) but I would happily help by reviewing any PRs that you may provide.

@maoueh
Copy link
Contributor Author

maoueh commented Feb 20, 2017

@dariuszseweryn No problem at all, hope you enjoyed them :)

I will provide a PR for sure and we can discuss further at that point. I already have a branch with the change, but it build up on another one which is the move to choose a different scheduler than main thread which is more problematic.

Will merge the two removing the main thread change and will send a PR for discussion.

@dariuszseweryn
Copy link
Owner

dariuszseweryn commented Feb 27, 2017

Added with fb2eccc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants