feat: add jumpstarter-driver-bt-peer - #986
Open
bennyz wants to merge 1 commit into
Open
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new Bumble-backed Bluetooth peer driver with lifecycle, pairing, connection, event, and AVDTP operations. It also adds a typed client, CLI commands, package integration, exporter configuration, tests, and reference documentation. ChangesBT Peer driver
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BtPeerClient
participant BtPeer
participant BumbleDevice
participant BluetoothConnection
BtPeerClient->>BtPeer: start_peer(config_json)
BtPeer->>BumbleDevice: Open transport and power on device
BumbleDevice->>BtPeer: Return address and name
BluetoothConnection->>BumbleDevice: Establish connection
BumbleDevice->>BtPeer: Emit connection event
BtPeerClient->>BtPeer: pair(handle) or get_connections()
BtPeerClient->>BtPeer: stop_peer()
BtPeer->>BumbleDevice: Power off device
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/reference/package-apis/drivers/bt-peer.md`:
- Around line 4-6: Update the capability description for BtPeer.start_peer() to
document only A2DP audio-source support over BR/EDR; remove claims for HFP,
PBAP, LE, and any unsupported profile or transport behavior, while retaining
only capabilities implemented by the driver.
- Line 3: Update the Bumble hyperlink in the package description to use the
canonical Google Bumble repository URL, while preserving the existing
description and link text.
In `@python/packages/jumpstarter-driver-bt-peer/.gitignore`:
- Around line 1-3: Add htmlcov/ to the package’s .gitignore alongside the
existing coverage artifacts so HTML coverage output generated by the test
configuration is ignored.
In `@python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py`:
- Around line 128-151: Enforce a single caller deadline across the complete
Bluetooth flow: in
python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py lines
128-151, reject expiration before waits or sleeps and replace fixed delays with
remaining-time checks; in lines 164-188, bound confirmation, connection waiting,
and encryption polling by the same deadline; in lines 191-205, reserve time for
encryption polling or propagate one shared deadline so no operation continues
after timeout. Update the relevant bonding, connection, and encryption helper
flows without changing successful-result behavior.
- Line 171: Update the tap_text command construction in the Bluetooth helper
around _adb_shell so the caller-controlled name is safely shell-quoted as a
single argument before being interpolated into the command. Preserve the
existing UI_HELPER invocation and “last” argument while preventing shell syntax
in name from executing on the AAOS device.
In `@python/packages/jumpstarter-driver-bt-peer/examples/bt_ui_helper.sh`:
- Line 36: Update the MATCHES assignment in bt_ui_helper.sh to make grep treat
TARGET_TEXT as literal text rather than a regular expression, while preserving
the existing XML filtering behavior.
In `@python/packages/jumpstarter-driver-bt-peer/examples/demo_commands.md`:
- Line 3: Update
python/packages/jumpstarter-driver-bt-peer/examples/demo_commands.md lines 3-3
and 7-7 to use python/ instead of cuttlefish-driver/python/ when describing the
working directory. Ensure the documented uv commands run from the top-level
Python workspace managed by python/pyproject.toml.
- Around line 180-183: Update the demo_commands bonding procedure so that, after
restarting the peer, it repeats the bonding step from step 7 before repeating
steps 8 through 10; revise the surrounding instruction to explicitly reference
steps 7–10 and preserve the existing new-address flow.
In `@python/packages/jumpstarter-driver-bt-peer/examples/demo_plan.md`:
- Around line 5-27: Add the text language identifier to the fenced code block
containing the Jumpstarter Exporter architecture diagram, changing the opening
fence to specify text while preserving the diagram content unchanged.
- Around line 175-193: Add an asyncio entry point after the demo coroutine
definition that invokes demo via asyncio.run, ensuring the copied example
executes the documented pairing flow instead of only defining demo.
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.py`:
- Around line 4-16: Add lifecycle coverage around BtPeer by starting and
stopping it with a controlled Bumble transport, then exercise connection and
pairing operations through the public client path. Use mocks for unavailable
transport or platform-dependent behavior while preserving assertions for
successful client interactions and clean shutdown.
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py`:
- Around line 149-197: Make start_peer exception-safe by wrapping the startup
sequence after _transport is opened in cleanup handling. On any startup failure,
power off the created _device if present, close _transport, clear both fields,
and re-raise the original exception so a later start_peer call can retry.
- Around line 232-248: Update wait_connection to inspect the existing
_connections collection before registering the “connection” listener and waiting
on the event; immediately return an active tracked connection when available,
while preserving the current event-based wait and timeout behavior when none
exists.
In `@python/packages/jumpstarter-driver-bt-peer/pyproject.toml`:
- Around line 24-26: Update the source_archive value in the Hatch VCS URL
metadata to use the canonical jumpstarter-dev/jumpstarter repository path while
preserving the existing commit_hash archive format.
In `@python/packages/jumpstarter-driver-bt-peer/README.md`:
- Around line 23-45: Replace the optional template sections in the README with
driver-specific Usage and, where applicable, Architecture or Troubleshooting
documentation. Cover peer startup and shutdown, transport requirements, pairing
behavior, connection-waiting behavior, and every supported operation exported by
BtPeer, while retaining the API Reference.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1ad52c6-90b8-4f84-873a-14ce3f223034
📒 Files selected for processing (15)
docs/source/reference/package-apis/drivers/bt-peer.mddocs/source/reference/package-apis/drivers/index.mdpython/packages/jumpstarter-driver-bt-peer/.gitignorepython/packages/jumpstarter-driver-bt-peer/README.mdpython/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.pypython/packages/jumpstarter-driver-bt-peer/examples/bt_ui_helper.shpython/packages/jumpstarter-driver-bt-peer/examples/demo_commands.mdpython/packages/jumpstarter-driver-bt-peer/examples/demo_plan.mdpython/packages/jumpstarter-driver-bt-peer/examples/exporter.yamlpython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/__init__.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.pypython/packages/jumpstarter-driver-bt-peer/pyproject.tomlpython/pyproject.toml
Comment on lines
+4
to
+6
| It can pair, connect, and stream profiles (A2DP, HFP, PBAP) to a DUT over | ||
| BR/EDR or LE. Transport-agnostic: works over TCP (rootcanal/netsim), USB | ||
| dongle, serial UART, or any bumble transport string. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py --items all
rg -n -C 3 'A2DP|HFP|PBAP|LE|GATT|sdp_service_records|Service' \
python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 5048
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l docs/source/reference/package-apis/drivers/bt-peer.md python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py
sed -n '1,60p' docs/source/reference/package-apis/drivers/bt-peer.md
sed -n '1,240p' python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py
printf '\n--- package files ---\n'
fd -a . python/packages/jumpstarter-driver-bt-peer | sed 's#^\./##'
rg -n -C 2 'HFP|SPP|PBAP|OBEX|GATT|profile|transport_string|connect_to|pair' python/packages/jumpstarter-driver-bt-peer python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peerRepository: jumpstarter-dev/jumpstarter
Length of output: 38474
Document only the supported BT profiles.
BtPeer.start_peer() advertises only an A2DP audio-source SDP record and AVDTP_AUDIO_MEDIA_TYPE streaming. HFP, PBAP, and LE profile support are not implemented in the driver, so do not list them as supported in docs/source/reference/package-apis/drivers/bt-peer.md.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/reference/package-apis/drivers/bt-peer.md` around lines 4 - 6,
Update the capability description for BtPeer.start_peer() to document only A2DP
audio-source support over BR/EDR; remove claims for HFP, PBAP, LE, and any
unsupported profile or transport behavior, while retaining only capabilities
implemented by the driver.
Comment on lines
+128
to
+151
| remaining = max(5, int(deadline - time.time())) | ||
|
|
||
| def bond_complete(): | ||
| result = _bthelper(adb, "bond-state", address) | ||
| return "result=12" in result | ||
|
|
||
| if not _poll_until(bond_complete, timeout=min(30, remaining)): | ||
| raise RuntimeError(f"bonding failed for {address}") | ||
|
|
||
| conn = bt_peer.wait_connection(timeout=max(5, int(deadline - time.time()))) | ||
| if isinstance(conn, str): | ||
| conn = json.loads(conn) | ||
|
|
||
| _bthelper(adb, "fetchuuids", address) | ||
| time.sleep(8) | ||
| _bthelper(adb, "connect", address) | ||
| time.sleep(5) | ||
|
|
||
| conns = bt_peer.get_connections() | ||
| if isinstance(conns, str): | ||
| conns = json.loads(conns) | ||
| if conns: | ||
| return conns[-1] | ||
| return conn |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce the caller timeout across the complete flow.
max(5, ...), fixed sleeps, and post-connection polling continue after deadline expires. This violates the documented total timeout and can delay failed test runs.
python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py#L128-L151: reject an expired deadline before waiting or sleeping.python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py#L164-L188: bound confirmation, connection waiting, and encryption polling by the remaining deadline.python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py#L191-L205: reserve part oftimeoutfor encryption polling or use one shared deadline.
📍 Affects 1 file
python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py#L128-L151(this comment)python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py#L164-L188python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py#L191-L205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py`
around lines 128 - 151, Enforce a single caller deadline across the complete
Bluetooth flow: in
python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py lines
128-151, reject expiration before waits or sleeps and replace fixed delays with
remaining-time checks; in lines 164-188, bound confirmation, connection waiting,
and encryption polling by the same deadline; in lines 191-205, reserve time for
encryption polling or propagate one shared deadline so no operation continues
after timeout. Update the relevant bonding, connection, and encryption helper
flows without changing successful-result behavior.
| ) | ||
| time.sleep(3) | ||
|
|
||
| result = _adb_shell(adb, f'sh {UI_HELPER} tap_text "{name}" last') |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Quote the Bluetooth device name before calling adb shell.
name is caller-controlled. A value containing shell syntax can execute arbitrary commands on the AAOS device as the Android shell user. Quote name as one shell argument before building this command.
Proposed fix
+import shlex
+
- result = _adb_shell(adb, f'sh {UI_HELPER} tap_text "{name}" last')
+ result = _adb_shell(
+ adb,
+ f"sh {UI_HELPER} tap_text {shlex.quote(name)} last",
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| result = _adb_shell(adb, f'sh {UI_HELPER} tap_text "{name}" last') | |
| import shlex | |
| result = _adb_shell( | |
| adb, | |
| f"sh {UI_HELPER} tap_text {shlex.quote(name)} last", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/packages/jumpstarter-driver-bt-peer/examples/aaos_bt_helpers.py` at
line 171, Update the tap_text command construction in the Bluetooth helper
around _adb_shell so the caller-controlled name is safely shell-quoted as a
single argument before being interpolated into the command. Preserve the
existing UI_HELPER invocation and “last” argument while preventing shell syntax
in name from executing on the AAOS device.
bennyz
changed the title
bennyz
force-pushed
the
worktree-bt-peer-driver
branch
from
August 11, 2026 05:06
b2aa0d6 to
d9bbf08
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.py (1)
500-519: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover cleanup after device initialization.
Lines 510-512 fail before
peer._deviceis assigned. This test verifies transport cleanup only. Makemock_device.power_onraise instead. Then assert thatmock_device.power_off()is awaited. This executes the cleanup branch for a created device.Proposed test adjustment
+ mock_device = _make_mock_device() + mock_device.power_on = AsyncMock(side_effect=RuntimeError("power on failed")) + with ( patch( "jumpstarter_driver_bt_peer.driver.open_transport", new=AsyncMock(return_value=mock_transport), ), patch( "jumpstarter_driver_bt_peer.driver.Device", - side_effect=RuntimeError("device init failed"), + return_value=mock_device, ), + patch("jumpstarter_driver_bt_peer.driver.Listener") as mock_listener_cls, ): - with pytest.raises(RuntimeError, match="device init failed"): + mock_listener_cls.for_device.return_value = MagicMock() + with pytest.raises(RuntimeError, match="power on failed"): await peer.start_peer() assert peer._device is None assert peer._transport is None + mock_device.power_off.assert_awaited_once() mock_transport.close.assert_called_once()As per coding guidelines, “Provide comprehensive package test coverage, prioritizing end-to-end tests that start a server and client; use mocks when system tools, services, or platform compatibility make end-to-end testing impractical.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.py` around lines 500 - 519, Update test_start_peer_exception_cleanup to return a mock_device from Device, make mock_device.power_on raise the expected RuntimeError, and preserve the transport mock setup. Assert mock_device.power_off was awaited once alongside the existing transport close and state cleanup assertions, exercising cleanup after device creation.Source: Coding guidelines
python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.py (2)
98-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a numeric type for
--since.The driver calls
float(since)on this value. A non-numeric value raises an unhandledValueErrorin the driver. Declare the option as a float and convert to string at the call site.♻️ Proposed refactor
`@bt_peer.command`("events") - `@click.option`("--since", default="0", help="Timestamp filter") - def events_cmd(since: str): + `@click.option`("--since", type=float, default=0.0, help="Timestamp filter (epoch seconds)") + def events_cmd(since: float): """Show events since timestamp.""" - _echo(self.get_events(since)) + _echo(self.get_events(str(since)))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.py` around lines 98 - 102, Update the --since option in events_cmd to use a numeric float type, then convert the parsed value to a string when passing it to self.get_events. Preserve the existing default timestamp behavior while preventing non-numeric input from reaching the driver.
72-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate CONFIG as JSON in the client.
start_cmdforwards CONFIG unchanged. The driver callsjson.loadson it. A typo in the CLI argument produces a remote driver error instead of a local usage error. Parse the value locally and fail early with a clear message.♻️ Proposed refactor
`@bt_peer.command`("start") `@click.argument`("config", default="{}") def start_cmd(config: str): """Start the BT peer device. CONFIG is JSON: {"name": "...", "classic_enabled": true, "class_of_device": 123} """ + try: + json.loads(config) + except json.JSONDecodeError as exc: + raise click.BadParameter(f"CONFIG must be valid JSON: {exc}") from exc _echo(self.start_peer(config))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.py` around lines 72 - 79, Update start_cmd to parse CONFIG locally as JSON before calling start_peer, and reject malformed input immediately with a clear Click usage error. Pass the validated configuration onward while preserving the existing start command behavior for valid JSON.python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py (2)
344-367: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the outgoing connection.
self._device.connect(...)at Line 359 has no bound. If the remote device does not answer, this exported call blocks the driver until the transport fails. Every other wait operation in this driver accepts atimeout. Add the same option here.♻️ Proposed refactor
- async def connect_to(self, address: str) -> str: + async def connect_to(self, address: str, timeout: int = 30) -> str: @@ - connection = await self._device.connect(target, transport=BT_BR_EDR_TRANSPORT) + try: + with anyio.fail_after(timeout): + connection = await self._device.connect(target, transport=BT_BR_EDR_TRANSPORT) + except TimeoutError: + raise BtPeerError(f"connection to {address} not established within {timeout}s") from NoneThe client method
connect_toinclient.pyneeds the matching parameter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py` around lines 344 - 367, Update connect_to to accept a timeout parameter and pass it to self._device.connect alongside the existing target and transport arguments, preserving the current connection response behavior. Update the matching client-side connect_to method to expose and forward the same timeout option.
101-126: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winYield timestamped packet objects from
silence()
MediaPacketPumpreadspacket.timestamp_secondsbefore callingbytes(packet), soyield b""raisesAttributeErrorwhen the first item arrives. The 3600-second wait is cancellable, but it does not provide a valid silence packet. Yield timestamped packet objects accepted by Bumble, and moveMediaPacketPumpto the module-level imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py` around lines 101 - 126, Update _on_avdtp_connection so silence() yields Bumble-compatible timestamped packet objects rather than raw bytes, preserving the cancellable periodic wait and empty-audio payload. Move MediaPacketPump from the local import to the module-level imports, then construct the pump with the updated silence generator.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py`:
- Around line 370-374: Update get_events to catch conversion failures from
float(since) and raise BtPeerError instead, preserving the existing event
filtering and JSON serialization behavior for numeric values.
- Around line 280-312: Update wait_disconnection to retain each registered
handler and remove those handlers from their connections in cleanup that runs
after both successful waits and TimeoutError paths. Preserve the existing result
and timeout behavior while ensuring repeated calls do not accumulate listeners
or append to discarded result holders.
- Around line 216-232: Update stop_peer to await the asynchronous
Transport.close() call in both transport cleanup paths, preserving the existing
cleanup order and state resets. Update the transport test double to expose close
as an AsyncMock, and assert it was awaited exactly once.
---
Nitpick comments:
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.py`:
- Around line 98-102: Update the --since option in events_cmd to use a numeric
float type, then convert the parsed value to a string when passing it to
self.get_events. Preserve the existing default timestamp behavior while
preventing non-numeric input from reaching the driver.
- Around line 72-79: Update start_cmd to parse CONFIG locally as JSON before
calling start_peer, and reject malformed input immediately with a clear Click
usage error. Pass the validated configuration onward while preserving the
existing start command behavior for valid JSON.
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.py`:
- Around line 500-519: Update test_start_peer_exception_cleanup to return a
mock_device from Device, make mock_device.power_on raise the expected
RuntimeError, and preserve the transport mock setup. Assert
mock_device.power_off was awaited once alongside the existing transport close
and state cleanup assertions, exercising cleanup after device creation.
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py`:
- Around line 344-367: Update connect_to to accept a timeout parameter and pass
it to self._device.connect alongside the existing target and transport
arguments, preserving the current connection response behavior. Update the
matching client-side connect_to method to expose and forward the same timeout
option.
- Around line 101-126: Update _on_avdtp_connection so silence() yields
Bumble-compatible timestamped packet objects rather than raw bytes, preserving
the cancellable periodic wait and empty-audio payload. Move MediaPacketPump from
the local import to the module-level imports, then construct the pump with the
updated silence generator.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c6841157-a833-497d-a0bd-96a5fce3e040
📒 Files selected for processing (7)
docs/source/reference/package-apis/drivers/bt-peer.mdpython/packages/jumpstarter-driver-bt-peer/.gitignorepython/packages/jumpstarter-driver-bt-peer/README.mdpython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.pypython/packages/jumpstarter-driver-bt-peer/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (4)
- python/packages/jumpstarter-driver-bt-peer/.gitignore
- python/packages/jumpstarter-driver-bt-peer/README.md
- docs/source/reference/package-apis/drivers/bt-peer.md
- python/packages/jumpstarter-driver-bt-peer/pyproject.toml
bennyz
force-pushed
the
worktree-bt-peer-driver
branch
from
August 11, 2026 08:55
d9bbf08 to
f3dd87a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py`:
- Around line 164-167: Require peer configuration to decode as a JSON object
before accessing fields: in
python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py
lines 164-167, catch JSON decoding failures and raise BtPeerError for non-dict
decoded values; in
python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.py
lines 79-86, reject valid non-object JSON values with click.BadParameter so CLI
validation reports a structured error.
- Around line 244-253: Update stop_peer cleanup around _device.power_off() and
_transport.close() so each resource reference is detached before its cleanup
operation and each operation runs in a finally block. Ensure
_connections.clear(), _events.clear(), and the peer_stopped emission still occur
after both attempts, while preserving and re-raising the original cleanup error
after all cleanup has been attempted.
- Around line 318-340: Fix the Ruff B023 closure issue in the
listener-registration loop by avoiding a handler closure that captures the loop
variable `conn`; bind the connection explicitly through the handler factory or
equivalent parameter pattern while preserving the recorded result and listener
cleanup behavior. Run `make lint-fix` to apply the required Ruff formatting and
lint fixes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e49b4f7e-933f-4e9e-803a-eb26a920f5de
📒 Files selected for processing (5)
python/packages/jumpstarter-driver-bt-peer/README.mdpython/packages/jumpstarter-driver-bt-peer/examples/exporter.yamlpython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/client.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.pypython/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- python/packages/jumpstarter-driver-bt-peer/README.md
- python/packages/jumpstarter-driver-bt-peer/examples/exporter.yaml
Comment on lines
+244
to
+253
| self._avdtp_listener = None | ||
| if self._device: | ||
| await self._device.power_off() | ||
| self._device = None | ||
| if self._transport: | ||
| await self._transport.close() | ||
| self._transport = None | ||
| self._connections.clear() | ||
| self._events.clear() | ||
| self._emit("peer_stopped", {}) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Complete cleanup when stop_peer fails.
If power_off() raises, the method skips transport closure and all state cleanup. If close() raises, it skips connection and event cleanup. The next start_peer call can then fail as already running or discard an open transport reference.
Detach the device and transport state, then run both cleanup operations in finally blocks. Preserve the original cleanup error after both cleanup attempts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@python/packages/jumpstarter-driver-bt-peer/jumpstarter_driver_bt_peer/driver.py`
around lines 244 - 253, Update stop_peer cleanup around _device.power_off() and
_transport.close() so each resource reference is detached before its cleanup
operation and each operation runs in a finally block. Ensure
_connections.clear(), _events.clear(), and the peer_stopped emission still occur
after both attempts, while preserving and re-raising the original cleanup error
after all cleanup has been attempted.
Bluetooth peer driver powered by Bumble for emulating BT devices in hardware testing scenarios Assisted-by: claude-opus-4.6 Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
bennyz
force-pushed
the
worktree-bt-peer-driver
branch
from
August 11, 2026 09:11
f3dd87a to
59ff270
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bluetooth peer driver powered by Bumble for emulating BT devices in hardware testing scenarios