Improve voice activation, add va status skill and improve hud rendering - #405
Conversation
Shackless
left a comment
There was a problem hiding this comment.
Thanks for the PR! The core idea of the toggle fix is sound — I verified that all real end-of-playback paths reliably fire on_playback_finished, and streamed responses produce exactly one started/finished cycle, so the saved intent is applied correctly in the normal flow. The outline-alpha change also checks out (content-layer borders staying opaque is consistent with the design).
I left inline comments on the issues found during review, roughly in order of severity:
- Race between the new playback branch and
on_playback_startedthat can silently swallow a mute press (wingman_core.py) - The skill's permanent
/wsconnection breaks Core's offline-message queueing for the real client (mic_status/main.py) - The intent fix only covers the hotkey path — the
/voice-activation/muteendpoint still has the old bug (wingman_core.py) - Icon paths containing
)never render, and the code comment about the parser is wrong (mic_status/main.py) - After a HUD server restart the icon comes back as a default-styled window, and failed draws are cached as successful (mic_status/main.py)
- The
connectedguard inupdate_configdrops config changes after any transient HUD hiccup (mic_status/main.py) - Reading Core internals via
sys.modules["__main__"]bypasses the skill facade — uniquely among bundled skills (mic_status/main.py) icon_sizecode default (96) drifted from default_config.yaml (72) (mic_status/main.py)
Minor nit not worth its own thread: (55, 62, 74) is now hardcoded in five places across hud_server; a DEFAULT_BORDER_COLOR in hud_server/constants.py would fit the existing pattern there.
|
All issues resolved in newest commit. I moved away from "lets not touch the core where I dont have to" to "lets modify core to give me what I need for a clean state". |
…sive during previews
…T setting changes apply without revalidation
…en condensation is skipped
|
I think its good to go now. Also included some elevenlabs adjustments for playback state detection fixes |
Shackless
left a comment
There was a problem hiding this comment.
Second review round — verified all 8 findings from the previous round against the new commits: all properly resolved ✓ (intent model, endpoint path, WS removal, paren regex, HUD restart/create-or-update, .connected guard, facade surface, yaml default sync). The sound_config parameter also fixes a genuine latent NameError in develop's direct-stream path — nice catch.
Four new findings below, posted inline: 1 blocker (VA doesn't resume after a naturally ending ElevenLabs playback — deterministic, both paths), 1 comment/code mismatch, 2 minor notes.
Note on the diff: the branch carries cherry-picked copies of four develop commits (condenser, voice service, config context, template renames) with different SHAs, which inflates the PR diff to 27 files. The effective diff vs. develop is 14 files. A rebase onto current develop before merge would clean that up.
|
@Shackless Open for review again. |
Also references #402
Summary
Improve voice activation, add va status skil and improve hud rendering
Changes
Testing
Checklist
develop