Use dbus-fast unpack_variants option (#3885)

* Use dbus-fast unpack_variants option

* More readable log on signals

requirements.txt

@@ -22,4 +22,4 @@ ruamel.yaml==0.17.21
 securetar==2022.2.0
 sentry-sdk==1.9.8
 voluptuous==0.13.1
-dbus-fast==1.6.0
+dbus-fast==1.7.0

supervisor/dbus/network/setting/__init__.py

@@ -130,7 +130,7 @@ class NetworkSetting(DBusInterface):
     async def update(self, settings: Any) -> None:
         """Update connection settings."""
         new_settings = await self.dbus.Settings.Connection.call_get_settings(
-            remove_signature=False
+            unpack_variants=False
         )
 
         _merge_settings_attribute(new_settings, settings, CONF_ATTR_CONNECTION)
@@ -165,9 +165,7 @@ class NetworkSetting(DBusInterface):
         await super().connect(bus)
         await self.reload()
 
-        # pylint: disable=unnecessary-lambda
+        self.dbus.Settings.Connection.on_updated(self.reload)
-        # wrapper created by annotation fails the signature test, varargs not supported
-        self.dbus.Settings.Connection.on_updated(lambda: self.reload())
 
     def disconnect(self) -> None:
         """Disconnect from D-Bus."""

supervisor/host/network.py

@@ -36,7 +36,6 @@ from ..exceptions import (
 from ..jobs.const import JobCondition
 from ..jobs.decorator import Job
 from ..resolution.checks.network_interface_ipv4 import CheckNetworkInterfaceIPV4
-from ..utils.dbus import DBus
 from .const import AuthMethod, InterfaceMethod, InterfaceType, WifiMode
 
 _LOGGER: logging.Logger = logging.getLogger(__name__)
@@ -148,7 +147,6 @@ class NetworkManager(CoreSysAttributes):
         if interface != DBUS_IFACE_NM:
             return
 
-        changed = DBus.remove_dbus_signature(changed)
         connectivity_check: bool | None = changed.get(DBUS_ATTR_CONNECTION_ENABLED)
         connectivity: bool | None = changed.get(DBUS_ATTR_CONNECTIVITY)
 

supervisor/utils/dbus.py

@@ -54,17 +54,6 @@ class DBus:
         _LOGGER.debug("Connect to D-Bus: %s - %s", bus_name, object_path)
         return self
 
-    @staticmethod
-    def remove_dbus_signature(data: Any) -> Any:
-        """Remove signature info."""
-        if isinstance(data, Variant):
-            return DBus.remove_dbus_signature(data.value)
-        if isinstance(data, dict):
-            return {k: DBus.remove_dbus_signature(v) for k, v in data.items()}
-        if isinstance(data, list):
-            return [DBus.remove_dbus_signature(item) for item in data]
-        return data
-
     @staticmethod
     def from_dbus_error(err: DBusError) -> HassioNotSupportedError | DBusError:
         """Return correct dbus error based on type."""
@@ -91,7 +80,7 @@ class DBus:
         proxy_interface: ProxyInterface,
         method: str,
         *args,
-        remove_signature: bool = True,
+        unpack_variants: bool = True,
     ) -> Any:
         """Call a dbus method and handle the signature and errors."""
         _LOGGER.debug(
@@ -101,8 +90,9 @@ class DBus:
             proxy_interface.path,
         )
         try:
-            body = await getattr(proxy_interface, method)(*args)
+            return await getattr(proxy_interface, method)(
-            return DBus.remove_dbus_signature(body) if remove_signature else body
+                *args, unpack_variants=unpack_variants
+            )
         except DBusError as err:
             raise DBus.from_dbus_error(err)
 
@@ -178,10 +168,18 @@ class DBus:
             if interface != prop_interface:
                 return
 
+            _LOGGER.debug(
+                "Property change for %s-%s: %s changed & %s invalidated",
+                self.bus_name,
+                self.object_path,
+                changed.keys(),
+                invalidated,
+            )
+
             if invalidated:
                 await update()
             else:
-                await update(DBus.remove_dbus_signature(changed))
+                await update(changed)
 
         self.properties.on_properties_changed(sync_property_change)
         return sync_property_change
@@ -195,7 +193,9 @@ class DBus:
         for intr, signals in self._signal_monitors.items():
             for name, callbacks in signals.items():
                 for callback in callbacks:
-                    getattr(self._proxies[intr], f"off_{name}")(callback)
+                    getattr(self._proxies[intr], f"off_{name}")(
+                        callback, unpack_variants=True
+                    )
 
         self._signal_monitors = {}
 
@@ -254,7 +254,7 @@ class DBusCallWrapper:
             if dbus_type == "on":
 
                 def _on_signal(callback: Callable):
-                    getattr(self._proxy, name)(callback)
+                    getattr(self._proxy, name)(callback, unpack_variants=True)
 
                     # pylint: disable=protected-access
                     if self.interface not in self.dbus._signal_monitors:
@@ -272,7 +272,7 @@ class DBusCallWrapper:
                 return _on_signal
 
             def _off_signal(callback: Callable):
-                getattr(self._proxy, name)(callback)
+                getattr(self._proxy, name)(callback, unpack_variants=True)
 
                 # pylint: disable=protected-access
                 if (
@@ -298,9 +298,9 @@ class DBusCallWrapper:
 
         if dbus_type in ["call", "get", "set"]:
 
-            def _method_wrapper(*args, remove_signature: bool = True) -> Awaitable:
+            def _method_wrapper(*args, unpack_variants: bool = True) -> Awaitable:
                 return DBus.call_dbus(
-                    self._proxy, name, *args, remove_signature=remove_signature
+                    self._proxy, name, *args, unpack_variants=unpack_variants
                 )
 
             return _method_wrapper

tests/conftest.py

@@ -171,7 +171,7 @@ def dbus(dbus_bus: MessageBus) -> DBus:
         proxy_interface: ProxyInterface,
         method: str,
         *args,
-        remove_signature: bool = True,
+        unpack_variants: bool = True,
     ):
         if (
             proxy_interface.introspection.name == DBUS_INTERFACE_PROPERTIES

tests/dbus/network/setting/test_init.py

@@ -72,10 +72,10 @@ SETTINGS_WITH_SIGNATURE = {
 
 
 async def mock_call_dbus_get_settings_signature(
-    _: ProxyInterface, method: str, *args, remove_signature: bool = True
+    _: ProxyInterface, method: str, *args, unpack_variants: bool = True
 ) -> list[dict[str, Any]]:
     """Call dbus method mock for get settings that keeps signature."""
-    if method == "call_get_settings" and not remove_signature:
+    if method == "call_get_settings" and not unpack_variants:
         return SETTINGS_WITH_SIGNATURE
     else:
         assert method == "call_update"

tests/dbus/test_interface.py

@@ -80,7 +80,7 @@ async def test_signal_listener_disconnect(
         device = dev
 
     proxy.obj.dbus.on_device_added(callback)
-    proxy.on_device_added.assert_called_once_with(callback)
+    proxy.on_device_added.assert_called_once_with(callback, unpack_variants=True)
 
     fire_watched_signal(
         proxy.obj, "org.freedesktop.NetworkManager.DeviceAdded", ["/test/obj/1"]
@@ -89,4 +89,4 @@ async def test_signal_listener_disconnect(
     assert device == "/test/obj/1"
 
     proxy.obj.disconnect()
-    proxy.off_device_added.assert_called_once_with(callback)
+    proxy.off_device_added.assert_called_once_with(callback, unpack_variants=True)

tests/host/test_network.py

@@ -140,7 +140,7 @@ async def test_scan_wifi_with_failures(coresys: CoreSys, caplog):
         proxy_interface: ProxyInterface,
         method: str,
         *args,
-        remove_signature: bool = True,
+        unpack_variants: bool = True,
     ):
         if method == "call_get_all_access_points":
             return [
@@ -150,7 +150,7 @@ async def test_scan_wifi_with_failures(coresys: CoreSys, caplog):
             ]
 
         return await call_dbus(
-            proxy_interface, method, *args, remove_signature=remove_signature
+            proxy_interface, method, *args, unpack_variants=unpack_variants
         )
 
     with patch("supervisor.host.network.asyncio.sleep"), patch(

tests/utils/test_dbus.py

@@ -1,19 +0,0 @@
-"""Check dbus-next implementation."""
-from dbus_fast.signature import Variant
-
-from supervisor.utils.dbus import DBus
-
-
-def test_remove_dbus_signature():
-    """Check D-Bus signature clean-up."""
-    test = DBus.remove_dbus_signature(Variant("s", "Value"))
-    assert isinstance(test, str)
-    assert test == "Value"
-
-    test_dict = DBus.remove_dbus_signature({"Key": Variant("s", "Value")})
-    assert isinstance(test_dict["Key"], str)
-    assert test_dict["Key"] == "Value"
-
-    test_dict = DBus.remove_dbus_signature([Variant("s", "Value")])
-    assert isinstance(test_dict[0], str)
-    assert test_dict[0] == "Value"