Skip to content

Commit 81cad48

Browse files
jfmlimajfmlima
authored
Handle null sys.status values from old firmware (#95)
Gen2 devices whose clock never synced report "unixtime": null in sys.status. SystemComponent.from_raw_data passed that None to a non-optional int field, Pydantic raised, the gateway fell back to the Gen1 path, and the device became a 404 on the detail page. Local firmware updates failed on the same read. Null-guard the numeric and dict fields in SystemComponent the way InputComponent already does, and make ComponentFactory fall back to the base Component with a warning when a typed model rejects its data, so one malformed component can no longer hide a reachable device. Closes #88
1 parent b55f917 commit 81cad48

4 files changed

Lines changed: 320 additions & 9 deletions

File tree

‎packages/core/src/core/domain/entities/components/factory.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,29 @@
1+
import logging
12
from typing import Any
23

4+
from pydantic import ValidationError
5+
36
from .base import Component
47
from .registry import model_for
58

9+
logger = logging.getLogger(__name__)
10+
611

712
class ComponentFactory:
813
@staticmethod
914
def create_component(component_data: dict[str, Any]) -> Component:
1015
key = component_data.get("key", "")
1116
component_type = key.split(":")[0] if ":" in key else key
1217

13-
return model_for(component_type).from_raw_data(component_data)
18+
try:
19+
return model_for(component_type).from_raw_data(component_data)
20+
except ValidationError as e:
21+
logger.warning(
22+
"Component %s did not match its typed model, keeping raw data: %s",
23+
key,
24+
e,
25+
)
26+
return Component.from_raw_data(component_data)
1427

1528
@staticmethod
1629
def create_component_from_status(

‎packages/core/src/core/domain/entities/components/system.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ def from_raw_data(cls, component_data: dict[str, Any]) -> "SystemComponent":
3535
device_name=device_config.get("name"),
3636
mac_address=status.get("mac"),
3737
firmware_version=device_config.get("fw_id"),
38-
uptime=status.get("uptime", 0),
39-
restart_required=status.get("restart_required", False),
40-
ram_total=status.get("ram_size", 0),
41-
ram_free=status.get("ram_free", 0),
42-
fs_total=status.get("fs_size", 0),
43-
fs_free=status.get("fs_free", 0),
44-
available_updates=status.get("available_updates", {}),
45-
unixtime=status.get("unixtime", 0),
38+
uptime=status.get("uptime", 0) or 0,
39+
restart_required=status.get("restart_required", False) or False,
40+
ram_total=status.get("ram_size", 0) or 0,
41+
ram_free=status.get("ram_free", 0) or 0,
42+
fs_total=status.get("fs_size", 0) or 0,
43+
fs_free=status.get("fs_free", 0) or 0,
44+
available_updates=status.get("available_updates", {}) or {},
45+
unixtime=status.get("unixtime", 0) or 0,
4646
timezone=location_config.get("tz"),
4747
)

‎packages/core/tests/unit/domain/entities/test_components.py‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
EMDataComponent,
88
InputComponent,
99
SwitchComponent,
10+
SystemComponent,
1011
WebSocketComponent,
1112
WifiComponent,
1213
ZigbeeComponent,
@@ -856,3 +857,100 @@ def test_it_creates_em_component_from_status_only(self):
856857
assert isinstance(component, EMComponent)
857858
assert component.total_act_power == 500.0
858859
assert component.a_act_power == 200.0
860+
861+
862+
class TestSystemComponent:
863+
def test_it_parses_sys_status_with_null_time_fields(self):
864+
raw_data = {
865+
"key": "sys",
866+
"status": {
867+
"mac": "E86BEAE5F208",
868+
"restart_required": False,
869+
"time": None,
870+
"unixtime": None,
871+
"uptime": 3539695,
872+
"ram_size": 252308,
873+
"ram_free": 132132,
874+
"fs_size": 393216,
875+
"fs_free": 98304,
876+
"cfg_rev": 11,
877+
"kvs_rev": 0,
878+
"schedule_rev": 1,
879+
"webhook_rev": 1,
880+
"available_updates": {},
881+
"reset_reason": 1,
882+
},
883+
"config": {
884+
"device": {
885+
"name": "Flur SZ Rollo links",
886+
"mac": "xxxx",
887+
"fw_id": "20240726-114505/1.4.0-gb2aeadb",
888+
"discoverable": True,
889+
"eco_mode": False,
890+
"profile": "cover",
891+
"addon_type": None,
892+
},
893+
"location": {"tz": None, "lat": None, "lon": None},
894+
"sntp": {"server": "xxxx"},
895+
"cfg_rev": 11,
896+
},
897+
"attrs": {},
898+
}
899+
900+
component = SystemComponent.from_raw_data(raw_data)
901+
902+
assert component.unixtime == 0
903+
assert component.uptime == 3539695
904+
assert component.mac_address == "E86BEAE5F208"
905+
assert component.device_name == "Flur SZ Rollo links"
906+
assert component.firmware_version == "20240726-114505/1.4.0-gb2aeadb"
907+
assert component.timezone is None
908+
909+
def test_it_parses_sys_status_with_nulls_in_every_guarded_field(self):
910+
raw_data = {
911+
"key": "sys",
912+
"status": {
913+
"restart_required": None,
914+
"unixtime": None,
915+
"uptime": None,
916+
"ram_size": None,
917+
"ram_free": None,
918+
"fs_size": None,
919+
"fs_free": None,
920+
"available_updates": None,
921+
},
922+
"config": {},
923+
"attrs": {},
924+
}
925+
926+
component = SystemComponent.from_raw_data(raw_data)
927+
928+
assert component.unixtime == 0
929+
assert component.uptime == 0
930+
assert component.restart_required is False
931+
assert component.ram_total == 0
932+
assert component.ram_free == 0
933+
assert component.fs_total == 0
934+
assert component.fs_free == 0
935+
assert component.available_updates == {}
936+
937+
938+
class TestComponentFactoryFallback:
939+
def test_it_falls_back_to_base_component_when_typed_model_rejects_data(
940+
self, caplog
941+
):
942+
raw_data = {
943+
"key": "sys",
944+
"status": {"uptime": "not-a-number"},
945+
"config": {},
946+
"attrs": {},
947+
}
948+
949+
with caplog.at_level("WARNING"):
950+
component = ComponentFactory.create_component(raw_data)
951+
952+
assert type(component) is Component
953+
assert component.key == "sys"
954+
assert component.component_type == "sys"
955+
assert component.status == {"uptime": "not-a-number"}
956+
assert any("sys" in record.message for record in caplog.records)

‎packages/core/tests/unit/domain/entities/test_device_status.py‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,3 +775,203 @@ def test_it_creates_em_components_from_a_raw_response(self):
775775

776776
summary = device_status.get_device_summary()
777777
assert summary["total_power"] == 312.278
778+
779+
780+
class TestDeviceStatusOldFirmwarePayload:
781+
"""Regression for GitHub issue 88: a 1.4.0 device with an unsynced clock."""
782+
783+
def test_it_builds_device_status_from_a_capture_with_null_sys_time(self):
784+
device_info_data = {
785+
"name": "Flur SZ Rollo links",
786+
"id": "shellyplus2pm-e86beae5f208",
787+
"mac": "XXXXX",
788+
"slot": 1,
789+
"model": "SNSW-102P16EU",
790+
"gen": 2,
791+
"fw_id": "20240726-114505/1.4.0-gb2aeadb",
792+
"ver": "1.4.0",
793+
"app": "Plus2PM",
794+
"auth_en": False,
795+
"auth_domain": None,
796+
"profile": "cover",
797+
}
798+
response_data = {
799+
"components": [
800+
{
801+
"key": "ble",
802+
"status": {},
803+
"config": {
804+
"enable": False,
805+
"rpc": {"enable": False},
806+
"observer": {"enable": False},
807+
},
808+
},
809+
{
810+
"key": "cloud",
811+
"status": {"connected": False},
812+
"config": {
813+
"enable": False,
814+
"server": "iot.shelly.cloud:6012/jrpc",
815+
},
816+
},
817+
{
818+
"key": "cover:0",
819+
"status": {
820+
"id": 0,
821+
"source": "timeout",
822+
"state": "stopped",
823+
"apower": 0.0,
824+
"voltage": 234.7,
825+
"current": 0.0,
826+
"pf": 0.0,
827+
"freq": 50.0,
828+
"aenergy": {"total": 966.07},
829+
"temperature": {"tC": 58.1, "tF": 136.6},
830+
"pos_control": True,
831+
"last_direction": "open",
832+
"current_pos": 50,
833+
},
834+
"config": {
835+
"id": 0,
836+
"name": None,
837+
"motor": {"idle_power_thr": 2.0, "idle_confirm_period": 0.25},
838+
"maxtime_open": 60.0,
839+
"maxtime_close": 60.0,
840+
"initial_state": "stopped",
841+
"invert_directions": True,
842+
"in_mode": "detached",
843+
"swap_inputs": False,
844+
"safety_switch": {
845+
"enable": False,
846+
"direction": "both",
847+
"action": "stop",
848+
"allowed_move": None,
849+
},
850+
"power_limit": 2800,
851+
"voltage_limit": 280,
852+
"undervoltage_limit": 0,
853+
"current_limit": 10.0,
854+
"obstruction_detection": {
855+
"enable": False,
856+
"direction": "both",
857+
"action": "stop",
858+
"power_thr": 113,
859+
"holdoff": 1.0,
860+
},
861+
},
862+
},
863+
{
864+
"key": "input:0",
865+
"status": {"id": 0, "state": None},
866+
"config": {
867+
"id": 0,
868+
"name": None,
869+
"type": "button",
870+
"enable": True,
871+
"invert": False,
872+
"factory_reset": True,
873+
},
874+
},
875+
{
876+
"key": "input:1",
877+
"status": {"id": 1, "state": None},
878+
"config": {
879+
"id": 1,
880+
"name": None,
881+
"type": "button",
882+
"enable": True,
883+
"invert": False,
884+
"factory_reset": True,
885+
},
886+
},
887+
{
888+
"key": "mqtt",
889+
"status": {"connected": False},
890+
"config": {
891+
"enable": False,
892+
"server": None,
893+
"client_id": "shellyplus2pm-e86beae5f208",
894+
"user": None,
895+
"ssl_ca": None,
896+
"topic_prefix": "shellyplus2pm-e86beae5f208",
897+
"rpc_ntf": True,
898+
"status_ntf": False,
899+
"use_client_cert": False,
900+
"enable_rpc": True,
901+
"enable_control": True,
902+
},
903+
},
904+
{
905+
"key": "sys",
906+
"status": {
907+
"mac": "E86BEAE5F208",
908+
"restart_required": False,
909+
"time": None,
910+
"unixtime": None,
911+
"uptime": 3539695,
912+
"ram_size": 252308,
913+
"ram_free": 132132,
914+
"fs_size": 393216,
915+
"fs_free": 98304,
916+
"cfg_rev": 11,
917+
"kvs_rev": 0,
918+
"schedule_rev": 1,
919+
"webhook_rev": 1,
920+
"available_updates": {},
921+
"reset_reason": 1,
922+
},
923+
"config": {
924+
"device": {
925+
"name": "Flur SZ Rollo links",
926+
"mac": "xxxx",
927+
"fw_id": "20240726-114505/1.4.0-gb2aeadb",
928+
"discoverable": True,
929+
"eco_mode": False,
930+
"profile": "cover",
931+
"addon_type": None,
932+
},
933+
"location": {"tz": None, "lat": None, "lon": None},
934+
"debug": {
935+
"level": 2,
936+
"file_level": None,
937+
"mqtt": {"enable": False},
938+
"websocket": {"enable": False},
939+
"udp": {"addr": None},
940+
},
941+
"ui_data": {"cover": ""},
942+
"rpc_udp": {"dst_addr": None, "listen_port": None},
943+
"sntp": {"server": "xxxx"},
944+
"cfg_rev": 11,
945+
},
946+
},
947+
],
948+
"cfg_rev": 11,
949+
"offset": 0,
950+
"total": 9,
951+
}
952+
status_data = {
953+
"wifi": {
954+
"sta_ip": "192.xxxx.22",
955+
"status": "got ip",
956+
"ssid": "xxxxIoT",
957+
"rssi": -41,
958+
},
959+
"ws": {"connected": False},
960+
}
961+
962+
device_status = DeviceStatus.from_raw_response(
963+
"192.168.1.22",
964+
response_data,
965+
available_methods=["Sys.GetStatus", "Cover.Open", "Cover.Close"],
966+
device_info_data=device_info_data,
967+
status_data=status_data,
968+
)
969+
970+
sys_info = device_status.get_system_info()
971+
assert sys_info is not None
972+
assert sys_info.unixtime == 0
973+
assert sys_info.uptime == 3539695
974+
assert len(device_status.get_covers()) == 1
975+
assert len(device_status.get_inputs()) == 2
976+
assert device_status.gen == 2
977+
assert device_status.get_device_summary()["wifi_connected"] is True

0 commit comments

Comments
 (0)