Add Bluetooth 6.3 CS Enhancements (Inline PCT) support - #953
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
JoeBakalor
changed the title
JoeBakalor
changed the title
2138970 to
15d8c07
Compare
| hci.HCI_LE_CS_Read_Local_Supported_Capabilities_V2_ReturnParameters | ||
| | hci.HCI_LE_CS_Read_Local_Supported_Capabilities_ReturnParameters | ||
| ) | ||
| try: |
There was a problem hiding this comment.
shouldn't we check command supported to decide which command to send?
There was a problem hiding this comment.
You're right, and my premise for the try/except was wrong. On the controller here (nRF54LM20A, NCS v3.4.0) octet 49 is 0x0c — bit 2 set, supports_command() returns True. Gated on it in 9413603; V2 is still selected, and forcing the check false falls back to the 6.0 read.
There was a problem hiding this comment.
Oh, I missed the explanation in PR description, but nice to catch that.
| # HCI event landed. | ||
| self._on_cs_remote_supported_capabilities_impl(event) | ||
|
|
||
| def _on_cs_remote_supported_capabilities_impl(self, event) -> None: |
|
|
||
| # V1 event carries rtt_random_sequence_n; V2 renamed the field to | ||
| # rtt_random_payload_n (same semantic per spec 6.3). | ||
| rtt_random = getattr( |
There was a problem hiding this comment.
if isinstance(event, V2):
...
else:
...
There was a problem hiding this comment.
Done in 9413603 — moved the isinstance branch into ChannelSoundingCapabilities.from_hci() so power_on() shares it.
| # Bluetooth 6.3 CS Enhancements — last byte of the command was called | ||
| # "reserved" pre-6.3. Bit 0 (`CsEnhancements1.INLINE_PCT`) enables Inline | ||
| # PCT transfer on this CS configuration. Set to zero for 6.0 behavior. | ||
| cs_enhancements_1: int = field(default=0, metadata=metadata(1)) |
There was a problem hiding this comment.
should not have default value for HCI events
01b00fc to
9413603
Compare
| # HCI event landed. | ||
| self._on_cs_remote_supported_capabilities_impl(event) | ||
|
|
||
| def _on_cs_remote_supported_capabilities_impl( |
There was a problem hiding this comment.
We can just keep a single on_cs_remote_supported_capabilities handler, and emit 'cs_remote_supported_capabilities' from Host.on_hci_le_cs_read_remote_supported_capabilities_complete_v2_event
There was a problem hiding this comment.
Done in 3eb16de — single handler taking the V1 | V2 union, and host.py emits cs_remote_supported_capabilities for both subevents.
zxzxwu
left a comment
zxzxwu
left a comment
There was a problem hiding this comment.
Thanks for adding Bluetooth 6.3 Inline PCT support and consolidating the capability parsers in ChannelSoundingCapabilities.from_hci()!
Comparing the implementation against the official Bluetooth Core Specification v6.3 (HCI Functional Specification), there are a few points to address:
-
Missing 2M RTT Fields in V2 structures:
Per Core v6.3 § 7.8.130 (LE CS Read Local Supported Capabilities command[v2]) and § 7.7.65.39 (LE CS Read Remote Supported Capabilities Complete event[v2]), Bluetooth 6.3 appends 5 fields (7 octets total) aftertx_snr_capability:t_ip2_ipt_times_supported: 2 octetst_sw_ipt_times_supported: 1 octet (namedT_SW_IPT_Times_Supportedin spec)rtt_2m_aa_only_n: 1 octetrtt_2m_sounding_n: 1 octetrtt_2m_random_sequence_n: 1 octet
Currently,
rtt_2m_aa_only_n,rtt_2m_sounding_n, andrtt_2m_random_sequence_nare missing in bothHCI_LE_CS_Read_Local_Supported_Capabilities_V2_ReturnParametersandHCI_LE_CS_Read_Remote_Supported_Capabilities_Complete_V2_Event. This results in a 32-octet payload instead of the spec-defined 35 octets (includingStatus). -
Field naming
rtt_random_sequence_nvsrtt_random_payload_n:
In Core v6.3 § 7.8.130 & § 7.7.65.39, the parameter name is stillRTT_Random_Sequence_N(andRTT_2M_Random_Sequence_Nfor 2M). Usingrtt_random_sequence_nuniformly across both V1 and V2 structures matches the official specification and removes the need to branch on the field name inChannelSoundingCapabilities.from_hci(). -
cs_enhancements_1naming:
In Core v6.3 § 7.8.137 (LE CS Create Config command), the parameter is namedCS_Enhancements(1 octet, Bit 0:IPT enabled in the CS reflector). Naming the fieldcs_enhancementsand the enumCsEnhancements(orCsEnhancement) would better align with the spec parameter name. -
Spec Section References in docstrings:
HCI_LE_CS_Read_Local_Supported_Capabilities_V2_Command: The section in Core v6.3 is § 7.8.130 (OCF 0x00A5), rather than § 7.8.161.HCI_LE_CS_Read_Remote_Supported_Capabilities_Complete_V2_Event: The section in Core v6.3 is § 7.7.65.39 (Subevent 0x38), rather than § 7.7.65.65.
Adds the HCI-layer definitions for the Bluetooth 6.3 Channel Sounding Enhancements feature (Inline Phase Correction Term transfer, "IPT"), none of which are wired to any existing behavior — this commit is purely additive so it can land independently of any policy change. New in bumble/hci.py: - HCI_LE_CS_READ_LOCAL_SUPPORTED_CAPABILITIES_V2_COMMAND (opcode 0x20A5) with matching HCI_LE_CS_Read_Local_Supported_Capabilities_V2_Command class + _V2_ReturnParameters that append `t_ip2_ipt_times_supported` (uint16) and `t_sw_ipt_time_supported` (uint8) after the 6.0 field list. - HCI_LE_CS_WRITE_CACHED_REMOTE_SUPPORTED_CAPABILITIES_V2_COMMAND opcode constant (0x20A6) for symmetry (no class yet — no in-tree caller). - HCI_LE_CS_READ_REMOTE_SUPPORTED_CAPABILITIES_COMPLETE_V2_EVENT subevent code (0x38) + matching event class carrying the same V2 tail. Emitted (instead of the 6.0 event 0x2C) when the LE Extended Feature Set has been negotiated on the link. - CsSubfeature IntFlag with the `CS_IPT_REFLECTOR = 1 << 4` bit — the subfeatures_supported flag advertised by a controller that can act as an IPT reflector. - CsEnhancements1 IntFlag with `INLINE_PCT = 0x01` — the value used in the last byte of HCI_LE_CS_Create_Config to enable IPT per- configuration. - Rename the trailing `reserved` byte of HCI_LE_CS_Create_Config_Command to `cs_enhancements_1` and drop it from the "pre-6.3 rsvd" comment. This is the field name Nordic uses in zephyr/bluetooth/hci_types.h:2986 and mirrors the semantics of `bt_conn_le_cs_config::cs_enhancements_1` in conn.h:851. - Add the V2 read command to HCI_SUPPORTED_COMMANDS_MASKS at octet 49 bit 2 per the spec. Callers can now feature-detect V2 via Host.supports_command(). bumble/host.py: - Add on_hci_le_cs_read_remote_supported_capabilities_complete_v2_event handler that emits `cs_remote_supported_capabilities_v2` so device.py can pick the right payload shape (see the follow-up commit). Verified end-to-end against a Nordic nRF54L15 (nRF Connect SDK v3.4.0) running as an IPT initiator against Nordic's channel_sounding/ipt_reflector sample: V2 caps read returned `t_ip2_ipt_times_supported = 0x7e`, `t_sw_ipt_time_supported = 0x0a`, `subfeatures_supported = 0x12` (CS_IPT_REFLECTOR bit set). Create_Config with `cs_enhancements_1 = 0x01` completed with status SUCCESS and CS procedures streamed subevent results.
Wires the HCI additions from the previous commit into Device: - ChannelSoundingCapabilities gains three trailing fields: `t_ip2_ipt_times_supported`, `t_sw_ipt_time_supported`, and the derived boolean `cs_ipt_reflector_supported` (from bit 4 of `subfeatures_supported`, per CsSubfeature.CS_IPT_REFLECTOR). Defaults keep the constructor backwards-compatible with any call site that still passes only the 6.0 field set positionally. - Device.power_on()'s CS bring-up now issues the V2 caps read (opcode 0x20A5) preferentially, falling back to the 6.0 read on UNKNOWN_HCI_COMMAND. Using try/except rather than supports_command() is deliberate: some hci_uart controllers implement the V2 command without setting octet 49 bit 2 in their LE Supported Commands reply, and this flow handles both cases without needing a per-vendor probe. The V2 dataclass renamed rtt_random_sequence_n → rtt_random_payload_n (same semantic) so the constructor unpacks either. - `on_cs_remote_supported_capabilities` is split into an outer V1/V2 dispatch pair and an inner _impl builder so both event shapes feed the same ChannelSoundingCapabilities instantiation. Same rtt_random field renaming applies. The new `cs_ipt_reflector_supported` bit propagates onto every emitted capability record regardless of which event fired. - create_cs_config() gains a `cs_enhancements_1: int = 0` keyword parameter (default 0 preserves the 6.0 behavior) and passes it through to HCI_LE_CS_Create_Config_Command. The old `reserved=0x00` call-site keyword was replaced by the rename in the previous commit; existing callers that did not set the reserved field remain unaffected. Verified against the same nRF54L15 rig described in the previous commit: caps report `cs_ipt_reflector_supported=True`, and a `create_cs_config(..., cs_enhancements_1=CsEnhancements1.INLINE_PCT)` call runs a full CS procedure with IPT enabled end-to-end.
Annotate the CS Read Local Supported Capabilities result as the union of the V2 and 6.0 return-parameter types so both the V2 read and the fallback send_sync_command type-check, and read the renamed rtt_random field in-branch (rtt_random_payload_n on V2, rtt_random_sequence_n on 6.0) where the concrete type is known. No behavior change.
Review feedback: HCI packet fields should not carry default values. The trailing byte of HCI_LE_CS_Create_Config is a required parameter like every other field in the class, so declare it that way. field(default=...) appeared exactly once in hci.py — this line — so removing it also restores the file's consistency. Device.create_cs_config already passes cs_enhancements_1 explicitly and is the only construction site in the tree, so no caller changes.
Addresses three review points on the CS bring-up path: 1. Use host.supports_command() to choose between the 6.0 and V2 caps reads instead of blind-calling V2 and catching UNKNOWN_HCI_COMMAND. The try/except was justified by a claim that Nordic hci_uart firmware implements the V2 command without setting octet 49 bit 2 in its LE Supported Commands reply. That does not reproduce: on an nRF54LM20A running hci_uart from nRF Connect SDK v3.4.0, octet 49 reads 0x0c, bit 2 is set, and supports_command() returns True. The gate now matches the supports_command() check used for HCI_LE_SET_HOST_FEATURE_COMMAND a few lines above. 2. Annotate _on_cs_remote_supported_capabilities_impl's event parameter with the V1 | V2 union rather than leaving it bare. 3. Replace the getattr() field probing with an explicit isinstance() branch. The isinstance branch lives in a new ChannelSoundingCapabilities.from_hci() classmethod that accepts all four HCI payload shapes (local V1/V2 return parameters, remote V1/V2 events). Those shapes share the 6.0 field names and differ only in the rtt_random_payload_n rename plus the IPT tail, so one branch covers them and the 23-argument constructor is no longer repeated at each call site. Net 94 deletions against 85 insertions. Verified on hardware: the V2 path is still selected and reports t_ip2_ipt_times_supported=0x7e, t_sw_ipt_time_supported=0x0a, subfeatures_supported=0x12, cs_ipt_reflector_supported=True — identical to the values this PR originally recorded. Forcing supports_command() to report the V2 command as unsupported sends the 6.0 read and leaves both IPT timing fields at zero.
Review feedback: the V2 capability structures were three fields short of the
spec. Core v6.3 § 7.8.130 (LE CS Read Local Supported Capabilities [v2]) and
§ 7.7.65.39 (LE CS Read Remote Supported Capabilities Complete [v2]) append
five parameters after TX_SNR_Capability, not two:
T_IP2_IPT_Times_Supported 2 octets
T_SW_IPT_Times_Supported 1 octet
RTT_2M_AA_Only_N 1 octet
RTT_2M_Sounding_N 1 octet
RTT_2M_Random_Sequence_N 1 octet
That makes the return parameters 35 octets including Status, where this branch
parsed 32. Add the three RTT_2M_* fields to both V2 shapes and rename
t_sw_ipt_time_supported to the spec's plural T_SW_IPT_Times_Supported.
ChannelSoundingCapabilities mirrors the three new fields with 0 defaults, the
same way the IPT timings already were, so the positional constructor stays
compatible with 6.0-only call sites. from_hci() now collects the whole V2 tail
into one mapping rather than naming each field twice and passing an explicit
zero on the 6.0 path.
The omission came from modelling these structures on Zephyr's. Those do carry
both IPT timings, but end there: RTT_2M_AA_Only_N, RTT_2M_Sounding_N and
RTT_2M_Random_Sequence_N are absent, which is exactly the 3 octets between
Zephyr's 32-octet reply and the spec's 35. Zephyr is also where the singular
t_sw_ipt_time_supported spelling came from.
bt_hci_rp_le_read_local_supported_capabilities_v2
https://github.com/zephyrproject-rtos/zephyr/blob/62acbd571c7294d899861a507e34533e16da7758/include/zephyr/bluetooth/hci_types.h#L2730-L2777
bt_hci_evt_le_cs_read_remote_supported_capabilities_complete_v2
https://github.com/zephyrproject-rtos/zephyr/blob/62acbd571c7294d899861a507e34533e16da7758/include/zephyr/bluetooth/hci_types.h#L4155-L4204
(bt_hci_cp_le_write_cached_remote_supported_capabilities_v2 in the same header
is short the same three fields, so this is Zephyr's consistent shape rather
than a one-off.) The comments citing Nordic header line numbers are replaced
with the spec section, since the spec is what the code should be read against.
One consequence worth recording: a controller built on those definitions
advertises octet 49 bit 2 but replies with 32 octets, which now fails to parse
instead of being silently accepted.
Review feedback: the V2 structures called this parameter rtt_random_payload_n, but Core v6.3 names it RTT_Random_Sequence_N in the [v2] variants just as it does in the 6.0 ones (§ 7.8.130, § 7.7.65.39). The payload_n spelling was taken from Zephyr, which uses it in every one of these structs. Rename it to match the spec. With the name shared across all four payload shapes, from_hci() no longer has to normalize it: the field is read straight off the report like every other 6.0 parameter, and the isinstance branch is left doing only what it should, which is collecting the V2 tail. That also removes the reason the remote-capabilities handler was split in two. Host.on_hci_le_cs_read_remote_supported_capabilities_complete_v2_event now emits 'cs_remote_supported_capabilities' rather than a separate _v2 event, since the V2 payload is a superset of the 6.0 one and every field a listener reads is present in both. Device keeps a single on_cs_remote_supported_capabilities annotated with the union of the two event types, so the outer V1/V2 dispatch pair and the _impl builder they shared both go away. This also addresses the earlier review comment asking for exactly that shape. Verified by decoding a synthesized subevent 0x38 off the wire: it round-trips byte for byte, from_hci() reads rtt_random_sequence_n and the 2M fields from it, and the 6.0 subevent 0x2C still decodes with the V2 tail left at zero.
Review feedback: Core v6.3 § 7.8.137 calls the trailing byte of LE CS Create Config `CS_Enhancements`, with bit 0 documented as "IPT enabled in the CS reflector". This branch called it cs_enhancements_1, following Zephyr's bt_hci_cp_le_cs_create_config and bt_conn_le_cs_config rather than the spec. Rename the field to cs_enhancements and the flag enum to CsEnhancements, and restate bit 0 in the spec's own words. Device.create_cs_config's keyword follows, keeping its 0x00 default so 6.0 behavior is unchanged when a caller says nothing. The _1 suffix was never meaningful outside Zephyr's naming: the spec defines a single CS_Enhancements parameter, not a numbered series, so nothing is lost by dropping it. Callers passing the old keyword will need to rename it; there is no in-tree caller other than create_cs_config itself.
Review feedback: the two V2 docstrings pointed at the wrong sections. The V2 command and event don't have sections of their own — each is documented in the same section as the 6.0 variant it extends: 7.8.130 LE CS Read Local Supported Capabilities command [v2] 7.7.65.39 LE CS Read Remote Supported Capabilities Complete event [v2] Those are the numbers the adjacent 6.0 classes already carry, so each pair now agrees instead of sending a reader to two different places for one section. Also move the variant marker into the spec's own "[v2]" form rather than spelling it into the command name, matching how the spec titles these. Docstrings only; no behavior change.
9413603 to
bd1813e
Compare
|
Thanks — all four addressed, and the branch is rebased onto
One thing worth flagging: Zephyr's structures stop after |
|
Followed up on the short-reply concern with a measurement, and it resolves in the spec's favour — worth recording here. Captured the raw command-complete return parameters off an nRF54L15 ( The [v2] reply is byte-identical to [v1] for its first 29 octets, then carries the full tail: Zephyr's structs are still 32 octets, but that's host-side only; no controller I have replies short, so I've left the parse strict. One correction to item 1: the tail is 6 octets, not 7 (2+1+1+1+1). Your 35-octets-including-Status figure is right — 29 + 6. I'd repeated the 7 in the PR description and have fixed it there. |
2 participants
Summary
Adds Bluetooth 6.3 Channel Sounding Enhancements support to Bumble — specifically the Inline Phase Correction Term transfer (IPT) feature.
IntFlagenums, supports-commands mask, host dispatch handler. One breaking rename insideHCI_LE_CS_Create_Config_Command: the trailingreservedbyte becomescs_enhancements(see notes).Device.power_on()prefers the [v2] capabilities read when the controller advertises it and falls back to the 6.0 read otherwise;create_cs_config()gains acs_enhancementskwarg, default 0.Net change: +225 / −50 across three files (
hci.py,device.py,host.py).What's implemented
HCI_LE_CS_READ_LOCAL_SUPPORTED_CAPABILITIES_V2_COMMAND0x20A5— new opcode; return parameters append the [v2] tail (6 octets, below).HCI_LE_CS_WRITE_CACHED_REMOTE_SUPPORTED_CAPABILITIES_V2_COMMAND0x20A6— opcode constant only (no in-tree caller yet).HCI_LE_CS_READ_REMOTE_SUPPORTED_CAPABILITIES_COMPLETE_V2_EVENT0x38— 6.3 variant of0x2Cwith the same [v2] tail.HCI_LE_CS_Read_Local_Supported_Capabilities_V2_Command/_V2_ReturnParametersHCI_LE_CS_Read_Remote_Supported_Capabilities_Complete_V2_EventCsSubfeature.CS_IPT_REFLECTOR1 << 4— bit insubfeatures_supportedadvertising IPT reflector capability.CsEnhancements.INLINE_PCT0x01— bit 0 of theCS_Enhancementsbyte ofHCI_LE_CS_Create_Config, enabling IPT in the CS reflector.HCI_LE_CS_Create_Config_Command.reserved→cs_enhancementsDevice.create_cs_config(..., cs_enhancements=0)Device.power_on()CS bring-upsupports_command()reports it, else the 6.0 read.ChannelSoundingCapabilitiest_ip2_ipt_times_supported,t_sw_ipt_times_supported,rtt_2m_aa_only_n,rtt_2m_sounding_n,rtt_2m_random_sequence_n,cs_ipt_reflector_supported.The [v2] tail
Per §7.8.130 and §7.7.65.39, the [v2] payloads append 6 octets after
TX_SNR_Capability, taking the return parameters from 29 to 35 octets includingStatus:T_IP2_IPT_Times_SupportedT_SW_IPT_Times_SupportedRTT_2M_AA_Only_NRTT_2M_Sounding_NRTT_2M_Random_Sequence_NRTT_Random_Sequence_Nkeeps its 6.0 name in the [v2] structures, so all four payload shapes (local [v1]/[v2] return parameters, remote [v1]/[v2] events) share the 6.0 field names and differ only by this tail.The
reserved→cs_enhancementsrenameThe trailing byte of
HCI_LE_CS_Create_Configwas reserved in 6.0. Bluetooth 6.3 §7.8.137 names itCS_Enhancementsand assigns bit 0 as IPT enabled in the CS reflector.Impact on downstream callers: the field is now named
cs_enhancementsrather thanreserved. Callers that were passingreserved=0x00explicitly must update tocs_enhancements=0x00(or drop the kwarg entirely — the default is0). No known in-tree caller does this —Device.create_cs_configis updated here.Design notes
supports_command()gate for the [v2] read —Device.power_on()selects between the 6.0 and [v2] capability reads on octet 49 bit 2 of the LE Supported Commands reply, matching thesupports_command()check used forHCI_LE_SET_HOST_FEATURE_COMMANDa few lines above.Hostemitscs_remote_supported_capabilitiesfor both subevents andDevicekeeps a single handler annotated with the union of the two event types. Existing listeners are unaffected.ChannelSoundingCapabilities.from_hci()— one classmethod builds the record from any of the four HCI payload shapes, so the 20-plus-argument constructor isn't repeated per call site. The [v2] tail fields default to0/False, keeping existing positional constructor calls working.Testing
Verified end-to-end against Nordic hardware:
hci_uartfrom nRF Connect SDK v3.4.0 (withCONFIG_BT_CTLR_EXTENDED_FEAT_SET=y), attached as Bumble's HCI transport over USB serial.channel_sounding/ipt_reflectorsample from nRF Connect SDK v3.4.0.Observed:
t_ip2_ipt_times_supported = 0x7E(bits 1–6 set → 10/20/30/40/50/60 μs),t_sw_ipt_times_supported = 0x0A(10 μs),subfeatures_supported = 0x12(bit 4 →CS_IPT_REFLECTORset).HCI_LE_CS_Create_Configwith the CS Enhancements byte set to0x01completed withstatus: SUCCESS.HCI_LE_CS_Config_Complete_Eventechoed the byte back as1.HCI_LE_CS_Procedure_Enable_Complete_Eventreturnedstate: ENABLED.HCI_LE_CS_Subevent_Result_Events streamed withprocedure_counterincrementing normally.Raw reply lengths, measured
Re-measured on the same nRF54L15 after the field list was corrected, capturing the raw command-complete return parameters for both opcodes:
The [v2] reply is byte-identical to the [v1] reply for its first 29 octets and then carries the full 6-octet tail, decoding as
t_ip2_ipt_times_supported = 0x7E,t_sw_ipt_times_supported = 0x0A,rtt_2m_aa_only_n = 30,rtt_2m_sounding_n = 0,rtt_2m_random_sequence_n = 30. The first two match the values this PR originally recorded, so the controller had been sending the full tail all along — the previous field list was discarding the last 6 octets, which Bumble ignores silently as trailing data.Parsing is additionally covered by decoding a synthesized subevent
0x38and checking it re-serializes byte for byte, and by confirming the 6.0 payloads still decode with the [v2] tail left at zero.Note on Zephyr's struct definitions
Worth flagging for anyone reading these structures against Zephyr rather than the spec: Zephyr's CS capability structs stop after
t_sw_ipt_time_supportedand omit the threeRTT_2M_*parameters, describing a 32-octet reply.bt_hci_rp_le_read_local_supported_capabilities_v2bt_hci_evt_le_cs_read_remote_supported_capabilities_complete_v2bt_hci_cp_le_write_cached_remote_supported_capabilities_v2in the same header is short the same three fields.That is a host-side header, and the measurement above shows the shipping controller replies with the spec's 35 octets, so it doesn't affect this PR. It is only worth knowing that a short reply from some other controller would raise during parsing rather than being rejected cleanly. Happy to add a tolerant tail parse or a fallback to the 6.0 read if you'd rather Bumble degrade there, but I've no controller that needs it.
Spec references
CS_Enhancements, bit 0 = IPT enabled in the CS reflector.Follow-ups (not in this PR)
HCI_LE_CS_Write_Cached_Remote_Supported_Capabilities_V2_Commanddataclass (opcode constant is defined; body not wired). §7.8.132 gives it the same [v2] tail. Uncommon path in practice.CS_Enhancementsremain reserved).