CI only: #2021 fix plus regression test (expected to pass) - #2
Open
cgoudie wants to merge 7 commits into
Open
Conversation
InProgress on StopDiscovery is bluetoothd relaying MGMT_STATUS_REJECTED from the kernel, which it returns when discovery is already not active. bluetoothd removes the client's discovery session before replying, so by the time bleak sees the error there is nothing left to stop; raising only turns a completed scan into an exception the caller cannot retry, since the scanner nulls _stop before awaiting it. The usual trigger is the kernel's own LE scan timeout: an LE-only scan is stopped after DISCOV_LE_TIMEOUT (10.24 s) and re-armed by bluetoothd IDLE_DISCOV_TIMEOUT (5 s) later, and a stop landing in that gap is rejected. find_device_by_address with a 15 s timeout on an absent device hits it essentially every time on BlueZ 5.72. Treat it like NotReady, which this branch already swallows. The integration test scans for 12 s so that the stop lands inside the gap. Fixes hbldh#2021
…n the test The kernel rejects StopDiscovery whenever it is not actively scanning, which includes the moments while BlueZ is restarting the scan after the kernel's own LE timeout, not only after it has stopped. In the idle gap itself BlueZ already knows the kernel is idle and stops cleanly, so the window is narrow and the failure is intermittent in the field. The test now stops at several points around the restart instead of one.
It cannot fail on unpatched code. The rejection needs bluetoothd to forward a stop after the kernel has left the FINDING state but before bluetoothd has processed the kernel's Discovering(false) event, and on a healthy system that window is a scheduling race, not a time offset: stops at 10, 12, 15, 15.2, 15.4 and 16 s after start were accepted 18 of 18 times on BlueZ 5.72, as were five find_device_by_address timeouts. The in-field occurrences (24 in one day) were under a crash-looping bluetoothd and have not recurred since it was fixed.
…he BlueZ path in the comment A faked-reply test rather than an integration test, because the condition cannot be produced on demand: bluetoothd only forwards a client's stop to the kernel while its own discovery_enable flag is set, and it clears that flag when it processes the kernel's Discovering(false) event, so the kernel's rejection needs a stop to land in the gap between the two. That is a scheduling race, not a time offset a test can aim at; on BlueZ 5.72 stops at 10, 12, 15, 15.2, 15.4 and 16 s after start were accepted 18 of 18 times. The test fakes the bus reply and pins bleak's handling: stop() returns, the callbacks are removed, and any other error still raises. The code comment now names the bluetoothd path (stop_discovery_complete removes the session before checking the status, then relays MGMT_STATUS_REJECTED as InProgress), as requested in the review.
CI failed on the typecheck subtask, not the tests: mypy flagged Message.member as str | None where the fake bus appended it to a list[str]. Annotate the recorded members, give the no-op scan callbacks their AdvertisementCallback / DeviceRemovedCallback types, and type the empty filter dict, so both mypy and pyright (strict) pass. Guard the module to Linux the way the other bluezdbus unit tests are, since it imports dbus_fast, which is a Linux-only dependency.
A single rejection is a race with the kernel's own scan timeout and the next stop on the adapter succeeds, so it is logged at INFO and tolerated. Two in a row with no clean stop between them cannot come from that race: bluetoothd has lost track of the kernel's scan state and no scan on the adapter reaches the kernel until it is reset. That one is logged at WARNING with the remedy and the original error is raised. Documents the condition and its limits in the troubleshooting guide.
…the other tests do pyright strict on Linux flags the test's reads of the manager's private callback and property stores. Read them once through helpers annotated with the repository's usual pyright ignore comment.
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.
Demonstration run for hbldh#2022: the fix and the regression test together. The BlueZ integration job is expected to PASS. Not for merge; the real PR is upstream.