Skip to content

Commit

Permalink
Fix double converting config (zigpy#1433)
Browse files Browse the repository at this point in the history
* Do not double convert the config schema in unit tests

* Use the processed schema when setting up OTA

* Test config with just a legacy OTA provider

* Revert "Test config with just a legacy OTA provider"

This reverts commit 5076450.

* Do not double convert in `probe`

* Remove validator branches to deal with double conversion

* Fix unit tests using double conversion

* Remove all config mutation methods
  • Loading branch information
puddly authored Aug 6, 2024
1 parent 925a112 commit 50d92d1
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 73 deletions.
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def make_app(
config_updates,
)

app = app_base(app_base.SCHEMA(config))
app = app_base(config)
app.state.node_info = app_state.NodeInfo(
nwk=t.NWK(0x0000), ieee=NCP_IEEE, logical_type=zdo_t.LogicalType.Coordinator
)
Expand Down
44 changes: 0 additions & 44 deletions tests/test_application.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import asyncio
import copy
import errno
import logging
from unittest import mock
from unittest.mock import ANY, PropertyMock, call

import pytest
import voluptuous as vol

import zigpy.application
import zigpy.config as conf
Expand Down Expand Up @@ -334,48 +332,6 @@ def test_props(app):
assert app.state.network_info.nwk_update_id is not None


def test_app_config_setter(app):
"""Test configuration setter."""

cfg_copy = copy.deepcopy(app.config)
assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is False
cfg_copy[conf.CONF_OTA][conf.CONF_OTA_ENABLED] = "invalid bool"

with pytest.raises(vol.Invalid):
app.config = cfg_copy

assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is False

cfg_copy[conf.CONF_OTA][conf.CONF_OTA_ENABLED] = True
app.config = cfg_copy
assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is True

cfg_copy[conf.CONF_OTA][conf.CONF_OTA_ENABLED] = "invalid bool"

with pytest.raises(vol.Invalid):
app.config = cfg_copy

assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is True


def test_app_update_config(app):
"""Test configuration partial update."""

assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is False
with pytest.raises(vol.Invalid):
app.update_config({conf.CONF_OTA: {conf.CONF_OTA_ENABLED: "invalid bool"}})

assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is False

app.update_config({conf.CONF_OTA: {conf.CONF_OTA_ENABLED: "yes"}})
assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is True

with pytest.raises(vol.Invalid):
app.update_config({conf.CONF_OTA: {conf.CONF_OTA_ENABLED: "invalid bool"}})

assert app.config[conf.CONF_OTA][conf.CONF_OTA_ENABLED] is True


async def test_uninitialized_message_handlers(app, ieee):
"""Test uninitialized message handlers."""
handler_1 = MagicMock(return_value=None)
Expand Down
10 changes: 4 additions & 6 deletions tests/test_topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@ def remove_request_delay():
@pytest.fixture()
def topology(make_initialized_device):
app = App(
conf.ZIGPY_SCHEMA(
{
conf.CONF_DEVICE: {conf.CONF_DEVICE_PATH: "/dev/null"},
conf.CONF_TOPO_SKIP_COORDINATOR: True,
}
)
{
conf.CONF_DEVICE: {conf.CONF_DEVICE_PATH: "/dev/null"},
conf.CONF_TOPO_SKIP_COORDINATOR: True,
}
)

coordinator = make_initialized_device(app)
Expand Down
13 changes: 2 additions & 11 deletions zigpy/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def __init__(self, config: dict) -> None:
self._config[conf.CONF_MAX_CONCURRENT_REQUESTS]
)

self.ota = zigpy.ota.OTA(config[conf.CONF_OTA], self)
self.ota = zigpy.ota.OTA(self._config[conf.CONF_OTA], self)
self.backups: zigpy.backups.BackupManager = zigpy.backups.BackupManager(self)
self.topology: zigpy.topology.Topology = zigpy.topology.Topology(self)

Expand Down Expand Up @@ -625,7 +625,7 @@ async def probe(cls, device_config: dict[str, Any]) -> bool | dict[str, Any]:
device_configs.append(new_config)

for config in device_configs:
app = cls(cls.SCHEMA({conf.CONF_DEVICE: config}))
app = cls({conf.CONF_DEVICE: config})

try:
await app.connect()
Expand Down Expand Up @@ -1299,20 +1299,11 @@ def get_dst_address(self, cluster: zigpy.zcl.Cluster) -> zdo_types.MultiAddress:
dstaddr.endpoint = self.get_endpoint_id(cluster.cluster_id, cluster.is_server)
return dstaddr

def update_config(self, partial_config: dict[str, Any]) -> None:
"""Update existing config."""
self.config = {**self.config, **partial_config}

@property
def config(self) -> dict:
"""Return current configuration."""
return self._config

@config.setter
def config(self, new_config) -> None:
"""Configuration setter."""
self._config = self.SCHEMA(new_config)

@property
def groups(self) -> zigpy.group.Groups:
return self._groups
Expand Down
13 changes: 2 additions & 11 deletions zigpy/config/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ def cv_key(key: list[int]) -> t.KeyData:

def cv_simple_descriptor(obj: dict[str, typing.Any]) -> zdo_t.SimpleDescriptor:
"""Validates a ZDO simple descriptor."""
if isinstance(obj, zdo_t.SimpleDescriptor):
return obj
elif not isinstance(obj, dict):
if not isinstance(obj, dict):
raise vol.Invalid("Not a dictionary")

descriptor = zdo_t.SimpleDescriptor(**obj)
Expand Down Expand Up @@ -135,15 +133,8 @@ def cv_ota_provider_name(name: str | None) -> type[zigpy.ota.providers.BaseOtaPr
return zigpy.ota.providers.OTA_PROVIDER_TYPES[name]


def cv_ota_provider(
obj: dict | zigpy.ota.providers.BaseOtaProvider,
) -> zigpy.ota.providers.BaseOtaProvider:
def cv_ota_provider(obj: dict) -> zigpy.ota.providers.BaseOtaProvider:
"""Validate OTA provider."""
import zigpy.ota.providers

if isinstance(obj, zigpy.ota.providers.BaseOtaProvider):
return obj

provider_type = obj.get(zigpy.config.CONF_OTA_PROVIDER_TYPE)
provider_cls = cv_ota_provider_name(provider_type)

Expand Down

0 comments on commit 50d92d1

Please # to comment.