diff --git a/matter_server/common/helpers/util.py b/matter_server/common/helpers/util.py index 9cf1a4ec..1786bcce 100644 --- a/matter_server/common/helpers/util.py +++ b/matter_server/common/helpers/util.py @@ -107,7 +107,7 @@ def parse_value( value: Any, value_type: Any, default: Any = MISSING, - allow_none: bool = True, + allow_none: bool = False, allow_sdk_types: bool = False, ) -> Any: """ @@ -121,6 +121,16 @@ def parse_value( # this shouldn't happen, but just in case value_type = get_type_hints(value_type, globals(), locals()) + # handle value is None/missing but a default value is set + if value is None and not isinstance(default, type(MISSING)): + return default + # handle value is None and sdk type is Nullable + if value is None and value_type is Nullable: + return Nullable() if allow_sdk_types else None + # handle value is None (but that is allowed according to the annotations) + if value is None and value_type is NoneType: + return None + if isinstance(value, dict): if descriptor := getattr(value_type, "descriptor", None): # handle matter TLV dicts where the keys are just tag identifiers @@ -132,14 +142,6 @@ def parse_value( return None value = None - if value is None and not isinstance(default, type(MISSING)): - return default - if value is None and value_type is NoneType: - return None - if value is None and value_type is Nullable: - return Nullable() if allow_sdk_types else None - if value is None and allow_none: - return None if is_dataclass(value_type) and isinstance(value, dict): return dataclass_from_dict(value_type, value) # get origin value type and inspect one-by-one @@ -156,7 +158,11 @@ def parse_value( subvalue_type = get_args(value_type)[1] return { parse_value(subkey, subkey, subkey_type): parse_value( - f"{subkey}.value", subvalue, subvalue_type + f"{subkey}.value", + subvalue, + subvalue_type, + allow_none=allow_none, + allow_sdk_types=allow_sdk_types, ) for subkey, subvalue in value.items() } @@ -169,7 +175,13 @@ def parse_value( return value # try them all until one succeeds try: - return parse_value(name, value, sub_arg_type) + return parse_value( + name, + value, + sub_arg_type, + allow_none=allow_none, + allow_sdk_types=allow_sdk_types, + ) except (KeyError, TypeError, ValueError): pass # if we get to this point, all possibilities failed @@ -189,8 +201,11 @@ def parse_value( # handle Any as value type (which is basically unprocessable) if value_type is Any: return value + # handle value is None (but that is allowed) + if value is None and allow_none: + return None # raise if value is None and the value is required according to annotations - if value is None and value_type is not NoneType and not allow_none: + if value is None: raise KeyError(f"`{name}` of type `{value_type}` is required.") try: diff --git a/matter_server/server/device_controller.py b/matter_server/server/device_controller.py index bdb06fec..d8b0d5fa 100644 --- a/matter_server/server/device_controller.py +++ b/matter_server/server/device_controller.py @@ -695,6 +695,7 @@ async def write_attribute( name=attribute_path, value=value, value_type=attribute.attribute_type.Type, + allow_none=False, allow_sdk_types=True, ) if node_id >= TEST_NODE_START: diff --git a/tests/common/test_parser.py b/tests/common/test_parser.py index 41f9a44e..e7609695 100644 --- a/tests/common/test_parser.py +++ b/tests/common/test_parser.py @@ -3,8 +3,9 @@ from dataclasses import dataclass import datetime from enum import Enum, IntEnum -from typing import Optional +from typing import Optional, Union +from chip.clusters.Types import Nullable, NullValue import pytest from matter_server.common.helpers.util import dataclass_from_dict, parse_value @@ -110,3 +111,21 @@ def test_dataclass_from_dict(): # test NOCStruct.noc edge case res = parse_value("NOCStruct.noc", 5, bytes) assert res == b"" + + +def test_parse_value(): + """Test special cases in the parse_value helper.""" + # test None value which is allowed + assert parse_value("test", None, int, allow_none=True) is None + # test unexpected None value + with pytest.raises(KeyError): + parse_value("test", None, int, allow_none=False) + # test sdk Nullable type + assert parse_value("test", None, Nullable) is None + assert parse_value("test", None, Nullable, allow_sdk_types=True) == NullValue + assert ( + parse_value( + "test", None, Union[int, Nullable], allow_none=False, allow_sdk_types=True + ) + == NullValue + )