Linux bluetooth development
 help / color / mirror / Atom feed
From: Bastien Nocera <hadess@hadess.net>
To: Matthias Kurz <m.kurz@irregular.at>, linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH BlueZ 0/4] Add component batteries and Fast Pair Message Stream
Date: Thu, 20 Aug 2026 15:59:46 +0200	[thread overview]
Message-ID: <4131b2bc92a81467d11ce8bdf8748ace7e146514.camel@hadess.net> (raw)
In-Reply-To: <20260819223144.82045-1-m.kurz@irregular.at>

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

  parent reply	other threads:[~2026-08-20 13:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19 22:31 [PATCH BlueZ 0/4] Add component batteries and Fast Pair Message Stream Matthias Kurz
2026-08-19 22:31 ` [PATCH BlueZ 1/4] battery: Add component battery objects Matthias Kurz
2026-08-19 23:37   ` Add component batteries and Fast Pair Message Stream bluez.test.bot
2026-08-19 22:31 ` [PATCH BlueZ 2/4] doc: Document component battery objects Matthias Kurz
2026-08-19 22:31 ` [PATCH BlueZ 3/4] fastpair: Add Message Stream battery profile Matthias Kurz
2026-08-19 22:31 ` [PATCH BlueZ 4/4] test: Add Fast Pair Message Stream tool Matthias Kurz
2026-08-20 13:59 ` Bastien Nocera [this message]
2026-08-20 17:52   ` [PATCH BlueZ 0/4] Add component batteries and Fast Pair Message Stream Matthias Kurz
2026-08-20 14:00 ` Luiz Augusto von Dentz
2026-08-20 17:54   ` Matthias Kurz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4131b2bc92a81467d11ce8bdf8748ace7e146514.camel@hadess.net \
    --to=hadess@hadess.net \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=m.kurz@irregular.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox