Clarify GATT connection status / behavior

[jracle]

Following my reaction in #169 , I'd like to be clear about GATT connection state (app vs physical).

For now it seems that gatt.connected reflects the state of an instance of BluetoothRemoteGATTServer being connected to an instance of BluetoothDevice (pure software matters). If we consider underlying connection from central to peripheral, we might already be connected to the peripheral (device) in question : why not having device.gatt.connected === true in that case?

Other concern is the ability to disconnect GATT server. I get the use cases, but maybe its wiser for a start to remove this feature, and leave underlying UA garbage-collect connection(s) on script exit. That would also permit not closing connections if they are needed by the OS. I agree here we can live with it..

[beaufortfrancois]

For now it seems that gatt.connected reflects the state of an instance of BluetoothRemoteGATTServer being connected to an instance of BluetoothDevice (pure software matters). If we consider underlying connection from central to peripheral, we might already be connected to the peripheral (device) in question : why not having device.gatt.connected === true in that case?

According to #215 (comment), the information leak from telling a Realm whether other Realms are connected seems small enough to accept.

What would that change to current behaviour?

navigator.bluetooth.requestDevice({filters: [{services: ['battery_service']}]})
.then(device => {
  if (!device.gatt.connected) {
    return device.gatt.connect();
  } else {
    // Already connected.
    return Promise.resolve();
  }
})
...

Are you thinking that one realm having bluetooth device foo getting connected would trigger in a second realm a brand new gattserverconnected event? Should we poll for the device.gatt.connected boolean status?

[jracle]

Thanks @beaufortfrancois for re-phrasing and quoting issue #215 , which makes my mind clearer after reading it..

I'm raising the point in along with issues #255 / #253 / #243 related to peripheral's GATT connection state.

I'm fine with conclusion of #215, in a sense that we don't need to have 2 events reporting disconnection (physical vs Realms). However, IMO I'd be more comfortable if gatt.connected was reflecting physical connection status.

It would be more aligned with currently specified disconnection behavior, where Realm is also notified of https://webbluetoothcg.github.io/web-bluetooth/#eventdef-bluetoothdevice-gattserverdisconnected in case of physical disconnection, also setting deviceObj.gatt.connected to false.

With that in place, if other Realm connects to device, or if device is already connected by OS after pairing from bluetooth menu, then device.gatt.connected would evaluate to true.

We would also indeed have to add a brand new gattserverconnected event, so other Realms could be notified of external connection to device. As for polling, I'm not in favor of it.. and we could as well make device.gatt.connect() be idempotent if device is already connected, and keep our code in simpler form :

navigator.bluetooth.requestDevice({filters: [{services: ['battery_service']}]})
.then(device => return device.gatt.connect())
...

Again I'm fine with current specification of gatt.connected, but just want to bring other view on it.

[jyasskin]

I like having the per-realm boolean (spelled device.gatt.connected today) saying whether that realm is holding the physical GATT connection open, because that lets the UA GC the physical connection when no more realms are holding it open, even if some realms still have a reference to the BluetoothDevice object. I also like the current state where attempts to use the connection only work if the current realm is holding the connection open, because that means the UA won't GC the physical connection while a realm is actually trying to use it.

I don't mind having another boolean saying whether there is a physical GATT connection at all, but what's the use case?

It would be more aligned with currently specified disconnection behavior, where Realm is also notified of gattserverdisconnected in case of physical disconnection

I think that's not quite correct: a physical disconnection causes a logical disconnection, and the logical disconnection causes the gattserverdisconnected event. For the reverse, a physical connection wouldn't intrinsically cause a logical connection, so we'd want just a gattserverphysicallyconnected event.

[jracle]

I like having the per-realm boolean (spelled device.gatt.connected today) saying whether that realm is holding the physical GATT connection open

Indeed that's a good description for device.gatt.connected, Realm holding physical connection, I get it now.

because that lets the UA GC the physical connection when no more realms are holding it open, even if some realms still have a reference to the BluetoothDevice object.

Just one warning here, in case system still requires physical connection, UA should leave it open ( see crbug.com/630586 ).

I think that's not quite correct: a physical disconnection causes a logical disconnection, and the logical disconnection causes the gattserverdisconnected event. For the reverse, a physical connection wouldn't intrinsically cause a logical connection, so we'd want just a gattserverphysicallyconnected event.

Agree! And also gattserverphysicallyconnected is probably not be relevant as of now as well.

[beaufortfrancois]

A gattserverphysicallyDISconnected event would actually be very useful in my use case. See http://crbug.com/635875

A device (beacon) being disconnected means that it is now broadcasting the URL I've set. The gattserverdisconnected event is not enough there sadly.

@jyasskin Should I start a specific issue for this?

[jyasskin]

This issue is fine.

Do you mean a gattserverphysicallydisconnected event? If we expose a boolean physicallyConnected, then probably a gattserverphysicalconnectionchanged event makes sense, since the page doesn't control either edge.

[beaufortfrancois]

Yes. I meant gattserverphysicallydisconnected.