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

GATT operation in progress - how to handle it? #188

Open
perja12 opened this Issue Nov 24, 2015 · 28 comments

Comments

Projects
None yet
9 participants
@perja12

perja12 commented Nov 24, 2015

I have a web page that writes to a characteristic, but if I do this too fast, I will get the error "GATT operation in progress". Is this something the page author should handle or should the implementation take care of queuing the requests?

If it is the page author, it will be good to include some recommendations for how to handle this. AFAIK there is no way to query the state of the implementation other than responding to the errors.

@beaufortfrancois

This comment has been minimized.

Member

beaufortfrancois commented Nov 24, 2015

Are you testing this on Chrome OS or Android?

@perja12

This comment has been minimized.

perja12 commented Nov 24, 2015

Testing this on Android (Chrome Dev build) and soon in Opera internal build.

@beaufortfrancois

This comment has been minimized.

Member

beaufortfrancois commented Nov 24, 2015

I believe this issue has been fixed recently (https://codereview.chromium.org/1465863003/).
You may want to try with the latest Chromium build at https://download-chromium.appspot.com/?platform=Android and let me know if you're still experiencing this.

@perja12

This comment has been minimized.

perja12 commented Nov 24, 2015

Tested the latest Chromium and the crash when "GATT operation in progress" occurs has been fixed - so that is good.

But my main question is still open: is it the responsibility of the page author to handle "GATT operation in progress" or should this be handled by the implementation? There seems to be pros and cons with both approaches.

@beaufortfrancois

This comment has been minimized.

Member

beaufortfrancois commented Nov 24, 2015

Glad to hear that.

Hopefully @g-ortuno @scheib or @jyasskin will be able to fully answer your question here.

@jyasskin

This comment has been minimized.

Member

jyasskin commented Nov 24, 2015

The implementation's supposed to handle it, but we don't yet (https://crbug.com/560862). @scheib has argued that we should make the web page handle it instead, in which case the rule would be that you wait for the writeValue() Promise to settle before calling writeValue() again. If there's a good reason you can't do that, it would probably settle the argument.

@scheib

This comment has been minimized.

Contributor

scheib commented Nov 24, 2015

We should keep the implementation flexible, and not force queuing if the application doesn't desire it.

12:35 AM Hi, short question about web-bluetooth: I have a page that writes to a characteristic whenever a button is pressed (or actually a touch event), but if I click to fast I get the "GATT operation in progress"-error. Is it so that the page author is supposed to handle this with a queue or similar, or will the implementation do the queing?
1:12 AM perja: Currently unspecified. We should pick and make it clear. Some devices may be fairly slow to accept a write. What should happen if the web page can generate write requests at 2 times the speed a device can process them? If the browser queues, should it pick any limit for how large the queue should get? An application can know if a queue of unlimited values is needed or if only 1 pending value needs to be queued and replaced if a newer value arrives.

@pzboyz

This comment has been minimized.

pzboyz commented Nov 26, 2015

Truly low power devices will be setting connection intervals of +100ms, and it is highly likely that only one GATT operation may be handled per connection interval, arguably the page author does not need to know the GATT operations have entered a queue, but it will need to know which ones have been executed. Imagine a case where the web page has written to a characteristic expecting the device behaviour to change, if that operation is 4th in the queue, the device may still send notification indicating it is still performing with the previous old behaviour, what is the web page to do given that it thinks the GATT operations have already been sent?

@jyasskin

This comment has been minimized.

Member

jyasskin commented Nov 26, 2015

When the web page wants to know which writes have completed, it needs to attach a handler to the Promise returned by writeValue(). If the implementation doesn't queue, every use of writeValue needs to be handled like this, while if we do queue, sites that don't care about latency (e.g. sites that only write in response to a user gesture, and for which the device itself gives feedback that the operation is finished) can write simpler code.

@g-ortuno

This comment has been minimized.

Contributor

g-ortuno commented Nov 30, 2015

I think we should keep the implementation as simple as possible for now. Writing a library/polyfill to queue GATT operations is fairly trivial and would allow us to iterate faster if unforeseen problems arise.

@entertailion

This comment has been minimized.

entertailion commented Dec 7, 2015

Agree with @scheib about keeping the implementation flexible. It is easy enough to wait for fast write operations and not all apps need to have queuing.

@jyasskin

This comment has been minimized.

Member

jyasskin commented Dec 7, 2015

SG. In order to write the queuing library, we'd need to guarantee that two separate tabs using Bluetooth concurrently, or a tab and an extension/app, or a tab and the OS, couldn't cause the tab to see an in progress error. Is that something the implementation can guarantee with simpler code than just queuing everything?

@g-ortuno

This comment has been minimized.

Contributor

g-ortuno commented Dec 7, 2015

We don't get any signals from the system that the there is an operation in progress. Which also means our implementation of queuing couldn't guarantee no in progress errors since a Tab could get an in progress error if another App is using bluetooth.

@Emill

This comment has been minimized.

Emill commented Sep 23, 2016

I'd strongly suggest implementing the queue directly inside the UA. That's what CoreBluetooth does (and I think Windows as well if I'm not mistaken).
On Android however each app must implement the queue itself. Since it's undocumented (as far as I've seen) on Android that you can only have one outstanding GATT operation, this has resulted in very many stack overflow questions. Also, because the same queue must be used for all different operations (read/write characteristic/descriptor) every app must basically create their own wrappers around BluetoothGatt which is kind of annoying that does the enqueuing. And since there are quite many Android programmers that are newbies it is sometimes hard for them to understand how to do this correct. Especially that if you enqueue all write operations for example and do some wrapper functions for that, you also need to remember to wrap all read requests using the same queue as well.

Just see
http://stackoverflow.com/questions/34215880,
http://stackoverflow.com/questions/34523451,
http://stackoverflow.com/questions/36163998,
http://stackoverflow.com/questions/39638731,
http://stackoverflow.com/questions/39661417,
http://stackoverflow.com/questions/39109178,
http://stackoverflow.com/questions/30398599,
http://stackoverflow.com/questions/28493281

Of course it's easier with promises to implement a queue yourself rather than having a global complete event as in Android, but you still would have the problem that you need to wrap all kind of GATT operations in an additional layer.

The problem with if a user enqueues requests faster than the peripheral can consume is pretty small I think. Normally you send so small amounts of data that upper bound is not a problem and the problem would exist regardless if the queue implementation is in the UA or the js app. The js app programmer could still implement flow-prevention mechanisms by waiting for the promise to finish before issuing the next request if he is worried about sending data too fast.

@scheib

This comment has been minimized.

Contributor

scheib commented Sep 23, 2016

I'm convinced that at a minimum we should ensure a queuing library is available that is easy to use. And, I agree that long term this is best to build into the UA. I see & agree that my concern that an app may not want queueing is resolved by the app waiting for each promise to resolve before proceeding.

How many times should an operation be retried, or how long to wait? I see the Nordic puck central sample uses a timer to abort all pending 'bundled' operations. I'm a bit concerned that these are opinionated decisions that either: need to be spec.ed, UA has to make a call for, or options need to be exposed.

@Emill

This comment has been minimized.

Emill commented Sep 23, 2016

How do you mean that an operation is retried? The request is sent to the remote device and then it responds either with success (promise resolves) or some error code (promise rejects). The only problems that can happen are that either the link gets disconnected or the GATT timer specified by the BLE standard of 30 seconds times out (in that case the link is disconnected as well). Or that the underlying bluetooth stack bails out and crashes but then you're probably out of luck anyway. Haven't seen that however as of yet regarding GATT operations on the common platforms.

On the GATT level you can't abort a request. Even if you for some reason want a shorter timeout than 30 seconds you are not allowed to send a new request until the previous one is complete. So in practice a custom timeout doesn't mean anything at all than basically notifying that the operation has still not yet completed.

Nordic's example seems kind of strange that they give the ability to cancel enqueued requests not yet sent. That is really something you would like to do on the application layer, if you want to do that at all. I'd like to see a use case here. I think that if you must decide what to send next depends on the result of some operation, then you should wait until the operation completes and then resume rather than enqueuing lots of operations and then remove them from the queue if some operation didn't return what you wanted...

@scheib

This comment has been minimized.

Contributor

scheib commented Sep 23, 2016

Retries: I would presume that a queue should re-try an operation upon receiving a "GATT operation in progress" error. e.g.:

  1. Presume a queue with operations A, B.
  2. Operation A is submitted.
  3. Operation A fails due to GATT operation in progress.
  4. Operation A is re-submitted.
  5. Operation A succeeds.
  6. Operation B is submitted.

It is possible multiple retries may be needed. And, that a delay before retry is warranted (likely with exponential backoff).

An alternative would be that if operation A fails due to an in progress error, then B would also fail. But then we don't need a library - we just use a chain of promises.

@Emill

This comment has been minimized.

Emill commented Sep 23, 2016

At least on Android, as long as you keep track if there is a pending GATT operation or not, and make sure you never execute a GATT operation if there already is one pending, it does not fail.

Are you talking about BlueZ in mind now? Does it have a restriction that pending GATT operations are global for all clients, meaning that you can't know if there is a pending operation and not, and it fails when there is?

@jyasskin

This comment has been minimized.

Member

jyasskin commented Sep 23, 2016

We should test how Android deals with multiple separate applications trying to do multiple operations at once. If it doesn't return InProgress, then we're home free, since the UA can serialize all of its operations. (@Emill, have you already tested this?) We could fix BlueZ if it's more global than Android.

@Emill

This comment has been minimized.

Emill commented Sep 23, 2016

Yes it works fine on Android. It has an internal queue of operations.

@Emill

This comment has been minimized.

Emill commented Sep 24, 2016

Actually the only bug I've found on Android so far when the callback never is called is this one https://code.google.com/p/android/issues/detail?id=223558 which I should have submitted a long time ago...

Anyway, have you found out how BlueZ behaves when another app is executing a GATT operation?

@beaufortfrancois

This comment has been minimized.

Member

beaufortfrancois commented Oct 20, 2016

For the record, here are not supported parallel operations and their respective errors I've found on current Dev Channel (55.0.2883.17):

Android

  • readValue - DOMException: GATT operation failed for unknown reason.
  • writeValue - DOMException: GATT operation already in progress.

Chrome OS / Linux

  • gatt.connect - DOMException: Connection failed for unknown reason.
  • readValue - DOMException: GATT Error Unknown.
  • writeValue - DOMException: GATT operation already in progress.

Mac OS

  • readValue - DOMException: GATT Error: Not supported.
  • writeValue - DOMException: GATT operation already in progress.
  • stopNotifications is not implemented yet.

As you can see, parallel connects works fine on Android and Mac OS while they're not on Chrome OS and Linux (BlueZ).
readValue error message is not consistent across platforms.
stopNotifications is not implemented yet on Mac OS but I suspect it should work fine eventually.

@jyasskin

This comment has been minimized.

Member

jyasskin commented Oct 20, 2016

I'm planning to add a statement that UAs MAY return a NetworkError for concurrent use of connect, readValue, and writeValue. I hope we can remove that again in the future, but I want the spec to reflect reality until the implementations are improved.

jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this issue Oct 20, 2016

Allow UAs to reject concurrent use of 6 functions.
* GattServer.connect()
* Characteristic.readValue()
* Characteristic.writeValue()
* Characteristic.startNotifications()
* Descriptor.readValue()
* Descriptor.writeValue()

This is uncomfortable because while serializing within a single realm
will reduce the frequency of these NetworkErrors, separate realms and
possibly other applications entirely can also cause these errors, and
the only way to recover is to retry and hope it doesn't happen again.

We're discussing the constraints imposed by the OS and the long-term
plan in WebBluetoothCG#188. Any change should reduce the number of exceptions and so
be backward-compatible.
@jyasskin

This comment has been minimized.

Member

jyasskin commented Oct 20, 2016

#316 includes startNotifications() too, because I can't believe that writing characteristics needs serialization, but writing the CCC descriptor doesn't.

@g-ortuno

This comment has been minimized.

Contributor

g-ortuno commented Oct 20, 2016

I think they are somewhat different. When you call writeValue() multiple times you expect each write to be sent to the device whereas when you call startNotifications() multiple times you don't expect each call to write to the descriptor so we can queue on the first write to the descriptor succeeding or failing.

@jyasskin

This comment has been minimized.

Member

jyasskin commented Oct 20, 2016

If you're writing one characteristic, and you concurrently start notifications on another one, does that work?

@beaufortfrancois

This comment has been minimized.

Member

beaufortfrancois commented Oct 21, 2016

FYI, with BlueZ, I get the error below:

characteristic.readValue().catch(err => console.log('readValue', err));
characteristic2.startNotifications().catch(err => console.log('startNotifications', err));
readValue DOMException: GATT operation failed for unknown reason.

jyasskin added a commit that referenced this issue Oct 21, 2016

Allow UAs to reject concurrent use of 6 functions. (#316)
* GattServer.connect()
* Characteristic.readValue()
* Characteristic.writeValue()
* Characteristic.startNotifications()
* Descriptor.readValue()
* Descriptor.writeValue()

This is uncomfortable because while serializing within a single realm
will reduce the frequency of these NetworkErrors, separate realms and
possibly other applications entirely can also cause these errors, and
the only way to recover is to retry and hope it doesn't happen again.

We're discussing the constraints imposed by the OS and the long-term
plan in #188. Any change should reduce the number of exceptions and so
be backward-compatible.

MXEBot pushed a commit to mirror/chromium that referenced this issue Jan 31, 2017

Serialize calling of characteristic.writeValue in LayoutTests.
characteristic.writeValue can not be called multiple times. If the UA is
currently using the Bluetooth system (for example, if we are doing a writeValue)
we may reject promise with a NetworkError. These "GATT operation in progress"
errors may be addressed in the future (like having the UA magically serialize).
See WebBluetoothCG/web-bluetooth#188 for the spec
issue.

Here we are fixing the tests to ensure that we are serializing. Futher work is
required to test for GATT operation in progress errors.

BUG=None

R=ortuno

Review-Url: https://codereview.chromium.org/2664003002
Cr-Commit-Position: refs/heads/master@{#447139}
@WayneKeenan

This comment has been minimized.

WayneKeenan commented Dec 23, 2017

I'm using the Nordic UART service with a tweak that inconsistently triggers Uncaught (in promise) DOMException: GATT operation already in progress.

The tweak is that during the UART writeCharacteristic my BLE service handler on the peripheral is sending a notification (on the other notification-only characteristic) before the service handler returns.

Sometimes it works, once or twice, sometimes the JS notification event isnt called and sometimes it errors with the above message sporadical and then sometimes continuously.

Works fine using LightBlue.

I'm on MacOS 10.12 and Chrome Version 63.0.3239.84 (Official Build) (64-bit)

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