Allow configuring SIP port in VoIP (#92210)

Co-authored-by: Franck Nijhof <git@frenck.dev>

homeassistant/components/voip/__init__.py

@@ -14,7 +14,7 @@ from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import device_registry as dr
 
-from .const import DOMAIN
+from .const import CONF_SIP_PORT, DOMAIN
 from .devices import VoIPDevices
 from .voip import HassVoipDatagramProtocol
 
@@ -39,6 +39,7 @@ class DomainData:
     """Domain data."""
 
     transport: asyncio.DatagramTransport
+    protocol: HassVoipDatagramProtocol
     devices: VoIPDevices
 
 
@@ -56,41 +57,57 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
             entry, data={**entry.data, "user": voip_user.id}
         )
 
+    sip_port = entry.options.get(CONF_SIP_PORT, SIP_PORT)
     devices = VoIPDevices(hass, entry)
     devices.async_setup()
-    transport = await _create_sip_server(
+    transport, protocol = await _create_sip_server(
         hass,
         lambda: HassVoipDatagramProtocol(hass, devices),
+        sip_port,
     )
-    _LOGGER.debug("Listening for VoIP calls on port %s", SIP_PORT)
+    _LOGGER.debug("Listening for VoIP calls on port %s", sip_port)
 
-    hass.data[DOMAIN] = DomainData(transport, devices)
+    hass.data[DOMAIN] = DomainData(transport, protocol, devices)
 
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
+    entry.async_on_unload(entry.add_update_listener(update_listener))
+
     return True
 
 
+async def update_listener(hass: HomeAssistant, entry: ConfigEntry):
+    """Handle options update."""
+    await hass.config_entries.async_reload(entry.entry_id)
+
+
 async def _create_sip_server(
     hass: HomeAssistant,
     protocol_factory: Callable[
         [],
         asyncio.DatagramProtocol,
     ],
-) -> asyncio.DatagramTransport:
+    sip_port: int,
-    transport, _protocol = await hass.loop.create_datagram_endpoint(
+) -> tuple[asyncio.DatagramTransport, HassVoipDatagramProtocol]:
+    transport, protocol = await hass.loop.create_datagram_endpoint(
         protocol_factory,
-        local_addr=(_IP_WILDCARD, SIP_PORT),
+        local_addr=(_IP_WILDCARD, sip_port),
     )
 
-    return transport
+    if not isinstance(protocol, HassVoipDatagramProtocol):
+        raise TypeError(f"Expected HassVoipDatagramProtocol, got {protocol}")
+
+    return transport, protocol
 
 
 async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Unload VoIP."""
     if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS):
-        _LOGGER.debug("Shut down VoIP server")
+        _LOGGER.debug("Shutting down VoIP server")
-        hass.data.pop(DOMAIN).transport.close()
+        data = hass.data.pop(DOMAIN)
+        data.transport.close()
+        await data.protocol.wait_closed()
+        _LOGGER.debug("VoIP server shut down successfully")
 
     return unload_ok
 

homeassistant/components/voip/config_flow.py

@@ -3,10 +3,15 @@ from __future__ import annotations
 
 from typing import Any
 
-from homeassistant import config_entries
+from voip_utils import SIP_PORT
-from homeassistant.data_entry_flow import FlowResult
+import voluptuous as vol
 
-from .const import DOMAIN
+from homeassistant import config_entries
+from homeassistant.core import callback
+from homeassistant.data_entry_flow import FlowResult
+from homeassistant.helpers import config_validation as cv
+
+from .const import CONF_SIP_PORT, DOMAIN
 
 
 class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
@@ -22,9 +27,49 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
             return self.async_abort(reason="single_instance_allowed")
 
         if user_input is None:
-            return self.async_show_form(step_id="user")
+            return self.async_show_form(
+                step_id="user",
+            )
 
         return self.async_create_entry(
             title="Voice over IP",
             data=user_input,
         )
+
+    @staticmethod
+    @callback
+    def async_get_options_flow(
+        config_entry: config_entries.ConfigEntry,
+    ) -> config_entries.OptionsFlow:
+        """Create the options flow."""
+        return VoipOptionsFlowHandler(config_entry)
+
+
+class VoipOptionsFlowHandler(config_entries.OptionsFlow):
+    """Handle VoIP options."""
+
+    def __init__(self, config_entry: config_entries.ConfigEntry) -> None:
+        """Initialize options flow."""
+        self.config_entry = config_entry
+
+    async def async_step_init(
+        self, user_input: dict[str, Any] | None = None
+    ) -> FlowResult:
+        """Manage the options."""
+        if user_input is not None:
+            return self.async_create_entry(title="", data=user_input)
+
+        return self.async_show_form(
+            step_id="init",
+            data_schema=vol.Schema(
+                {
+                    vol.Required(
+                        CONF_SIP_PORT,
+                        default=self.config_entry.options.get(
+                            CONF_SIP_PORT,
+                            SIP_PORT,
+                        ),
+                    ): cv.port
+                }
+            ),
+        )

homeassistant/components/voip/const.py

@@ -11,3 +11,5 @@ RTP_AUDIO_SETTINGS = {
     "channels": CHANNELS,
     "sleep_ratio": 0.99,
 }
+
+CONF_SIP_PORT = "sip_port"

homeassistant/components/voip/strings.json

@@ -28,5 +28,14 @@
         }
       }
     }
+  },
+  "options": {
+    "step": {
+      "init": {
+        "data": {
+          "sip_port": "SIP port"
+        }
+      }
+    }
   }
 }

homeassistant/components/voip/voip.py

@@ -84,12 +84,21 @@ class HassVoipDatagramProtocol(VoipDatagramProtocol):
         )
         self.hass = hass
         self.devices = devices
+        self._closed_event = asyncio.Event()
 
     def is_valid_call(self, call_info: CallInfo) -> bool:
         """Filter calls."""
         device = self.devices.async_get_or_create(call_info)
         return device.async_allow_call(self.hass)
 
+    def connection_lost(self, exc):
+        """Signal wait_closed when transport is completely closed."""
+        self.hass.loop.call_soon_threadsafe(self._closed_event.set)
+
+    async def wait_closed(self) -> None:
+        """Wait for connection_lost to be called."""
+        await self._closed_event.wait()
+
 
 class PipelineRtpDatagramProtocol(RtpDatagramProtocol):
     """Run a voice assistant pipeline in a loop for a VoIP call."""

tests/components/voip/conftest.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-from unittest.mock import Mock, patch
+from unittest.mock import AsyncMock, Mock, patch
 
 import pytest
 from voip_utils import CallInfo
@@ -33,7 +33,10 @@ def config_entry(hass: HomeAssistant) -> MockConfigEntry:
 @pytest.fixture
 async def setup_voip(hass: HomeAssistant, config_entry: MockConfigEntry) -> None:
     """Set up VoIP integration."""
-    with patch("homeassistant.components.voip._create_sip_server", return_value=Mock()):
+    with patch(
+        "homeassistant.components.voip._create_sip_server",
+        return_value=(Mock(), AsyncMock()),
+    ):
         assert await async_setup_component(hass, DOMAIN, {})
         assert config_entry.state == ConfigEntryState.LOADED
         yield

tests/components/voip/test_config_flow.py

@@ -1,11 +1,13 @@
 """Test VoIP config flow."""
 from unittest.mock import patch
 
-from homeassistant import config_entries
+from homeassistant import config_entries, data_entry_flow
 from homeassistant.components import voip
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 
+from tests.common import MockConfigEntry
+
 
 async def test_form_user(hass: HomeAssistant) -> None:
     """Test user form config flow."""
@@ -40,3 +42,40 @@ async def test_single_instance(
     )
     assert result["type"] == "abort"
     assert result["reason"] == "single_instance_allowed"
+
+
+async def test_options_flow(hass: HomeAssistant) -> None:
+    """Test config flow options."""
+    config_entry = MockConfigEntry(
+        domain=voip.DOMAIN,
+        data={},
+        unique_id="1234",
+    )
+    config_entry.add_to_hass(hass)
+
+    assert config_entry.options == {}
+
+    result = await hass.config_entries.options.async_init(
+        config_entry.entry_id,
+    )
+    assert result["type"] == data_entry_flow.FlowResultType.FORM
+    assert result["step_id"] == "init"
+
+    # Default
+    result = await hass.config_entries.options.async_configure(
+        result["flow_id"],
+        user_input={},
+    )
+    assert result["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
+    assert config_entry.options == {"sip_port": 5060}
+
+    # Manual
+    result = await hass.config_entries.options.async_init(
+        config_entry.entry_id,
+    )
+    result = await hass.config_entries.options.async_configure(
+        result["flow_id"],
+        user_input={"sip_port": 5061},
+    )
+    assert result["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
+    assert config_entry.options == {"sip_port": 5061}

tests/components/voip/test_sip.py

@@ -0,0 +1,55 @@
+"""Test SIP server."""
+import socket
+
+import pytest
+
+from homeassistant import config_entries
+from homeassistant.components import voip
+from homeassistant.core import HomeAssistant
+
+
+async def test_create_sip_server(hass: HomeAssistant, socket_enabled) -> None:
+    """Tests starting/stopping SIP server."""
+    result = await hass.config_entries.flow.async_init(
+        voip.DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        {},
+    )
+    entry = result["result"]
+    await hass.async_block_till_done()
+
+    with pytest.raises(OSError), socket.socket(
+        socket.AF_INET, socket.SOCK_DGRAM
+    ) as sock:
+        # Server should have the port
+        sock.bind(("127.0.0.1", 5060))
+
+    # Configure different port
+    result = await hass.config_entries.options.async_init(
+        entry.entry_id,
+    )
+    result = await hass.config_entries.options.async_configure(
+        result["flow_id"],
+        user_input={"sip_port": 5061},
+    )
+    await hass.async_block_till_done()
+
+    # Server should be stopped now on 5060
+    with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:
+        sock.bind(("127.0.0.1", 5060))
+
+    with pytest.raises(OSError), socket.socket(
+        socket.AF_INET, socket.SOCK_DGRAM
+    ) as sock:
+        # Server should now have the new port
+        sock.bind(("127.0.0.1", 5061))
+
+    # Shut down
+    await hass.config_entries.async_remove(entry.entry_id)
+    await hass.async_block_till_done()
+
+    # Server should be stopped
+    with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:
+        sock.bind(("127.0.0.1", 5061))

tests/components/voip/test_voip.py

@@ -237,9 +237,15 @@ async def test_tts_timeout(
             )
         )
 
-    def send_audio(*args, **kwargs):
+    tone_bytes = bytes([1, 2, 3, 4])
+
+    def send_audio(audio_bytes, **kwargs):
+        if audio_bytes == tone_bytes:
+            # Not TTS
+            return
+
         # Block here to force a timeout in _send_tts
-        time.sleep(1)
+        time.sleep(2)
 
     async def async_get_media_source_audio(
         hass: HomeAssistant,
@@ -263,10 +269,13 @@ async def test_tts_timeout(
             hass.config.language,
             voip_device,
             Context(),
-            listening_tone_enabled=False,
+            listening_tone_enabled=True,
-            processing_tone_enabled=False,
+            processing_tone_enabled=True,
-            error_tone_enabled=False,
+            error_tone_enabled=True,
         )
+        rtp_protocol._tone_bytes = tone_bytes
+        rtp_protocol._processing_bytes = tone_bytes
+        rtp_protocol._error_bytes = tone_bytes
         rtp_protocol.transport = Mock()
         rtp_protocol.send_audio = Mock(side_effect=send_audio)