search

Apollon77 / hap-controller-node

Node.js library to implement a HAP (HomeKit) controller
Mozilla Public License 2.0
54 stars 16 forks source link

Question: subscribe/unsubscribe #37

Closed Apollon77 closed 3 years ago

Apollon77 commented 3 years ago

@mrstegeman I have a question :-)

In HTTPClient the subscribe command returns the HTTPConnection and on unsubscribe this connection is provided back and used to unsubscribe. This makes sense, BUT after unsubscribe there is no "connection.close()" ... Is this forgotten or by intend? Would an unsubscribe also work with just closing that connection?

In GattClient on subscribe the GattConnection is returned and not disconnected, but the "unsubscribe" method is NOT taking this GattConnection for unsubscribe. So also here one connection is left open without a disconnect. The connection created in subscribe is used in the "event" cases to retrieve data.

In fact I see the dilemma with the current code structure. The subscribe allows (in HTTP case) to subscribe for X datapoints and the unsubscribe is not using the same set and I could unsubscribe for just some of them. But this means that the developer needs to take care to close the HTTP/GattConnections once he do not want to receive data anymore. or?

Thnak you for your thoughts on this

Apollon77 commented 3 years ago

@mrstegeman But before we take over, also here it would be great to get your "knowledge from the past"

mrstegeman commented 3 years ago

The main issue here is that subscriptions are fundamentally different between the two. In Bluetooth, each characteristic subscription is a single persistent connection. Bluetooth chipsets are limited in the number of open connections they can maintain.

As for automatically closing things... In HTTP, the connection can be reused for other operations (and in many cases, should be). With the GATT connection, successive operations require re-pairing. However, I probably left out the auto-close from GATT just to maintain a semi-consistent API between the two.

Apollon77 commented 3 years ago

Hm ... for Gatt there is in fact one connection which is initialized on subscription and then used in the "characteristic.on(data)" method to read the real data after the "data changed" trigger came ...

Whats your info on how many parallel connections can be maintained at the same time?

So here my idea and would be great to get your opinion:

I simply add logic so that lib maintains one connection per type for all subscriptions and also the list of subscribed entities and removes stuff here when someone unsubscribes. when last is unsubscribed also the connection will be closed. Additionally I add a way to "unsubscribe all"

Because on HTTP it should be easier that way and Gatt you have the "subscription persistant connections" plus the one to read data, so basically "the same just different" and on BLE it is more important to clean up bia unsubscribe.

Makes that sense?

mrstegeman commented 3 years ago

Whats your info on how many parallel connections can be maintained at the same time?

It's entirely dependent on the chipset and Bluetooth software stack.

Anyway, give it a shot! Might make things easier.

Apollon77 commented 3 years ago

Thanks. I hope I can sometimes mention you to get a second opionion (if you have time) and thank you for all your work done so far - stepping in this shoes will be not that hard :-)

Apollon77 commented 3 years ago

@mrstegeman I found that ( i think from you:

https://github.com/WebThingsIO/homekit-adapter/blob/master/lib/homekit-device.js#L3391-L3397

That basically matches with your words above.

mainly questions to 2.) Is that meaned for the subscriptions itself or for the "read data when subscription info triggers"? What hapens if the connection drops? Is read then getting an error? Is there any way to get a disconnect from a gatt connection somehow?

Thank you for infos on this

mrstegeman commented 3 years ago

disconnected events != connection drops

HAP accessories can emit events when they are not connected to anything (hence "disconnected") by changing their broadcasts.

Apollon77 commented 3 years ago

Sorry I do not understand. I can not find any "disconnected" logic in the BLE/Gatt code nor noble.

I have read the HAP doc part on "Global State Number" ( I think you mean this or?!) ... is that used anywhere here?

My question was mainly concerned about:

Keeping an active connection requires that you refresh the security session frequently, otherwise the connection will drop.

on how to detect that this happend and for which cases it was meaned to be an issue

mrstegeman commented 3 years ago

Yes, the GSN. In the version of the spec I have, it's Section 7.4.6.3 "Disconnected Events". In the WebThings code, they're handled here: https://github.com/WebThingsIO/homekit-adapter/blob/master/lib/homekit-adapter.js#L233-L235

Apollon77 commented 3 years ago

Got that ... but what you mean with the "connection drops after some time if security context is not refreshed"? how to detect that?

mrstegeman commented 3 years ago

Check out "7.5 Testing Bluetooth LE Accessories". There are multiple points in there about the device dropping the connection.

To catch disconnects, you can do that on the noble peripheral directly via the disconnect event: https://github.com/Apollon77/hap-controller-node/blob/main/src/transport/ble/gatt-utils.ts#L153

From a higher level, I was catching them like this: https://github.com/WebThingsIO/homekit-adapter/blob/master/lib/homekit-device.js#L295-L303

As far as I can tell, you can't tell directly that the disconnect was due to the security session. I did write the code to refresh the session, but I'm not sure how well it works: https://github.com/Apollon77/hap-controller-node/blob/main/src/transport/ble/gatt-client.ts#L566-L567

Apollon77 commented 3 years ago

To catch disconnects, you can do that on the noble peripheral directly via the disconnect event: https://github.com/Apollon77/hap-controller-node/blob/main/src/transport/ble/gatt-utils.ts#L153

Ok, I had the opimion after checking noble code that this event is only fired after you send the disconnect "command" kind of as acknowledgement ... Maybe I overlooked something.

As far as I can tell, you can't tell directly that the disconnect was due to the security session. I did write the code to refresh the session, but I'm not sure how well it works: https://github.com/Apollon77/hap-controller-node/blob/main/src/transport/ble/gatt-client.ts#L566-L567

Ok will look at that deeper ... my question was more "outside of pair" so like for that long running connection here https://github.com/Apollon77/hap-controller-node/blob/main/src/transport/ble/gatt-client.ts#L1751

Apollon77 commented 3 years ago

I did some adjustments in the GitHub version ...