fw/comm/ble: hand clients a single service instance, rotating on failure [FIRM-2018]#1627
Conversation
Some peer GATT servers (notably buggy Android companions) leave a stale duplicate of a service behind -- same UUID, different handle range -- when they re-add services without removing the old one. PPoGATT then started a handshake (meta read) per instance, and the concurrent reads deadlocked the single ATT bearer when the stale instance never answered, so the whole connection failed. With two PPoGATT instances the meta read failed with ENOTCONN once the phone gave up and disconnected; with one it succeeded. Hand each kernel LE client exactly one instance: collect every instance that exposes all required characteristics, and try the newest (highest-handle) one first. We can't tell which instance is live at discovery time, so PPoGATT advances the selection after a failed handshake and resets it on a successful session; across reconnects this cycles through the complete instances until the live one connects. Newest-first matches the Android re-add bug, where the server only retains a reference to the most recently added characteristics. This drops support for multiple concurrent instances of the same service UUID (legacy multi-app PPoGATT); the Core companion uses a single Hybrid server. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Joshua Jun <lets@throw.rocks>
ffa7d13 to
6bc0362
Compare
| // Collect every complete instance of this client's service. Some peer GATT | ||
| // servers (notably buggy Android companions) leave a stale duplicate of a |
There was a problem hiding this comment.
has this been confirmed to be buggy Android, or a bug in the mobile app?
There was a problem hiding this comment.
https://linear.app/core-dev/issue/FIRM-2018/nimble-bug-pt2-fails-to-connect-to-android-12-stuck-in#comment-7ffcb7b5 seems like they enable the option to ignore other pebble apps being installed, so that is the likely cause
There was a problem hiding this comment.
So I guess that's it, no workarounds needed on our side.
There was a problem hiding this comment.
I'm not sure to be honest - they enabled the pref to allow other apps to be installed without blocking connection, but looking again, it doesn't show another app being detected (could be a different package that we don't know about / a custom built coreapp or something). I'd say that rotating between the two is probably a complex workaround - maybe a really simple workaround to just use the first one it finds, rather than get stuck?
There was a problem hiding this comment.
note: won't reverse ppog fix all these problems?
There was a problem hiding this comment.
That's the hope (although I supposed we still need a mechanism to stop multiple clients subscribing to it)
There was a problem hiding this comment.
that's easier to handle, let's wait till then.
Some peer GATT servers (notably buggy Android companions) leave a stale duplicate of a service behind -- same UUID, different handle range -- when they re-add services without removing the old one. PPoGATT then started a handshake (meta read) per instance, and the concurrent reads deadlocked the single ATT bearer when the stale instance never answered, so the whole connection failed. With two PPoGATT instances the meta read failed with ENOTCONN once the phone gave up and disconnected; with one it succeeded.
Hand each kernel LE client exactly one instance: collect every instance that exposes all required characteristics, and try the newest (highest-handle) one first. We can't tell which instance is live at discovery time, so PPoGATT advances the selection after a failed handshake and resets it on a successful session; across reconnects this cycles through the complete instances until the live one connects. Newest-first matches the Android re-add bug, where the server only retains a reference to the most recently added characteristics.
This drops support for multiple concurrent instances of the same service UUID (legacy multi-app PPoGATT); the Core companion uses a single Hybrid server.