Re: [PATCH BlueZ 0/4] Add component batteries and Fast Pair Message Stream
Bastien Nocera <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <[email protected]> |
Hey Matthias, I have a couple of very high-level comments about various patches, I'm putting them here so as to avoid getting too deep into the details. On Thu, 2026-08-20 at 00:31 +0200, Matthias Kurz wrote: > True wireless earbuds can report separate charge states for the left > bud, > right bud, and charging case. Battery1 currently has one fixed object > per > Device1, so BlueZ cannot expose those values without collapsing them > into > one percentage. > > Extend the battery core and provider API to support child Battery1 > objects > with a stable identifier, optional percentage, and optional charging > state. > The existing Battery1 object at the Device1 path remains the > aggregate > compatibility interface. Component objects and their new properties > remain > experimental. I know of one direct consumer of the org.bluez.Battery1 interface, and it's upower. Did you verify whether your changes cause the current versions of UPower any problems? Do you have any work planned on upower to add support for those sub-devices? I think that it might be very useful to show how exactly D-Bus objects appear on the bus, as well their paths, interfaces and properties, so people without the hardware can reproduce "mock" versions using python- dbusmock: https://github.com/martinpitt/python-dbusmock This is most likely what I would do to be able to test gnome- bluetooth's battery information, where it coalesces info from both bluetoothd and upower to show battery info next to Bluetooth devices. > > Add an experimental Fast Pair Message Stream profile which connects > to the > advertised RFCOMM service and publishes its left, right, and case > battery > updates through the new component objects. The generic unknown-level > status > bit is retained for earbuds. Treat the TWS-specific case value 0xff > as > unavailable. I've seen some magic numbers appearing in the implementation. It might be useful to have those defined in a header which you can reference in the tests. > If the Message Stream closes while BR/EDR remains connected, > invalidate the > values and reconnect with exponential backoff. Reset the backoff only > after > a battery-producing stream remains connected for the maximum backoff > interval, and do not retry permanent local errors. Once BR/EDR > disappears, > cancel pending work and remove the component objects. The final patch > adds > a standalone diagnostic and provider tool for interoperability > testing. > > This was tested with Pixel Buds Pro using an ASan/UBSan build. The > live > tests covered fresh left/right/case reports, an unavailable case > value, > explicit Message Stream disconnection, remote device disconnection, > reconnection, adapter power-down, and cancellation of a profile > connection > in progress. Component properties were invalidated or removed as > appropriate, and the daemon reported no sanitizer failure. > > The full 40-test make check suite passes under ASan/UBSan. The Fast > Pair > parser tests cover payload fragmentation, a complete frame followed > by a > partial frame, invalid input, a zero-length frame, the maximum 65535- > byte > payload, unknown-level status bits, the unavailable-case sentinel, > and > reserved battery values. The Python tool compiles and its matching > decoder > was checked directly. Tests are great, but what's the coverage like? :) Cheers > > Matthias Kurz (4): > battery: Add component battery objects > doc: Document component battery objects > fastpair: Add Message Stream battery profile > test: Add Fast Pair Message Stream tool > > .gitignore | 1 + > Makefile.am | 8 + > Makefile.plugins | 5 + > Makefile.tools | 2 +- > doc/org.bluez.Battery.rst | 35 +- > doc/org.bluez.BatteryProvider.rst | 16 + > profiles/fastpair/fastpair.c | 650 > +++++++++++++++++++++++++++++ > profiles/fastpair/message-stream.c | 129 ++++++ > profiles/fastpair/message-stream.h | 42 ++ > src/battery.c | 401 +++++++++++++++--- > src/battery.h | 4 + > test/test-fastpair | 561 +++++++++++++++++++++++++ > unit/test-fastpair.c | 286 +++++++++++++ > 13 files changed, 2079 insertions(+), 61 deletions(-) > create mode 100644 profiles/fastpair/fastpair.c > create mode 100644 profiles/fastpair/message-stream.c > create mode 100644 profiles/fastpair/message-stream.h > create mode 100755 test/test-fastpair > create mode 100644 unit/test-fastpair.c