Re: [PATCH BlueZ 0/4] Add component batteries and Fast Pair Message Stream

Hi Bastien,

Thanks for the review.

Existing UPower releases continue to use org.bluez.Battery1 on the
Device1 object path. This series does not change that aggregate object's
API or behaviour. The component objects and their new properties are
experimental.

With experimental BlueZ enabled, current UPower does not understand the
child Battery1 objects. It may log a coldplug warning because it expects
Device1 on the same path, and then ignores that component. The existing
aggregate battery remains available, so this does not regress the
currently displayed value.

I have working UPower support for the component objects here:

https://gitlab.freedesktop.org/mkurz/upower/-/commits/fastpair-multi-battery

The branch is currently based on:

https://gitlab.freedesktop.org/upower/upower/-/merge_requests/337

It creates one UPower device for every component, handles optional and
invalidated Percentage and Charging properties, and falls back to the
aggregate battery when no components are present. It extends UPower's
python-dbusmock-based BlueZ integration tests. I have not opened an
UPower MR yet while the BlueZ API is still under review.

The object layout observed with Pixel Buds Pro was, with the address
anonymised:

  /org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF/battery_left
  /org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF/battery_right
  /org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF/battery_case

Each child implements org.bluez.Battery1. A GetAll snapshot was:

  battery_left:
    Percentage = 92
    Source = "Fast Pair Message Stream"
    Device = /org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF
    Identifier = "left"
    Charging = false

  battery_right:
    Percentage = 90
    Source = "Fast Pair Message Stream"
    Device = /org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF
    Identifier = "right"
    Charging = false

  battery_case:
    Source = "Fast Pair Message Stream"
    Device = /org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF
    Identifier = "case"
    Charging = false

Percentage was absent from the case object because its reported level was
unknown. I will include this layout in the v2 cover letter so consumers
can reproduce it with python-dbusmock.

For v2 I have also moved the Message Stream layout, battery masks,
sentinels and component indexes into the shared header used by the C
unit test. The Python diagnostic tool uses corresponding named
constants.

unit/test-fastpair contains seven cases covering complete, fragmented,
coalesced/partial, zero-length and maximum-length frames, invalid input,
and the battery decoding rules. GCC/gcov reports 100% line coverage
(55/55 lines) and 95.83% branch-direction coverage (46/48 outcomes) for
profiles/fastpair/message-stream.c. I also added test-fastpair to
doc/test-coverage.txt. The complete BlueZ make check suite passes all
40 test programs.

The daemon/profile lifecycle and D-Bus integration are not fully
unit-covered, but were exercised under ASan with real Pixel Buds Pro
hardware.

I also tested the complete path through patched BlueZ, UPower, Solid,
PowerDevil, BlueZQt and BlueDevil. KDE Plasma's Power & Battery applet
displayed separate left, right and case batteries, including an unknown
case level, and the Bluetooth applet displayed all three components.

The related independent KDE changes are:

https://invent.kde.org/frameworks/solid/-/merge_requests/264
https://invent.kde.org/plasma/powerdevil/-/merge_requests/666

The BlueZQt and BlueDevil component-display changes remain local while
the BlueZ component API is under review.

Regards,
Matthias