Assorted fixes to allow pairing/bonding, and working with Apple Siri Remotes #27
Conversation
MTU for client and server can be different. When we negotiate MTU, don't use the returned server's MTU for our client's MTU. Instead, set our MTU, notify the server, and be done.
A random address may be used when pairing to a peripheral. For later reference, the public address needs to be known. Until this change, there was no way to know the public address from the information available when performing the pairing. The kernel returns the public address at pair completion, and bluepy now propogates it to the caller.
Bondable/pairable controls if bonding is enabled when pairing with a device. If it is not set when pairing, some devices will refuse to pair with an insufficient authentication error.
Devices notify however they want, and BTLE should faithfully reflect that.
|
Thank you @retsyx for this PR. We will look in to it. |
|
If you'd like, you can test your changes by installing |
|
We're willing to pull the first three commits, but commit e0bf351 is unrelated to the other commits and more controversial. I'd like to ask you to do two things:
|
|
All the commits are necessary for bluepy3 Siri Remote compatibility. Without Perhaps I'm misunderstanding the exception. I failed to find a commit message explaining the change (relative to bluepy). On the face of it, it looks like a transport layer, bluepy3, is making determinations about what devices are allowed to do. Given you think the exception is necessary, what is the correct way to use bluepy3 so that the exception isn't raised by arbitrary device behavior? |
|
There are/were a lot of problems like this one and this one with Xiaomi Mija LYWSD03MMC devices that would hang
As I see it now, there is a bug in the implementation of BT/BLE in some devices in the market. Notably Xiaomi devices seem infected. I can't fix those. I agree with you that the stack should allow devices to communicate however they see fit. But, the next best thing AFAICS is to fix the erroneous device behaviour at this point in the stack. We could make the number of |
|
Ok. I looked at the code again and I think that your spam limiting change is entirely untenable. At least as it is structured at the moment. The explanation is really simple. The entire Siri Remote protocol is based on notifications. The way the Siri remote protocol works is that the client (bluepy3) connects to the server (the remote), and requests various notifications for button, touch, or audio events. When a user presses a button, or moves a finger on the remote's touchpad, the remote generates a notification. So there will be a practically infinite number of notifications. From looking at the code, I think it is counting notifications globally over the lifetime of the helper. That obviously won't work for a remote that uses notifications for everything. I don't know why the Xiaomi sensors generate a notification flood. Perhaps it has a similar notification protocol, and it is being misconfigured by your handshake code (not that that justifies a notification flood), or perhaps it is has not been tested by Xiaomi for long duration BT sessions (I assume the sensor is for use with phones which will naturally have only short term sessions). If it's a session duration problem than a periodic disconnect by higher level code could resolve the issue. If possible, I would try to get a BT trace to see if the Xiaomi sensor floods a Xiaomi certified compatible device. The alternative is to change the global limit to a rate limit. But I think the problem is really elsewhere. |
|
I understand your point and have to agree that this needs to go. I haven't previously used the stack in this way (with paired devices). So it never came up. I'm going to have to rethink this at some point, but will reinstate the |
Thank you. I tested v2.9.4, and it works well for me. |
|
I'm giving it a few more days of testing on my system. My experience with BT/BLE is that it seems to have a bit of a random timing when problems will reproduce. If all goes well I expect to merge the coming weekend. |
See individual commit messages.