Skip to content

fw/comm/ble: hand clients a single service instance, rotating on failure [FIRM-2018]#1627

Open
jplexer wants to merge 1 commit into
coredevices:mainfrom
jplexer:jp/firm-2018-ppogatt-duplicate-service-dedup
Open

fw/comm/ble: hand clients a single service instance, rotating on failure [FIRM-2018]#1627
jplexer wants to merge 1 commit into
coredevices:mainfrom
jplexer:jp/firm-2018-ppogatt-duplicate-service-dedup

Conversation

@jplexer

@jplexer jplexer commented Jun 29, 2026

Copy link
Copy Markdown
Member

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.

@jplexer
jplexer requested a review from gmarull as a code owner June 29, 2026 13:57
@jplexer jplexer changed the title fw/comm/ble: hand clients a single service instance, rotating on failure fw/comm/ble: hand clients a single service instance, rotating on failure [FIRM-2018] Jun 29, 2026
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>
@jplexer
jplexer force-pushed the jp/firm-2018-ppogatt-duplicate-service-dedup branch from ffa7d13 to 6bc0362 Compare June 29, 2026 14:10
Comment on lines +227 to +228
// Collect every complete instance of this client's service. Some peer GATT
// servers (notably buggy Android companions) leave a stale duplicate of a

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has this been confirmed to be buggy Android, or a bug in the mobile app?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess that's it, no workarounds needed on our side.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: won't reverse ppog fix all these problems?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the hope (although I supposed we still need a mechanism to stop multiple clients subscribing to it)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's easier to handle, let's wait till then.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants