-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
support windows (using Bumble) for hearing aid #333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
uses bumble
WalkthroughA new comprehensive Python script implementing a PyQt5-based GUI application for controlling hearing aid devices via Bluetooth. The script includes ATT/AACP protocol managers, BlueZ/Bumble stack support, hearing aid settings parsing, signal emission for UI updates, and CLI integration for device connection and control. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant QtUI as PyQt5 UI
participant SignalEmitter as Signal Emitter
participant MainThread as Main/Event Loop
participant AACPMgr as AACP Manager
participant ATTMgr as ATT Manager
participant BluezCh as BlueZ Channel
participant Device as Hearing Aid Device
User->>QtUI: Launch app with device address
QtUI->>MainThread: Create managers & UI, spawn async thread
MainThread->>BluezCh: Connect to device via BlueZ/Bumble
BluezCh->>Device: Establish PDU socket connection
Device-->>BluezCh: Connection established
MainThread->>AACPMgr: Send handshake
AACPMgr->>BluezCh: Send AACP handshake PDU
BluezCh->>Device: Transmit handshake
Device-->>BluezCh: Handshake response
MainThread->>ATTMgr: Enable notifications & read initial state
ATTMgr->>Device: Read ATT handles
Device-->>ATTMgr: Return hearing aid settings data
ATTMgr->>AACPMgr: Request notification enablement
MainThread->>SignalEmitter: Emit connected signal
SignalEmitter->>QtUI: Update UI with initial settings
QtUI-->>User: Render controls (amplification, tone, etc.)
User->>QtUI: Adjust amplification slider
QtUI->>AACPMgr: Send control command via signal
AACPMgr->>BluezCh: Format & queue AACP command PDU
BluezCh->>Device: Transmit control command
Device-->>BluezCh: Command ACK/notification
AACPMgr->>SignalEmitter: Emit UI update signal
SignalEmitter->>QtUI: Update UI to reflect applied setting
QtUI-->>User: Show confirmation (toggle/value changed)
User->>QtUI: Close application
QtUI->>MainThread: Trigger shutdown event
MainThread->>AACPMgr: Stop listener & async tasks
MainThread->>ATTMgr: Stop notification listener
BluezCh->>Device: Close PDU socket
MainThread->>QtUI: Exit event loop
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
windows/hearing-aid.py (6)
26-27: Unnecessary or misplacedsys.pathmodification.This
sys.path.insertappears after all imports have already been resolved, making it ineffective for the current file's imports. If it's intended to help with relative imports of sibling modules, it should be placed before those imports. If no such imports exist, consider removing this dead code.-import os -sys.path.insert(0, os.path.dirname(__file__))
317-328: Chain the exception for better debugging.When re-raising a new exception inside an
exceptblock, usefromto preserve the original traceback context. This aids debugging by showing the root cause.except Exception: - raise Exception("No response received") + raise Exception("No response received") from None
702-712: Fire-and-forget task may be garbage collected prematurely.The task created by
asyncio.create_task(handle_data())is not stored, risking premature garbage collection before completion. Store a reference to prevent this.+ _background_tasks = set() + def on_l2cap_connection(channel: ClassicChannel): logger.info("Incoming L2CAP connection on PSM %d", channel.psm) async def handle_data(): try: reader = _make_reader(channel) while True: data = await reader() print(f"Received PDU on PSM {channel.psm}: {data.hex() if data else 'None'}") except Exception as e: logger.info("L2CAP channel on PSM %d closed: %s", channel.psm, e) - asyncio.create_task(handle_data()) + task = asyncio.create_task(handle_data()) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard)
812-818: Uselogging.exceptionto include traceback in error logs.When logging errors within an
exceptblock,logging.exceptionautomatically includes the traceback, aiding debugging.except HCI_Error as e: if "PAIRING_NOT_ALLOWED_ERROR" in str(e): logger.error("Put your device into pairing mode and run the script again") else: - logger.error("HCI error: %s", e) + logger.exception("HCI error: %s", e) except Exception as e: - logger.error("Unexpected error: %s", e) + logger.exception("Unexpected error: %s", e)
850-861: Event loop is not properly closed on shutdown.The asyncio event loop created in
main()runs in a daemon thread and is never explicitly stopped or closed. This can lead to resource leaks or warnings. Consider adding cleanup logic.def run_async(): asyncio.set_event_loop(loop) - loop.run_until_complete(run_bumble(args.bdaddr, att_manager, aacp_manager, window)) + try: + loop.run_until_complete(run_bumble(args.bdaddr, att_manager, aacp_manager, window)) + finally: + loop.run_until_complete(loop.shutdown_asyncgens()) + loop.close()
709-709: Use logger instead of print for consistency.The codebase uses the logging framework throughout. Replace
print()withlogger.debug()orlogger.info()for consistent output handling.- print(f"Received PDU on PSM {channel.psm}: {data.hex() if data else 'None'}") + logger.debug(f"Received PDU on PSM {channel.psm}: {data.hex() if data else 'None'}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
windows/hearing-aid.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.6)
windows/hearing-aid.py
1-1: Shebang is present but file is not executable
(EXE001)
31-37: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
84-84: Do not catch blind exception: Exception
(BLE001)
89-89: Do not catch blind exception: Exception
(BLE001)
180-180: Do not catch blind exception: Exception
(BLE001)
228-228: Do not catch blind exception: Exception
(BLE001)
271-271: Do not catch blind exception: Exception
(BLE001)
303-303: Do not catch blind exception: Exception
(BLE001)
314-314: Do not catch blind exception: Exception
(BLE001)
326-326: Do not catch blind exception: Exception
(BLE001)
327-327: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
327-327: Create your own exception
(TRY002)
327-327: Avoid specifying long messages outside the exception class
(TRY003)
349-349: Do not catch blind exception: Exception
(BLE001)
562-562: Do not catch blind exception: Exception
(BLE001)
563-563: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
627-627: Do not catch blind exception: Exception
(BLE001)
628-628: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
685-685: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
695-695: Unused lambda argument: conn
(ARG005)
710-710: Do not catch blind exception: Exception
(BLE001)
712-712: Store a reference to the return value of asyncio.create_task
(RUF006)
814-814: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
816-816: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
817-817: Do not catch blind exception: Exception
(BLE001)
818-818: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
843-843: Unused function argument: signum
(ARG001)
843-843: Unused function argument: frame
(ARG001)
🔇 Additional comments (1)
windows/hearing-aid.py (1)
118-165: Parsing logic and offset arithmetic verified.The binary parsing correctly handles the 104-byte minimum structure with proper offset tracking for left/right EQ arrays (8 floats each), amplification, tone, conversation boost, ANR, and own-voice values.
| left_amp = amp + (0.5 - balance) * amp * 2 if balance < 0 else amp | ||
| right_amp = amp + (balance - 0.5) * amp * 2 if balance > 0 else amp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Balance calculation appears incorrect.
The current formula uses 0.5 - balance and balance - 0.5 with the * 2 multiplier, but balance is already normalized to [-1, 1]. This produces asymmetric and potentially extreme amplification values:
- At
balance = -1:left_amp = amp + 3*amp = 4*amp - At
balance = 1:right_amp = amp + amp = 2*amp
A typical stereo balance implementation would be:
- left_amp = amp + (0.5 - balance) * amp * 2 if balance < 0 else amp
- right_amp = amp + (balance - 0.5) * amp * 2 if balance > 0 else amp
+ # Balance: negative = favor left, positive = favor right
+ left_amp = amp * (1 - max(0, balance))
+ right_amp = amp * (1 + min(0, balance))📝 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.
| left_amp = amp + (0.5 - balance) * amp * 2 if balance < 0 else amp | |
| right_amp = amp + (balance - 0.5) * amp * 2 if balance > 0 else amp | |
| # Balance: negative = favor left, positive = favor right | |
| left_amp = amp * (1 - max(0, balance)) | |
| right_amp = amp * (1 + min(0, balance)) |
🤖 Prompt for AI Agents
In windows/hearing-aid.py around lines 616-617, the balance math treats
`balance` as if centered at 0.5 and produces asymmetric, extreme gains; replace
it with a standard mapping from balance in [-1, 1] to per-channel gains: compute
left_gain = (1 - balance) / 2 and right_gain = (1 + balance) / 2 and then set
left_amp = amp * left_gain and right_amp = amp * right_gain (if you want to
preserve center amplitude, multiply both gains by 2 or otherwise normalize
accordingly).
| left_eq = [float(input_box.text() or 0) for input_box in self.left_eq_inputs] | ||
| right_eq = [float(input_box.text() or 0) for input_box in self.right_eq_inputs] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing input validation for EQ values.
float(input_box.text() or 0) will raise ValueError if the user enters non-numeric text (e.g., "abc"). Add validation or use a try/except to handle invalid input gracefully.
- left_eq = [float(input_box.text() or 0) for input_box in self.left_eq_inputs]
- right_eq = [float(input_box.text() or 0) for input_box in self.right_eq_inputs]
+ def safe_float(text: str) -> float:
+ try:
+ return float(text) if text else 0.0
+ except ValueError:
+ return 0.0
+ left_eq = [safe_float(input_box.text()) for input_box in self.left_eq_inputs]
+ right_eq = [safe_float(input_box.text()) for input_box in self.right_eq_inputs]📝 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.
| left_eq = [float(input_box.text() or 0) for input_box in self.left_eq_inputs] | |
| right_eq = [float(input_box.text() or 0) for input_box in self.right_eq_inputs] | |
| def safe_float(text: str) -> float: | |
| try: | |
| return float(text) if text else 0.0 | |
| except ValueError: | |
| return 0.0 | |
| left_eq = [safe_float(input_box.text()) for input_box in self.left_eq_inputs] | |
| right_eq = [safe_float(input_box.text()) for input_box in self.right_eq_inputs] |
🤖 Prompt for AI Agents
In windows/hearing-aid.py around lines 619-620, the list comprehensions directly
calling float(...) can raise ValueError on non-numeric text; replace them with
safe parsing that validates each input (e.g., try/except or a small helper parse
function) so invalid strings are caught and handled—convert valid numeric
strings to float, fall back to 0.0 (or another sane default), and optionally
surface the error to the user/UI (highlight the offending input or show a
message) instead of letting the exception propagate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
windows/hearing-aid.py (2)
606-614: Balance-to-channel amplification mapping is likely incorrect and non‑invertibleThe mapping from
ampandbalancetoleft_amp/right_ampis asymmetric and hard to reconcile with howparse_hearing_aid_settingsderivesnet_ampandbalance:avg = (left_amp + right_amp) / 2 diff = right_amp - left_amp amplification = max(-1, min(1, avg)) balance = max(-1, min(1, diff))but when sending:
left_amp = amp + (0.5 - balance) * amp * 2 if balance < 0 else amp right_amp = amp + (balance - 0.5) * amp * 2 if balance > 0 else ampThis does not invert the above relation and produces odd edge cases (e.g., extreme gains and discontinuities around 0). A more consistent approach is to solve for
left_amp/right_ampfrom(avg, diff):- left_amp = amp + (0.5 - balance) * amp * 2 if balance < 0 else amp - right_amp = amp + (balance - 0.5) * amp * 2 if balance > 0 else amp + # balance in [-1, 1] represents (right_amp - left_amp), amp is the average + left_amp = max(-1.0, min(1.0, amp - balance / 2.0)) + right_amp = max(-1.0, min(1.0, amp + balance / 2.0))This keeps
avg == ampandright_amp - left_amp == balance, staying aligned with the parsing logic and avoiding excessive amplification on either side.
616-617: EQ text parsing should be validated to avoidValueErrorand unclear failures
float(input_box.text() or 0)will raiseValueErrorif a user types non‑numeric text (e.g., “abc”), which is then only caught by the outer blanketexceptand logged as a generic “Failed to send settings”.It would be better to validate each field individually and fall back to a safe default (or highlight invalid inputs) instead of letting a single bad entry invalidate the entire update:
- left_eq = [float(input_box.text() or 0) for input_box in self.left_eq_inputs] - right_eq = [float(input_box.text() or 0) for input_box in self.right_eq_inputs] + def _safe_float(text: str) -> float: + try: + return float(text) if text else 0.0 + except ValueError: + # Optionally: surface this to the user instead of silently coercing + return 0.0 + + left_eq = [_safe_float(input_box.text()) for input_box in self.left_eq_inputs] + right_eq = [_safe_float(input_box.text()) for input_box in self.right_eq_inputs]You can later extend
_safe_floatto flag invalid entries in the UI (e.g., red border or a status message) if needed.
🧹 Nitpick comments (1)
windows/hearing-aid.py (1)
314-324: ATT response handling and error surfacing could be more precise
_read_responsetreats any item pulled fromself.responsesas a valid response and slices off the first byte without checking opcode or error conditions; it also collapses all failures into a genericException("No response received").This makes it hard to distinguish between:
- A true timeout (no PDU arrived),
- An ATT error response (e.g., error opcode),
- Other unexpected issues in the executor/queue path.
Consider:
- Checking the opcode of the response and explicitly handling error responses vs normal READ/WRITE responses.
- Raising a more specific exception type (e.g., a small
AttTimeoutError/AttErrorResponse), and preserving the original exception viaraise ... from errwhen wrapping.- Using
asyncio.get_running_loop()instead ofget_event_loop()and/or switching to anasyncio.Queuefor responses to avoid the threadpool hop entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
windows/hearing-aid.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.6)
windows/hearing-aid.py
1-1: Shebang is present but file is not executable
(EXE001)
28-34: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
81-81: Do not catch blind exception: Exception
(BLE001)
86-86: Do not catch blind exception: Exception
(BLE001)
177-177: Do not catch blind exception: Exception
(BLE001)
225-225: Do not catch blind exception: Exception
(BLE001)
268-268: Do not catch blind exception: Exception
(BLE001)
300-300: Do not catch blind exception: Exception
(BLE001)
311-311: Do not catch blind exception: Exception
(BLE001)
323-323: Do not catch blind exception: Exception
(BLE001)
324-324: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
324-324: Create your own exception
(TRY002)
324-324: Avoid specifying long messages outside the exception class
(TRY003)
346-346: Do not catch blind exception: Exception
(BLE001)
559-559: Do not catch blind exception: Exception
(BLE001)
560-560: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
624-624: Do not catch blind exception: Exception
(BLE001)
625-625: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
682-682: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
692-692: Unused lambda argument: conn
(ARG005)
707-707: Do not catch blind exception: Exception
(BLE001)
709-709: Store a reference to the return value of asyncio.create_task
(RUF006)
811-811: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
813-813: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
814-814: Do not catch blind exception: Exception
(BLE001)
815-815: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
840-840: Unused function argument: signum
(ARG001)
840-840: Unused function argument: frame
(ARG001)
merging windows and linux scripts into one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/hearing-aid.py (2)
692-693: Balance calculation produces asymmetric and extreme amplification values.This was flagged in a previous review. The formula treats
balanceas centered at 0.5, but it's actually in [-1, 1]. Atbalance = -1, left amplification becomes4 * amp, which is likely unintended.- left_amp = amp + (0.5 - balance) * amp * 2 if balance < 0 else amp - right_amp = amp + (balance - 0.5) * amp * 2 if balance > 0 else amp + # Balance: negative = favor left, positive = favor right + left_amp = amp * (1 - max(0, balance)) + right_amp = amp * (1 + min(0, balance))
695-696: Missing input validation for EQ values.This was flagged in a previous review.
float(input_box.text() or 0)raisesValueErrorfor non-numeric input like "abc".+ def safe_float(text: str) -> float: + try: + return float(text) if text else 0.0 + except ValueError: + return 0.0 - left_eq = [float(input_box.text() or 0) for input_box in self.left_eq_inputs] - right_eq = [float(input_box.text() or 0) for input_box in self.right_eq_inputs] + left_eq = [safe_float(input_box.text()) for input_box in self.left_eq_inputs] + right_eq = [safe_float(input_box.text()) for input_box in self.right_eq_inputs]
🧹 Nitpick comments (6)
scripts/hearing-aid.py (6)
107-112: Consider logging exceptions in cleanup code.The bare
exceptclause silently swallows all exceptions during socket cleanup. While this is common for cleanup code, logging at debug level would aid troubleshooting.def stop(self): self._running = False try: self.socket.close() - except: - pass + except Exception as e: + logger.debug(f"Socket close error: {e}")
136-153: Consider using@dataclassfor cleaner code.The
HearingAidSettingsclass is a data container that could benefit from using@dataclassdecorator to reduce boilerplate and gain automatic__repr__,__eq__, etc.+from dataclasses import dataclass + +@dataclass class HearingAidSettings: - def __init__(self, left_eq: List[float], right_eq: List[float], left_amp: float, right_amp: float, - left_tone: float, right_tone: float, left_conv: bool, right_conv: bool, - left_anr: float, right_anr: float, net_amp: float, balance: float, own_voice: float) -> None: - self.left_eq = left_eq - self.right_eq = right_eq - self.left_amplification = left_amp - self.right_amplification = right_amp - self.left_tone = left_tone - self.right_tone = right_tone - self.left_conversation_boost = left_conv - self.right_conversation_boost = right_conv - self.left_ambient_noise_reduction = left_anr - self.right_ambient_noise_reduction = right_anr - self.net_amplification = net_amp - self.balance = balance - self.own_voice_amplification = own_voice + left_eq: List[float] + right_eq: List[float] + left_amplification: float + right_amplification: float + left_tone: float + right_tone: float + left_conversation_boost: bool + right_conversation_boost: bool + left_ambient_noise_reduction: float + right_ambient_noise_reduction: float + net_amplification: float + balance: float + own_voice_amplification: float
354-364: Consider defining a custom exception for ATT timeouts.Raising a generic
Exceptionmakes it harder for callers to distinguish timeout errors from other failures. A custom exception class would improve error handling.+class ATTTimeoutError(Exception): + """Raised when an ATT response is not received within the timeout.""" + pass async def _read_response(self, timeout: float = 2.0) -> bytes: try: response = await asyncio.wait_for( asyncio.get_event_loop().run_in_executor( None, lambda: self.responses.get(timeout=timeout) ), timeout=timeout + 0.5 ) return response[1:] # Skip opcode except Exception: - raise Exception("No response received") from None + raise ATTTimeoutError("No ATT response received") from None
638-639: Uselogging.exceptionto include traceback in error logs.When logging caught exceptions,
logging.exceptionautomatically includes the traceback, which aids debugging.except Exception as e: - logger.error(f"Initial ATT setup failed: {e}") + logger.exception(f"Initial ATT setup failed: {e}")
865-867: Consider making USB transport index configurable.The transport
"usb:0"is hardcoded, which may not work if the user has multiple USB Bluetooth adapters or the desired adapter is at a different index.- transport = await open_transport("usb:0") + transport_name = os.environ.get("BUMBLE_TRANSPORT", "usb:0") + transport = await open_transport(transport_name)Then add
import osat the top of the file.
1058-1064: Process check is platform-specific and fragile.The
ps -Acheck for "librepods" only works on Linux/macOS and uses string matching which could produce false positives (e.g., a file named "librepods.txt" in current directory).- ps_output = subprocess.run(["ps", "-A"], capture_output=True, text=True).stdout - if "librepods" in ps_output: + try: + ps_output = subprocess.run( + ["pgrep", "-x", "librepods"], capture_output=True, text=True + ) + if ps_output.returncode == 0: + logger.error("LibrePods is running. Please close it before using this script.") + loop.call_soon_threadsafe(loop.stop) + quit_app(None, None) + return + except FileNotFoundError: + pass # pgrep not available, skip check - logger.error("LibrePods is running. Please close it before using this script.") - loop.call_soon_threadsafe(loop.stop) - quit_app(None, None) - return
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/hearing-aid.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/hearing-aid.py (1)
linux/airpods_packets.h (1)
Connection(149-155)
🪛 Ruff (0.14.7)
scripts/hearing-aid.py
1-1: Shebang is present but file is not executable
(EXE001)
30-36: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
93-93: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
111-111: Do not use bare except
(E722)
111-112: try-except-pass detected, consider logging the exception
(S110)
121-121: Do not catch blind exception: Exception
(BLE001)
126-126: Do not catch blind exception: Exception
(BLE001)
217-217: Do not catch blind exception: Exception
(BLE001)
265-265: Do not catch blind exception: Exception
(BLE001)
308-308: Do not catch blind exception: Exception
(BLE001)
340-340: Do not catch blind exception: Exception
(BLE001)
351-351: Do not catch blind exception: Exception
(BLE001)
363-363: Do not catch blind exception: Exception
(BLE001)
364-364: Create your own exception
(TRY002)
364-364: Avoid specifying long messages outside the exception class
(TRY003)
386-386: Do not catch blind exception: Exception
(BLE001)
638-638: Do not catch blind exception: Exception
(BLE001)
639-639: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
703-703: Do not catch blind exception: Exception
(BLE001)
704-704: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
759-759: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
807-807: Do not catch blind exception: Exception
(BLE001)
808-808: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
818-819: try-except-pass detected, consider logging the exception
(S110)
818-818: Do not catch blind exception: Exception
(BLE001)
826-827: try-except-pass detected, consider logging the exception
(S110)
826-826: Do not catch blind exception: Exception
(BLE001)
831-832: try-except-pass detected, consider logging the exception
(S110)
831-831: Do not catch blind exception: Exception
(BLE001)
836-837: try-except-pass detected, consider logging the exception
(S110)
836-836: Do not catch blind exception: Exception
(BLE001)
855-855: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
872-872: Unused lambda argument: conn
(ARG005)
887-887: Do not catch blind exception: Exception
(BLE001)
889-889: Store a reference to the return value of asyncio.create_task
(RUF006)
979-979: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
981-981: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
982-982: Do not catch blind exception: Exception
(BLE001)
983-983: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
993-994: try-except-pass detected, consider logging the exception
(S110)
993-993: Do not catch blind exception: Exception
(BLE001)
1001-1002: try-except-pass detected, consider logging the exception
(S110)
1001-1001: Do not catch blind exception: Exception
(BLE001)
1008-1009: try-except-pass detected, consider logging the exception
(S110)
1008-1008: Do not catch blind exception: Exception
(BLE001)
1015-1016: try-except-pass detected, consider logging the exception
(S110)
1015-1015: Do not catch blind exception: Exception
(BLE001)
1038-1038: Unused function argument: signum
(ARG001)
1038-1038: Unused function argument: frame
(ARG001)
1044-1045: try-except-pass detected, consider logging the exception
(S110)
1044-1044: Do not catch blind exception: Exception
(BLE001)
1059-1059: Starting a process with a partial executable path
(S607)
1068-1068: Do not catch blind exception: Exception
(BLE001)
1069-1069: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (6)
scripts/hearing-aid.py (6)
62-77: LGTM!Protocol constants and opcodes are well-defined and align with the existing C++ implementation in
linux/airpods_packets.h.
155-203: LGTM!The binary parsing logic correctly handles the hearing aid settings format with proper offset tracking and length validation.
204-292: LGTM!The
AACPManagerclass provides clean abstractions for AACP protocol operations with proper packet handling and listener dispatch.
397-404: LGTM!Clean signal definitions following standard PyQt5 patterns for thread-safe communication between async/Bluetooth threads and the UI thread.
754-840: LGTM!The BlueZ connection setup follows a sound pattern with proper channel initialization, protocol handshake sequence, and thorough cleanup in the finally block.
1021-1091: LGTM!The main function properly orchestrates the Qt application, async event loop, and signal handling with appropriate cleanup on shutdown.
| def on_l2cap_connection(channel: ClassicChannel): | ||
| logger.info("Incoming L2CAP connection on PSM %d", channel.psm) | ||
| async def handle_data(): | ||
| try: | ||
| reader = _make_reader(channel) | ||
| while True: | ||
| data = await reader() | ||
| print(f"Received PDU on PSM {channel.psm}: {data.hex() if data else 'None'}") | ||
| except Exception as e: | ||
| logger.info("L2CAP channel on PSM %d closed: %s", channel.psm, e) | ||
| asyncio.create_task(handle_data()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fire-and-forget task may silently fail or be garbage collected.
The task created at line 889 is not stored, which can cause:
- Silent exception swallowing if the task fails
- Potential garbage collection before completion
Store the task reference and cancel it during cleanup.
+ pending_tasks = []
+
def on_l2cap_connection(channel: ClassicChannel):
logger.info("Incoming L2CAP connection on PSM %d", channel.psm)
async def handle_data():
try:
reader = _make_reader(channel)
while True:
data = await reader()
print(f"Received PDU on PSM {channel.psm}: {data.hex() if data else 'None'}")
except Exception as e:
logger.info("L2CAP channel on PSM %d closed: %s", channel.psm, e)
- asyncio.create_task(handle_data())
+ task = asyncio.create_task(handle_data())
+ pending_tasks.append(task)Then cancel pending_tasks in the finally block.
🧰 Tools
🪛 Ruff (0.14.7)
887-887: Do not catch blind exception: Exception
(BLE001)
889-889: Store a reference to the return value of asyncio.create_task
(RUF006)
🤖 Prompt for AI Agents
In scripts/hearing-aid.py around lines 879 to 889, the created fire-and-forget
task isn't stored which can let exceptions be lost or the task be
garbage-collected; fix this by creating a container (e.g. pending_tasks list) in
the surrounding scope, store the returned asyncio.Task when calling
asyncio.create_task(handle_data()), and ensure that in the surrounding
cleanup/finally block you iterate over pending_tasks to cancel each task and
optionally await their cancellation (suppressing CancelledError) so tasks are
properly cleaned up and exceptions surfaced.
Quoting bumble's docs:
Run with
python windows/hearing-aid.py [AirPods' MAC]. You will have to re-pair your AirPods with windows later.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.