rhasspy / wyoming-satellite

Remote voice satellite using Wyoming protocol
MIT License
578 stars 83 forks source link

New update_info method causing side effects on new satellite setup #124

Open llluis opened 6 months ago

llluis commented 6 months ago

Hello @synesthesiam.

This change https://github.com/rhasspy/wyoming-satellite/blob/962b2f5dc6ed0d8b05671f1ade092407ed6251cd/wyoming_satellite/satellite.py#L1330 from https://github.com/rhasspy/wyoming-satellite/commit/70652bbea2285621271b5b5fb9938924a7684fbf is causing some side effects when creating new satellites (I believe there's no issue if the satellite is already configured in HA).

Before submitting a fix, I would like to confirm is intended.

This causes the satellite reporting to HA that wake word is installed during zeroconf discovery. First side effect: this will cause https://github.com/home-assistant/core/blob/d5e1934942f4c629e896603787b6970df3c88004/homeassistant/components/wyoming/data.py#L44 to return openWakeWord as name instead of the satellite name, causing confusion during setup.

image

Second side effect: the config flow in effectvely creating a new wake_word entity in HA:

image

However, I don't think the satellite is able to forward / handle wake word detections from streamings from HA. At least, I couldn't make my Atom work properly with it.

The last commit in HA https://github.com/home-assistant/core/commit/f6622ea8e0a46e61b0ebba5008cb04e18375ef76 by @balloob doesn't seem to address this, just use the info from Info event for other stuff.

I believe a simple fix would be changing wake.installed to false in the local Describe event would prevent both side effects in HA while still allowing the usage of the rest of the info.

llluis commented 6 months ago

Forgot to mention that second issue won't show the openWakeWord entity under the Wyoming integration (as it's probably attached to the satellite device).

Did a quick test and indeed, changing Installed to False fixes both issues.

kbromer commented 6 months ago

Confirming this behavior (fine for existing satellites, incorrect for a newly setup one), thought something was misconfigured with my wyoming implementation, glad to know its not just me.

synesthesiam commented 6 months ago

Thanks! Got a PR in for a fix :+1: