Skip to content
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

VOIP - handle ongoing calls for announce and start conversation #136970

Closed
wants to merge 1 commit into from

Conversation

jaminh
Copy link
Contributor

@jaminh jaminh commented Jan 31, 2025

Allow announcements and conversation starts to be queued up for an existing call. Also allow specifying a SIP username for the HA endpoint which is required for some VOIP servers.

I tried to combine elements from the current implementation and my original implementation to handle announcements and conversation start when there is already a call to the specified satellite.

Proposed change

Allow announcements and conversation starts to be queued up for an existing call. Also allow specifying a SIP username for the HA endpoint which is required for some VOIP servers.

I tried to combine elements from the current implementation and my original implementation to handle announcements and conversation start when there is already a call to the specified satellite.

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:

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:

Allow announcements and conversation starts to be queued up for an
existing call. Also allow specifying a SIP username for the HA endpoint
which is required for some VOIP servers.
@home-assistant
Copy link

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

Code owner commands

Code owners of wyoming 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 wyoming 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.

@home-assistant
Copy link

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.

@synesthesiam
Copy link
Contributor

Thanks!

Can we make the SIP user stuff a separate PR?

What is the use case for queuing up announcements and calls? It makes the code much more complex for what seems to me an edge case.

@jaminh
Copy link
Contributor Author

jaminh commented Jan 31, 2025

Yeah, I'll try to split things up into more discrete features.

There were a couple things I was trying to handle with the queuing of service calls.

The first case would be if someone had called in to HA and was saying commands and something happened in the house that triggers an announcement. In that case we could play the announcement after their command processed and then go back to the regular command loop.

The second scenario I had in mind was commands or questions in a start conversation action that require follow up questions. So potentially the action called in response to a command could be a call to start conversation, and then that question could be asked in the same call.

Edit: I thought of a more concrete example of the second use case. Suppose someone called in to change the temperature in a room, but there are multiple climate devices in the room. Instead of just responding with an error response about ambiguous climate devices it could call start conversation to ask which climate device to change. Not sure if the ability to pass along that context is available yet in start conversation but I assume that's the direction you are heading.

@jaminh jaminh closed this Jan 31, 2025
@synesthesiam
Copy link
Contributor

The second case will be handled with future improvements to the builtin conversation agent to handle follow-up questions. The conversation id will keep the context with nothing special needed from VoIP 👍

For the first case, I'm still of the opinion that this isn't worth the large increase in code complexity. As the maintainer of this integration, I need to draw the line somewhere 😄

@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants