VoIP: Add support for announce #135291

jaminh · 2025-01-10T13:51:31Z

Breaking change

Proposed change

This adds the "Announce" service to the VOIP Assist Satellite component. Currently when announce is called HA will call the phone and go into the standard assist pipeline when the call is answered.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2025-01-10T13:51:39Z

Hey there @balloob, @synesthesiam, mind taking a look at this pull request as it has been labeled with an integration (voip) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of voip can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign voip Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

jaminh · 2025-01-10T13:57:07Z

We will need to add some way for the HassVoipDatagramProtocol to pick an appropriate protocol factory for the different use cases: HA receiving a call, HA calling out for an announcement, and HA calling out as part of the start_conversation service.

I imagine at some point we will want to account for what happens when one of those services, announce or start_conversation, is called while there is an active call with the target VOIP device. Currently the code assumes there is not an active call and will always start by calling the phone.

homeassistant/components/voip/assist_satellite.py

balloob · 2025-01-11T01:38:37Z

homeassistant/components/voip/config_flow.py

+                        ),
+                    ): str,
+                    vol.Required(
+                        CONF_SIP_HOST,


Why do we want to ask the user these values?

Is this to get the IP address of the HA box? We should instead import async_get_source_ip from homeassistant.components.network

Is that guaranteed to be the external IP for the HA box? I think there would still be value in allowing the host sent in the outgoing invite message to be configurable, but using async_get_source_ip as a default value would make sense. Depending on SIP server configurations a specific user might be required in the INVITE From header, so even though that configuration isn't officially supported, it is helpful for me. If we make those optional with sane defaults is that still a workable solution?

We don't want them in the main configuration flow, as users will be confused (we want to ask as little information as possible when setting up an integration).

Can you try if it works for you with async_get_source_ip ?

I moved user and host to an "advanced options" page and defaulted host to the value of async_get_source_ip, I hope that is an acceptable solution?

No, let's stick with just calling async_get_source_ip when we need to do an announcement. That method exists to get the right IP address for exactly this reason:

@bind_hass async def async_get_source_ip( hass: HomeAssistant, target_ip: str | UndefinedType = UNDEFINED ) -> str: """Get the source ip for a target ip."""

Adding this option puts an extra burden on the user having a bunch of IP addresses configured hidden inside config entry configurations.

homeassistant/components/voip/devices.py

balloob · 2025-01-11T01:41:16Z

homeassistant/components/voip/devices.py

+        voip_id = (
+            call_info.caller_endpoint.uri
+            if call_info.caller_endpoint
+            else call_info.caller_ip


Can we do an auto-migration here? So every incoming device, we check if it exists under the old ID, and if so, we migrate it to the new endpoint uri.

I tried to update it to do an auto-migration, but it would be difficult for me to test that since I already have new devices for all of my phones.

balloob

Can you break out the caller ID breaking change into its own PR? I also added a note how we can try to auto-migrate old devices automatically. Would be cool to add to that PR if possible.

home-assistant · 2025-01-11T01:51:03Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

balloob · 2025-01-11T02:12:19Z

Currently when announce is called HA will call the phone and go into the standard assist pipeline when the call is answered.

We should also make sure that we start with speaking the announcement message. Once we did that, Home Assistant should hang up. Users should use start_conversation if they want to continue talking.

jaminh · 2025-01-14T14:29:17Z

Can you break out the caller ID breaking change into its own PR? I also added a note how we can try to auto-migrate old devices automatically. Would be cool to add to that PR if possible.

#135603

homeassistant/components/voip/assist_satellite.py

jaminh · 2025-01-18T02:44:43Z

homeassistant/components/assist_satellite/entity.py

@@ -416,6 +416,7 @@ async def _resolve_announcement_media_id(
                language=pipeline.tts_language,
                options=tts_options,
            )
+            return AssistSatelliteAnnouncement(message, media_id, media_id_source)


Seems like this has the potential to affect other assist satellite components, but the VOIP announcements wouldn't work otherwise.

I added an internal_media_id property to the AssistSatelliteAnnouncement to hopefully mitigate this issue.

jaminh · 2025-01-20T02:56:50Z

Currently when announce is called HA will call the phone and go into the standard assist pipeline when the call is answered.

We should also make sure that we start with speaking the announcement message. Once we did that, Home Assistant should hang up. Users should use start_conversation if they want to continue talking.

This PR is already pretty unwieldy as it is, I figure we can get this stable and add start_conversation support as a separate PR.

balloob · 2025-01-21T03:30:22Z

homeassistant/components/assist_satellite/entity.py

@@ -96,7 +96,11 @@ class AssistSatelliteAnnouncement:
    media_id: str
    """Media ID to be played."""

+    internal_media_id: str


I've extracted this as #136141 and renamed to original_media_id

Should I rebase this branch to pick up that change or clean that up later?

balloob · 2025-01-21T03:38:23Z

homeassistant/components/voip/voip.py

+    def on_call(self, call_info: CallInfo):
+        """Set up state when starting a call."""
+        device = self.devices.async_get_or_create(call_info)
+        if device is not None:


The response can never be None.

balloob · 2025-01-21T03:39:21Z

homeassistant/components/voip/voip.py

+        """Set up state when starting a call."""
+        device = self.devices.async_get_or_create(call_info)
+        if device is not None:
+            device.current_call = call_info


Is this something that we want to pass to set_active instead? Having a call info then indicates it's active.

synesthesiam · 2025-01-21T17:08:04Z

homeassistant/components/wyoming/stt.py

@@ -126,6 +126,9 @@ async def async_process_audio_stream(
                        text = transcript.text
                        break

+        except TimeoutError:


Why was this added?

Without it the call state wouldn't get properly cleaned up which I think was causing subsequent calls to not create a new RTP server and there would be no audio on the phone.

I removed it in the latest change, I am still having issues with it but it is not the same as before so that might have been a red herring when I added it the first time. I think I need to increase the timeout value because it seems like when I say a command it will make the tone like it heard the command, but then shortly after it hangs up on me from what I assume is caused by the timeout error.

I added this back in because hanging up after the regular command pipeline loop wouldn't work without it.

synesthesiam · 2025-01-21T17:11:39Z

We need to add an implementation for this still, right?
TypeError: Can't instantiate abstract class HassVoipDatagramProtocol without an implementation for abstract method 'on_hangup'

jaminh · 2025-01-22T01:44:01Z

We need to add an implementation for this still, right? TypeError: Can't instantiate abstract class HassVoipDatagramProtocol without an implementation for abstract method 'on_hangup'

It should be implemented, line 134 homeassistant/components/voip/voip.py

balloob · 2025-01-23T05:12:29Z

It's not explicitely mentioned, but this PR requires home-assistant-libs/voip-utils#25 to be merged and released right?

jaminh · 2025-01-23T12:24:07Z

It's not explicitely mentioned, but this PR requires home-assistant-libs/voip-utils#25 to be merged and released right?

Yes, is there a way to specify an unreleased version in the manifest?

Edit: I think I found it, https://developers.home-assistant.io/docs/creating_integration_manifest/#custom-requirements-during-development--testing

Currently when making an announcement the phone will be called and will go into the regular assist pipeline when the call is answered.

The create rtp server method is in voip_utils

Made a queue of pipeline operations that can be added to in case the announcement service is called during an ongoing call.

Improve task queue handling by using put_nowait for adding tasks to avoid the need for creating a task for adding items. The task queue should also be cleared when the call ends. If the call needs to end, either because it is the end of an announcement or there was an unrecoverable error, HA should initiate a hang up in an attempt to cleanly end the call.

jaminh · 2025-01-24T03:54:55Z

The pre-commit checks complain if I try to set the voip_utils requirement to point to my branch, so I left that as is.

I am still having an issue where when I hang up after doing the regular voice command pipeline stuff the TimeoutError isn't being caught in the _run_pipeline method which means the call isn't hung up. This means if I try to then play an announcement after that it thinks the previous call is ongoing and tries to play the announcement on that non-existent call. I still haven't figured out a way to get that to work such that it hangs up after I hang up my phone without it randomly hanging up in the middle of processing commands.

jaminh requested review from balloob and synesthesiam as code owners January 10, 2025 13:51

home-assistant bot added cla-signed integration: voip new-feature labels Jan 10, 2025

home-assistant bot assigned balloob and synesthesiam Jan 10, 2025

home-assistant bot added the Quality Scale: internal label Jan 10, 2025

balloob reviewed Jan 11, 2025

View reviewed changes

homeassistant/components/voip/assist_satellite.py Outdated Show resolved Hide resolved

balloob reviewed Jan 11, 2025

View reviewed changes

homeassistant/components/voip/assist_satellite.py Show resolved Hide resolved

balloob reviewed Jan 11, 2025

View reviewed changes

homeassistant/components/voip/devices.py Outdated Show resolved Hide resolved

balloob reviewed Jan 11, 2025

View reviewed changes

balloob changed the title ~~Voip assist sat announce~~ VoIP: Add support for announce Jan 11, 2025

balloob requested changes Jan 11, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 11, 2025 01:51

jaminh force-pushed the voip-assist-sat-announce branch from 430dda1 to a4b7063 Compare January 12, 2025 20:44

jaminh force-pushed the voip-assist-sat-announce branch from 293d6b6 to 8216fd1 Compare January 16, 2025 02:19

jaminh commented Jan 18, 2025

View reviewed changes

homeassistant/components/voip/assist_satellite.py Outdated Show resolved Hide resolved

jaminh commented Jan 18, 2025

View reviewed changes

homeassistant/components/voip/assist_satellite.py Show resolved Hide resolved

jaminh commented Jan 18, 2025

View reviewed changes

jaminh marked this pull request as ready for review January 18, 2025 02:45

jaminh requested a review from a team as a code owner January 18, 2025 02:45

home-assistant bot requested a review from balloob January 18, 2025 02:45

balloob mentioned this pull request Jan 21, 2025

Satellite announcement to track original media id #136141

Merged

19 tasks

balloob reviewed Jan 21, 2025

View reviewed changes

synesthesiam reviewed Jan 21, 2025

View reviewed changes

jaminh added 9 commits January 23, 2025 07:54

Add the assist satelite announce feature for VOIP component.

b4e62ac

Currently when making an announcement the phone will be called and will go into the regular assist pipeline when the call is answered.

Make VOIP host configuration optional

62acf4a

Add advanced options for Voip host and user

1b0e06b

Remove create rtp server method

30b13aa

The create rtp server method is in voip_utils

Outgoing calls seem to be working

6da9e41

Made a queue of pipeline operations that can be added to in case the announcement service is called during an ongoing call.

Use original media ID for announcement

6b3463f

Change set_is_active to take CallInfo

4daa79a

Remove host configuration

4897501

jaminh force-pushed the voip-assist-sat-announce branch from 6796078 to 4897501 Compare January 23, 2025 14:14

Remove unnecessary audio streaming from announce

c144e6e

Restore catching/re-raising TimeoutError in STT

cb1398e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VoIP: Add support for announce #135291

VoIP: Add support for announce #135291

jaminh commented Jan 10, 2025 •

edited by balloob

Loading

home-assistant bot commented Jan 10, 2025

jaminh commented Jan 10, 2025

balloob Jan 11, 2025

balloob Jan 11, 2025

jaminh Jan 11, 2025

balloob Jan 13, 2025

jaminh Jan 20, 2025

balloob Jan 21, 2025

balloob Jan 11, 2025

jaminh Jan 20, 2025

balloob left a comment

home-assistant bot commented Jan 11, 2025

balloob commented Jan 11, 2025

jaminh commented Jan 14, 2025

jaminh Jan 18, 2025

jaminh Jan 20, 2025

jaminh commented Jan 20, 2025

balloob Jan 21, 2025

jaminh Jan 22, 2025

balloob Jan 23, 2025

balloob Jan 21, 2025

balloob Jan 21, 2025

synesthesiam Jan 21, 2025

jaminh Jan 22, 2025

jaminh Jan 22, 2025

jaminh Jan 24, 2025

synesthesiam commented Jan 21, 2025

jaminh commented Jan 22, 2025

balloob commented Jan 23, 2025

jaminh commented Jan 23, 2025 •

edited

Loading

jaminh commented Jan 24, 2025

VoIP: Add support for announce #135291

Are you sure you want to change the base?

VoIP: Add support for announce #135291

Conversation

jaminh commented Jan 10, 2025 • edited by balloob Loading

Breaking change

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Jan 10, 2025

jaminh commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob left a comment

Choose a reason for hiding this comment

home-assistant bot commented Jan 11, 2025

balloob commented Jan 11, 2025

jaminh commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaminh commented Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

synesthesiam commented Jan 21, 2025

jaminh commented Jan 22, 2025

balloob commented Jan 23, 2025

jaminh commented Jan 23, 2025 • edited Loading

jaminh commented Jan 24, 2025

jaminh commented Jan 10, 2025 •

edited by balloob

Loading

jaminh commented Jan 23, 2025 •

edited

Loading