Refactor connect-omi.py for improved device selection and user intera…#294
Refactor connect-omi.py for improved device selection and user intera…#294AnkushMalaker wants to merge 3 commits intodevfrom
Conversation
…ction - Replaced references to the chronicle Bluetooth library with friend_lite for device management. - Removed the list_devices function and implemented a new prompt_user_to_pick_device function to enhance user interaction when selecting OMI/Neo devices. - Updated the find_and_set_omi_mac function to utilize the new device selection method, improving the overall flow of device connection. - Added a new scan_devices.py script for quick scanning of neo/neosapien devices, enhancing usability. - Updated README.md to reflect new usage instructions and prerequisites for connecting to OMI devices over Bluetooth. - Enhanced start.sh to ensure proper environment variable setup for macOS users.
📝 WalkthroughWalkthroughThis pull request introduces a button event tracking feature to the backend with markers storage and persistence, adds a comprehensive friend-lite-sdk library for BLE connectivity and transcription, and refactors local OMI BT tooling to use the new SDK. Changes
Sequence Diagram(s)sequenceDiagram
participant Device as OMI Device
participant WS as WebSocket<br/>Controller
participant ClientState as Client<br/>State
participant Redis as Redis
participant Router as Plugin<br/>Router
participant Plugin as Plugin
participant Conversation as Conversation<br/>Jobs
Device->>WS: button-event message
activate WS
WS->>WS: _handle_button_event()
WS->>WS: create marker dict<br/>(type, state, timestamp, etc.)
WS->>ClientState: add_marker(marker)
activate ClientState
ClientState->>ClientState: append to markers list
deactivate ClientState
WS->>Router: dispatch button.event
activate Router
Router->>Plugin: on_button_event(context)
activate Plugin
Plugin-->>Router: PluginResult
deactivate Plugin
deactivate Router
WS->>Redis: persist markers as JSON<br/>(session key)
WS->>ClientState: clear markers list
deactivate WS
Conversation->>Redis: fetch markers by session key
activate Conversation
Conversation->>Conversation: parse JSON markers
Conversation->>Conversation: attach to conversation.markers
Conversation->>Conversation: save conversation
deactivate Conversation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
…end Lite BLE devices - Introduced the friend-lite-sdk, a Python SDK for OMI/Friend Lite BLE devices, enabling audio streaming, button events, and transcription functionalities. - Added LICENSE and NOTICE files to clarify licensing and attribution. - Created pyproject.toml for package management, specifying dependencies and project metadata. - Developed core modules including bluetooth connection handling, button event parsing, audio decoding, and transcription capabilities. - Implemented example usage in README.md to guide users on installation and basic functionality. - Enhanced connect-omi.py to utilize the new SDK for improved device management and event handling. - Updated requirements.txt to reference the new SDK for local development. This commit lays the foundation for further enhancements and integrations with OMI devices.
- Introduced a new `markers` list in `ClientState` to collect button event data during sessions. - Added `add_marker` method to facilitate the addition of markers to the current session. - Implemented `on_button_event` method in the `BasePlugin` class to handle device button events, providing context data for button state and timestamps. - Updated `PluginRouter` to route button events to the appropriate plugin handler. - Enhanced conversation job handling to attach markers from Redis sessions, improving the tracking of button events during conversations.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@backends/advanced/src/advanced_omi_backend/client.py`:
- Around line 54-55: The timeout path can race with websocket_controller
finalization because start_new_conversation() is scheduled via
asyncio.create_task() and close_current_conversation() is legacy and doesn't
clear markers; update start_new_conversation() to explicitly clear markers
before resetting conversation state (call self.markers.clear() or
client_state.markers.clear() as appropriate) so any leftover markers from the
previous conversation are removed even if websocket_controller finalization
hasn't run, leaving the rest of the reset (current_audio_uuid, conversation IDs,
etc.) unchanged.
In `@backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py`:
- Around line 307-316: The block reading markers uses session_key before it's
defined, causing a NameError; move or compute session_key =
f"audio:session:{session_id}" so it is defined before the
redis_client.hget(session_key, "markers") call (session_id is the identifier to
use), then assign conversation.markers from json.loads(markers_data) and save as
before; also narrow the except to catch json.JSONDecodeError and
UnicodeDecodeError (instead of broad Exception) around the json.loads/decoding
step to avoid swallowing unrelated errors.
In `@extras/friend-lite-sdk/friend_lite/bluetooth.py`:
- Around line 28-39: The connect method assigns self._client before awaiting
self._client.connect(), so if BleakClient.connect() raises the instance is left
with a non-None _client and future connect() calls are blocked; update
OmiConnection.connect to catch exceptions from await self._client.connect(), and
on any failure reset self._client to None (and optionally clear/set any related
state like self._disconnected) before re-raising the exception so the instance
can retry connecting later; look for the connect function, the _on_disconnect
callback, and usages of self._client / BleakClient to implement this change.
In `@extras/friend-lite-sdk/friend_lite/discover_characteristics.py`:
- Line 19: The module currently calls asyncio.run(main()) at import time which
triggers BLE connection; wrap that call in a standard module guard so that
main() only runs when executed as a script. Locate the top-level
asyncio.run(main()) invocation (and the async def main function) and move or
replace the bare call with a guarded call using if __name__ == "__main__": so
importing friend_lite.discover_characteristics no longer executes the BLE logic.
- Line 4: The file currently hardcodes a device MAC in the constant OMI_MAC;
remove that literal and change the script in discover_characteristics.py to
accept the device address via a CLI argument (e.g., argparse) or an environment
variable (e.g., FRIEND_DEVICE_MAC) with the CLI taking precedence, validate the
address format before use, and update any calls/usage/help text to reflect the
new parameter so no real device identifiers remain in source.
In `@extras/friend-lite-sdk/friend_lite/transcribe.py`:
- Around line 153-157: The logger.debug call uses an unnecessary f-string prefix
in the block that sends AudioStop when segment_has_audio is true; remove the
extra "f" so the call is logger.debug("Wyoming: Sending AudioStop...") in the
same conditional where AudioStop().event() is written to the client (see
references: segment_has_audio, logger.debug, client.write_event,
AudioStop().event()).
- Around line 96-100: The connection creation and connect call are outside the
try so connection failures bypass error handling and cleanup; move the
AsyncClient.from_uri(wyoming_url) and await client.connect() into the existing
try block that contains the subsequent logic (the try that currently starts
before the main processing and has the except handlers and finally cleanup),
ensuring any exceptions from AsyncClient.from_uri or client.connect are caught
by the existing except clauses and that the finally block still runs to
close/cleanup the client and call logger as appropriate.
In `@extras/local-omi-bt/connect-omi.py`:
- Around line 212-219: The async with OmiConnection(...) block never exits
because asyncio.gather(conn.wait_until_disconnected(), process_audio(),
backend_stream_wrapper()) never completes, preventing OmiConnection.__aexit__ /
disconnect() from running; fix by creating separate tasks for process_audio()
and backend_stream_wrapper() (e.g., via asyncio.create_task), await
conn.wait_until_disconnected(), then cancel and await those background tasks
before leaving the async with so that __aexit__/disconnect() runs and resources
are cleaned up; reference OmiConnection, wait_until_disconnected, process_audio,
backend_stream_wrapper, handle_ble_data, handle_button_event, __aexit__, and
disconnect when making the changes.
- Around line 74-78: The fire-and-forget call to
loop.create_task(send_button_event(state.name)) loses exceptions; change the
block to capture the created Task (e.g., task = loop.create_task(...)) and
attach a done callback that logs exceptions from task.exception() using the
existing logger so errors are not silently dropped; keep the RuntimeError except
handling for no running loop and ensure the callback checks for cancelled/None
exceptions before logging.
- Around line 215-219: Gathering wait_until_disconnected(), process_audio(), and
backend_stream_wrapper() with asyncio.gather hangs because the latter two are
infinite queue.get() loops; change to explicit task management: create tasks for
process_audio() and backend_stream_wrapper() via asyncio.create_task, await
conn.wait_until_disconnected(), then cancel the remaining tasks (task.cancel()),
and await them with return_exceptions=True while handling asyncio.CancelledError
inside those coroutines (or catching it when awaiting) to ensure clean shutdown;
update the block referencing wait_until_disconnected, process_audio, and
backend_stream_wrapper to use try/finally to perform cancellations and cleanup.
In `@extras/local-omi-bt/send_to_adv.py`:
- Around line 39-55: The send_button_event function can raise an unhandled
ConnectionClosed if _active_websocket closes after the None check; wrap the
await _active_websocket.send(...) call in a try/except that catches the
websocket connection closed error (e.g., websockets.exceptions.ConnectionClosed
or the relevant library exception) inside send_button_event, log the failure
with logger.debug/logger.info including the button_state and the exception, and
return gracefully so the exception doesn't propagate to callers.
🧹 Nitpick comments (11)
extras/friend-lite-sdk/friend_lite/discover_characteristics.py (1)
6-18: Consider adding error handling for connection failures.
BleakClientcan raiseBleakError,TimeoutError, orOSErrorif the device is unreachable. A try/except with a user-friendly message would make this diagnostic tool more robust.extras/local-omi-bt/requirements.txt (1)
5-5: Local file reference may break in CI/Docker environments.The
file:../friend-lite-sdkreference assumes a specific relative directory layout. This works for local development but will fail in environments where the sibling directory isn't present (CI pipelines, Docker builds, or consumers who install from a different working directory). Consider publishing to a private index or using a git-based dependency as a fallback, or at minimum document this constraint clearly.extras/friend-lite-sdk/pyproject.toml (1)
14-16: Homepage URL points to the author's fork, not the organization repo.The
HomepageURL referencesAnkushMalaker/chroniclerather than the target organization repository (SimpleOpenSoftware/chronicle). Since this PR targets theSimpleOpenSoftwareorg, consider updating the URL to reflect the canonical upstream.-Homepage = "https://github.com/AnkushMalaker/chronicle" +Homepage = "https://github.com/SimpleOpenSoftware/chronicle"extras/friend-lite-sdk/friend_lite/decoder.py (1)
19-24: Uselogginginstead ofTwo issues flagged by static analysis (and good practice for library code):
print()in library code — Consumers cannot suppress or redirect this output. Uselogging.getLogger(__name__).warning(...)instead.- Catching bare
Exception— This swallows programming errors (e.g.,TypeErrorfrom bad arguments). Catch the specific Opus decode exception instead.♻️ Proposed fix
+import logging from opuslib import Decoder + +logger = logging.getLogger(__name__) class OmiOpusDecoder: def __init__(self): self.decoder = Decoder(16000, 1) # 16kHz mono def decode_packet(self, data: bytes, strip_header: bool = True): if len(data) <= 3: return b'' # Remove 3-byte header if strip_header: clean_data = bytes(data[3:]) else: clean_data = data # Decode Opus to PCM 16-bit try: pcm = self.decoder.decode(clean_data, 960, decode_fec=False) - return pcm - except Exception as e: - print("Opus decode error:", e) + except Exception: + logger.warning("Opus decode error", exc_info=True) return b'' + return pcmextras/friend-lite-sdk/friend_lite/transcribe.py (1)
38-83:transcribe()usesprint()throughout while the module defines alogger.This function uses
print()for all output whereastranscribe_wyomingconsistently uses the module-levellogger. For consistency and production use (log levels, filtering, structured output), switch tologger.extras/friend-lite-sdk/friend_lite/bluetooth.py (1)
9-12:print_devices()usesasyncio.run(), which will crash if called from within an existing event loop.This is a synchronous convenience wrapper, but if a consumer mistakenly calls it from async context (e.g., a Jupyter notebook or an already-running loop), it will raise
RuntimeError. Consider making thisasyncor documenting the limitation.extras/friend-lite-sdk/README.md (1)
22-30: Usage example importsButtonStateandparse_button_eventbut never uses them.Consider either removing the unused imports from the example or adding a brief button-event snippet to demonstrate their use. Unused imports in documentation examples can confuse readers.
extras/friend-lite-sdk/friend_lite/button.py (1)
16-24:ButtonState(state)will raiseValueErrorfor unrecognized state values from firmware.If the device sends a state value not in the enum (e.g., a new firmware adds state 6),
ButtonState(state)throws. This is acceptable fail-fast behavior, but worth noting since BLE data can be unpredictable. Consider whether callers should be expected to catchValueErroror if the SDK should handle it (e.g., return a raw int or a sentinel).extras/local-omi-bt/connect-omi.py (3)
43-46:as_audio_chunksappears unused.This function is defined but never called in the file — audio chunks are constructed inline at Line 188 instead. Consider removing it if it's no longer needed.
148-166: Probe-connect creates two connection cycles & minor lint cleanup.The
test_clientvariable triggers an F841 lint warning. Since you only need the side-effect of connecting, prefix it with_to signal intent. Also,f-string formatting inloggercalls eagerly evaluates even when the log level is disabled — prefer%-style.Suggested cleanup
- async with BleakClient(mac_address) as test_client: - logger.info(f"Successfully connected to device {mac_address}") + async with BleakClient(mac_address) as _client: + logger.info("Successfully connected to device %s", mac_address) except Exception as e: - logger.error(f"Failed to connect to device {mac_address}: {e}") + logger.error("Failed to connect to device %s: %s", mac_address, e)Apply the same pattern to the retry block at Lines 161-166.
49-54: Consider removing the commented-out Deepgram code.This dead code adds noise. If it's needed later, it's recoverable from version control.
| # Markers (e.g., button events) collected during the session | ||
| self.markers: List[dict] = [] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C5 'markers' --type=py -g '*websocket*'Repository: SimpleOpenSoftware/chronicle
Length of output: 4063
🏁 Script executed:
cat -n backends/advanced/src/advanced_omi_backend/client.py | sed -n '50,75p; 105,115p; 120,170p'Repository: SimpleOpenSoftware/chronicle
Length of output: 4243
🏁 Script executed:
rg -n 'markers' --type=py -B2 -A2 | head -100Repository: SimpleOpenSoftware/chronicle
Length of output: 7187
Markers clearing in websocket_controller confirmed, but timeout path introduces potential race condition.
The websocket_controller does clear client_state.markers (at lines 537 and 1158) after persisting them to Redis, confirming external coordination. However, the timeout path at line 72 calls start_new_conversation() via asyncio.create_task() without blocking, meaning the websocket_controller's finalization may not have run yet when a new conversation begins, allowing markers from the previous conversation to accumulate.
Since close_current_conversation() is documented as "legacy V1 code" (lines 139–140) and doesn't participate in the websocket_controller's finalization flow, markers must be cleared explicitly in start_new_conversation() before resetting conversation state:
async def start_new_conversation(self):
"""Start a new conversation by closing current and resetting state."""
await self.close_current_conversation()
self.markers.clear() # Clear before resetting conversation
# Reset conversation state
self.current_audio_uuid = None
...🤖 Prompt for AI Agents
In `@backends/advanced/src/advanced_omi_backend/client.py` around lines 54 - 55,
The timeout path can race with websocket_controller finalization because
start_new_conversation() is scheduled via asyncio.create_task() and
close_current_conversation() is legacy and doesn't clear markers; update
start_new_conversation() to explicitly clear markers before resetting
conversation state (call self.markers.clear() or client_state.markers.clear() as
appropriate) so any leftover markers from the previous conversation are removed
even if websocket_controller finalization hasn't run, leaving the rest of the
reset (current_audio_uuid, conversation IDs, etc.) unchanged.
| # Attach markers from Redis session (e.g., button events captured during streaming) | ||
| markers_json = await redis_client.hget(session_key, "markers") | ||
| if markers_json: | ||
| try: | ||
| markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode() | ||
| conversation.markers = json.loads(markers_data) | ||
| await conversation.save() | ||
| logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}") | ||
| except Exception as marker_err: | ||
| logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}") |
There was a problem hiding this comment.
session_key is undefined here — runtime NameError.
session_key is first defined on line 362 (session_key = f"audio:session:{session_id}"), but this block at line 308 references it before that definition. This will crash open_conversation_job with a NameError every time markers exist in Redis.
🐛 Fix: define session_key before use
# Attach markers from Redis session (e.g., button events captured during streaming)
+ session_key = f"audio:session:{session_id}"
markers_json = await redis_client.hget(session_key, "markers")
if markers_json:
try:
markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode()
conversation.markers = json.loads(markers_data)
await conversation.save()
logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}")
- except Exception as marker_err:
+ except (json.JSONDecodeError, UnicodeDecodeError) as marker_err:
logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}")Note: The except Exception is also overly broad for what should only be a JSON parsing concern — narrowing to json.JSONDecodeError and UnicodeDecodeError avoids silently swallowing unexpected failures.
📝 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.
| # Attach markers from Redis session (e.g., button events captured during streaming) | |
| markers_json = await redis_client.hget(session_key, "markers") | |
| if markers_json: | |
| try: | |
| markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode() | |
| conversation.markers = json.loads(markers_data) | |
| await conversation.save() | |
| logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}") | |
| except Exception as marker_err: | |
| logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}") | |
| # Attach markers from Redis session (e.g., button events captured during streaming) | |
| session_key = f"audio:session:{session_id}" | |
| markers_json = await redis_client.hget(session_key, "markers") | |
| if markers_json: | |
| try: | |
| markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode() | |
| conversation.markers = json.loads(markers_data) | |
| await conversation.save() | |
| logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}") | |
| except (json.JSONDecodeError, UnicodeDecodeError) as marker_err: | |
| logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}") |
🧰 Tools
🪛 Ruff (0.14.14)
[error] 308-308: Undefined name session_key
(F821)
[warning] 315-315: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py`
around lines 307 - 316, The block reading markers uses session_key before it's
defined, causing a NameError; move or compute session_key =
f"audio:session:{session_id}" so it is defined before the
redis_client.hget(session_key, "markers") call (session_id is the identifier to
use), then assign conversation.markers from json.loads(markers_data) and save as
before; also narrow the except to catch json.JSONDecodeError and
UnicodeDecodeError (instead of broad Exception) around the json.loads/decoding
step to avoid swallowing unrelated errors.
| async def connect(self) -> None: | ||
| if self._client is not None: | ||
| return | ||
|
|
||
| def _on_disconnect(_client: BleakClient) -> None: | ||
| self._disconnected.set() | ||
|
|
||
| self._client = BleakClient( | ||
| self._mac_address, | ||
| disconnected_callback=_on_disconnect, | ||
| ) | ||
| await self._client.connect() |
There was a problem hiding this comment.
If BleakClient.connect() raises, the OmiConnection becomes permanently broken.
self._client is assigned on line 35 but if await self._client.connect() (line 39) throws, the guard on line 29 (if self._client is not None: return) will short-circuit all future connect() attempts on this instance.
🐛 Proposed fix — reset client on connection failure
async def connect(self) -> None:
if self._client is not None:
return
def _on_disconnect(_client: BleakClient) -> None:
self._disconnected.set()
self._client = BleakClient(
self._mac_address,
disconnected_callback=_on_disconnect,
)
- await self._client.connect()
+ try:
+ await self._client.connect()
+ except Exception:
+ self._client = None
+ raise🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/bluetooth.py` around lines 28 - 39, The
connect method assigns self._client before awaiting self._client.connect(), so
if BleakClient.connect() raises the instance is left with a non-None _client and
future connect() calls are blocked; update OmiConnection.connect to catch
exceptions from await self._client.connect(), and on any failure reset
self._client to None (and optionally clear/set any related state like
self._disconnected) before re-raising the exception so the instance can retry
connecting later; look for the connect function, the _on_disconnect callback,
and usages of self._client / BleakClient to implement this change.
| import asyncio | ||
| from bleak import BleakClient | ||
|
|
||
| OMI_MAC = "7F52EC55-50C9-D1B9-E8D7-19B83217C97D" # Replace with your actual MAC |
There was a problem hiding this comment.
Hardcoded device MAC address should be removed or parameterized.
This looks like a real device identifier committed to the repository. For a utility script in an SDK package, accept the address as a CLI argument or environment variable instead of hardcoding it.
Proposed fix
-OMI_MAC = "7F52EC55-50C9-D1B9-E8D7-19B83217C97D" # Replace with your actual MAC
+import sys
+import os
+
+OMI_MAC = os.environ.get("OMI_MAC") or (sys.argv[1] if len(sys.argv) > 1 else None)
+if not OMI_MAC:
+ print("Usage: python discover_characteristics.py <MAC_ADDRESS>")
+ print(" or set OMI_MAC environment variable")
+ sys.exit(1)🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/discover_characteristics.py` at line 4,
The file currently hardcodes a device MAC in the constant OMI_MAC; remove that
literal and change the script in discover_characteristics.py to accept the
device address via a CLI argument (e.g., argparse) or an environment variable
(e.g., FRIEND_DEVICE_MAC) with the CLI taking precedence, validate the address
format before use, and update any calls/usage/help text to reflect the new
parameter so no real device identifiers remain in source.
| if "notify" in char.properties: | ||
| print(" ✅ Notifiable (can stream data)") | ||
|
|
||
| asyncio.run(main()) |
There was a problem hiding this comment.
Missing if __name__ == "__main__" guard — importing this module triggers BLE connection.
This file is inside the friend_lite package. The top-level asyncio.run(main()) will execute on any import of this module, blocking while it attempts a BLE connection. Wrap it in a main guard.
Proposed fix
-asyncio.run(main())
+if __name__ == "__main__":
+ asyncio.run(main())📝 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.
| asyncio.run(main()) | |
| if __name__ == "__main__": | |
| asyncio.run(main()) |
🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/discover_characteristics.py` at line 19,
The module currently calls asyncio.run(main()) at import time which triggers BLE
connection; wrap that call in a standard module guard so that main() only runs
when executed as a script. Locate the top-level asyncio.run(main()) invocation
(and the async def main function) and move or replace the bare call with a
guarded call using if __name__ == "__main__": so importing
friend_lite.discover_characteristics no longer executes the BLE logic.
| # 5. Stop Audio Segment (only if audio was sent in this window) | ||
| if segment_has_audio: | ||
| logger.debug(f"Wyoming: Sending AudioStop...") | ||
| await client.write_event(AudioStop().event()) | ||
| logger.debug("Wyoming: AudioStop sent.") |
There was a problem hiding this comment.
Extraneous f prefix on line 155.
f"Wyoming: Sending AudioStop..." has no placeholders. Remove the f prefix.
Proposed fix
- logger.debug(f"Wyoming: Sending AudioStop...")
+ logger.debug("Wyoming: Sending AudioStop...")📝 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.
| # 5. Stop Audio Segment (only if audio was sent in this window) | |
| if segment_has_audio: | |
| logger.debug(f"Wyoming: Sending AudioStop...") | |
| await client.write_event(AudioStop().event()) | |
| logger.debug("Wyoming: AudioStop sent.") | |
| # 5. Stop Audio Segment (only if audio was sent in this window) | |
| if segment_has_audio: | |
| logger.debug("Wyoming: Sending AudioStop...") | |
| await client.write_event(AudioStop().event()) | |
| logger.debug("Wyoming: AudioStop sent.") |
🧰 Tools
🪛 Ruff (0.14.14)
[error] 155-155: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/transcribe.py` around lines 153 - 157, The
logger.debug call uses an unnecessary f-string prefix in the block that sends
AudioStop when segment_has_audio is true; remove the extra "f" so the call is
logger.debug("Wyoming: Sending AudioStop...") in the same conditional where
AudioStop().event() is written to the client (see references: segment_has_audio,
logger.debug, client.write_event, AudioStop().event()).
| try: | ||
| loop = asyncio.get_running_loop() | ||
| loop.create_task(send_button_event(state.name)) | ||
| except RuntimeError: | ||
| logger.debug("No running event loop, cannot send button event") |
There was a problem hiding this comment.
Fire-and-forget task loses exceptions silently.
loop.create_task() without storing the returned task means any exception raised inside send_button_event will be silently dropped (Python logs "Task exception was never retrieved" at GC time, which is unreliable). Store the task and attach an error-logging callback.
Proposed fix
try:
loop = asyncio.get_running_loop()
- loop.create_task(send_button_event(state.name))
+ task = loop.create_task(send_button_event(state.name))
+ task.add_done_callback(
+ lambda t: t.exception() and logger.error("Button event send failed: %s", t.exception())
+ )
except RuntimeError:
logger.debug("No running event loop, cannot send button event")🧰 Tools
🪛 Ruff (0.14.14)
[warning] 76-76: Store a reference to the return value of loop.create_task
(RUF006)
🤖 Prompt for AI Agents
In `@extras/local-omi-bt/connect-omi.py` around lines 74 - 78, The fire-and-forget
call to loop.create_task(send_button_event(state.name)) loses exceptions; change
the block to capture the created Task (e.g., task = loop.create_task(...)) and
attach a done callback that logs exceptions from task.exception() using the
existing logger so errors are not silently dropped; keep the RuntimeError except
handling for no running loop and ensure the callback checks for cancelled/None
exceptions before logging.
| async with OmiConnection(mac_address) as conn: | ||
| await conn.subscribe_audio(handle_ble_data) | ||
| await conn.subscribe_button(handle_button_event) | ||
| await asyncio.gather( | ||
| conn.wait_until_disconnected(), | ||
| process_audio(), | ||
| backend_stream_wrapper(), | ||
| ) |
There was a problem hiding this comment.
OmiConnection context manager exit is also blocked by gather.
Worth noting: since gather never completes (as described above), the async with OmiConnection block body never exits, meaning __aexit__ / disconnect() is never called. The fix proposed above (cancelling tasks after wait_until_disconnected) also resolves this — __aexit__ would then properly clean up the BLE connection.
🤖 Prompt for AI Agents
In `@extras/local-omi-bt/connect-omi.py` around lines 212 - 219, The async with
OmiConnection(...) block never exits because
asyncio.gather(conn.wait_until_disconnected(), process_audio(),
backend_stream_wrapper()) never completes, preventing OmiConnection.__aexit__ /
disconnect() from running; fix by creating separate tasks for process_audio()
and backend_stream_wrapper() (e.g., via asyncio.create_task), await
conn.wait_until_disconnected(), then cancel and await those background tasks
before leaving the async with so that __aexit__/disconnect() runs and resources
are cleaned up; reference OmiConnection, wait_until_disconnected, process_audio,
backend_stream_wrapper, handle_ble_data, handle_button_event, __aexit__, and
disconnect when making the changes.
| await asyncio.gather( | ||
| conn.wait_until_disconnected(), | ||
| process_audio(), | ||
| backend_stream_wrapper(), | ||
| ) |
There was a problem hiding this comment.
asyncio.gather will hang after device disconnects.
wait_until_disconnected() completes when the BLE link drops, but process_audio() and backend_stream_wrapper() are infinite loops blocked on their respective queue.get() calls. Since gather waits for all tasks, the program will never exit cleanly after disconnection.
Consider cancelling the remaining tasks once disconnection is detected, e.g.:
Proposed fix using explicit task management
async with OmiConnection(mac_address) as conn:
await conn.subscribe_audio(handle_ble_data)
await conn.subscribe_button(handle_button_event)
- await asyncio.gather(
- conn.wait_until_disconnected(),
- process_audio(),
- backend_stream_wrapper(),
- )
+ audio_task = asyncio.create_task(process_audio())
+ backend_task = asyncio.create_task(backend_stream_wrapper())
+ await conn.wait_until_disconnected()
+ logger.info("Device disconnected, shutting down…")
+ audio_task.cancel()
+ backend_task.cancel()
+ for t in (audio_task, backend_task):
+ try:
+ await t
+ except asyncio.CancelledError:
+ pass🤖 Prompt for AI Agents
In `@extras/local-omi-bt/connect-omi.py` around lines 215 - 219, Gathering
wait_until_disconnected(), process_audio(), and backend_stream_wrapper() with
asyncio.gather hangs because the latter two are infinite queue.get() loops;
change to explicit task management: create tasks for process_audio() and
backend_stream_wrapper() via asyncio.create_task, await
conn.wait_until_disconnected(), then cancel the remaining tasks (task.cancel()),
and await them with return_exceptions=True while handling asyncio.CancelledError
inside those coroutines (or catching it when awaiting) to ensure clean shutdown;
update the block referencing wait_until_disconnected, process_audio, and
backend_stream_wrapper to use try/finally to perform cancellations and cleanup.
| async def send_button_event(button_state: str) -> None: | ||
| """Send a button event to the backend via the active WebSocket connection. | ||
|
|
||
| Args: | ||
| button_state: Button state string (e.g., "SINGLE_TAP", "DOUBLE_TAP") | ||
| """ | ||
| if _active_websocket is None: | ||
| logger.debug("No active websocket, dropping button event: %s", button_state) | ||
| return | ||
|
|
||
| event = { | ||
| "type": "button-event", | ||
| "data": {"state": button_state}, | ||
| "payload_length": None, | ||
| } | ||
| await _active_websocket.send(json.dumps(event) + "\n") | ||
| logger.info("Sent button event to backend: %s", button_state) |
There was a problem hiding this comment.
send_button_event can raise unhandled ConnectionClosed if the websocket closes between the guard and the send.
Between the _active_websocket is None check on line 45 and the .send() on line 54, the connection may have been closed (e.g., server-side disconnect). This would propagate an unhandled exception to the caller.
🛡️ Proposed fix
async def send_button_event(button_state: str) -> None:
...
if _active_websocket is None:
logger.debug("No active websocket, dropping button event: %s", button_state)
return
event = {
"type": "button-event",
"data": {"state": button_state},
"payload_length": None,
}
- await _active_websocket.send(json.dumps(event) + "\n")
- logger.info("Sent button event to backend: %s", button_state)
+ try:
+ await _active_websocket.send(json.dumps(event) + "\n")
+ logger.info("Sent button event to backend: %s", button_state)
+ except Exception:
+ logger.warning("Failed to send button event (connection may have closed): %s", button_state)📝 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.
| async def send_button_event(button_state: str) -> None: | |
| """Send a button event to the backend via the active WebSocket connection. | |
| Args: | |
| button_state: Button state string (e.g., "SINGLE_TAP", "DOUBLE_TAP") | |
| """ | |
| if _active_websocket is None: | |
| logger.debug("No active websocket, dropping button event: %s", button_state) | |
| return | |
| event = { | |
| "type": "button-event", | |
| "data": {"state": button_state}, | |
| "payload_length": None, | |
| } | |
| await _active_websocket.send(json.dumps(event) + "\n") | |
| logger.info("Sent button event to backend: %s", button_state) | |
| async def send_button_event(button_state: str) -> None: | |
| """Send a button event to the backend via the active WebSocket connection. | |
| Args: | |
| button_state: Button state string (e.g., "SINGLE_TAP", "DOUBLE_TAP") | |
| """ | |
| if _active_websocket is None: | |
| logger.debug("No active websocket, dropping button event: %s", button_state) | |
| return | |
| event = { | |
| "type": "button-event", | |
| "data": {"state": button_state}, | |
| "payload_length": None, | |
| } | |
| try: | |
| await _active_websocket.send(json.dumps(event) + "\n") | |
| logger.info("Sent button event to backend: %s", button_state) | |
| except Exception: | |
| logger.warning("Failed to send button event (connection may have closed): %s", button_state) |
🤖 Prompt for AI Agents
In `@extras/local-omi-bt/send_to_adv.py` around lines 39 - 55, The
send_button_event function can raise an unhandled ConnectionClosed if
_active_websocket closes after the None check; wrap the await
_active_websocket.send(...) call in a try/except that catches the websocket
connection closed error (e.g., websockets.exceptions.ConnectionClosed or the
relevant library exception) inside send_button_event, log the failure with
logger.debug/logger.info including the button_state and the exception, and
return gracefully so the exception doesn't propagate to callers.
|
| Metric | Count |
|---|---|
| ✅ Passed | 102 |
| ❌ Failed | 17 |
| 📊 Total | 119 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html-no-api - HTML reports
- robot-test-results-xml-no-api - XML output
…ction
Summary by CodeRabbit
Release Notes
New Features
Documentation