raise validation error for all zero response data (#148)

* raise validation error for all zero response data

* Fix incorrect type annotation

---------

Co-authored-by: Kraus, Vadim <[email protected]>

solax/discovery.py

@@ -3,7 +3,7 @@
 import sys
 from asyncio import Future, Task
 from collections import defaultdict
-from typing import Dict, Literal, Sequence, Set, TypedDict, Union, cast
+from typing import Dict, Literal, Sequence, Set, Type, TypedDict, Union, cast
 
 from solax.inverter import Inverter
 from solax.inverter_http_client import InverterHttpClient
@@ -21,14 +21,18 @@
     from typing_extensions import Unpack
 
 # registry of inverters
-REGISTRY = {ep.load() for ep in entry_points(group="solax.inverter")}
+REGISTRY: Set[Type[Inverter]] = {
+    ep.load()
+    for ep in entry_points(group="solax.inverter")
+    if issubclass(ep.load(), Inverter)
+}
 
 logging.basicConfig(level=logging.INFO)
 
 
 class DiscoveryKeywords(TypedDict, total=False):
-    inverters: Sequence[Inverter]
-    return_when: Union[Literal["ALL_COMPLETED"], Literal["FIRST_COMPLETED"]]
+    inverters: Sequence[Type[Inverter]]
+    return_when: Literal["ALL_COMPLETED", "FIRST_COMPLETED"]
 
 
 if sys.version_info >= (3, 9):

solax/response_parser.py

@@ -9,7 +9,7 @@
 from voluptuous.humanize import humanize_error
 
 from solax.units import SensorUnit
-from solax.utils import PackerBuilderResult
+from solax.utils import PackerBuilderResult, contains_none_zero_value
 
 __all__ = ("ResponseParser", "InverterResponse", "ResponseDecoder")
 
@@ -39,6 +39,28 @@ def serial_number(self):
         return self.dongle_serial_number
 
 
+_KEY_DATA = "data"
+_KEY_SERIAL = "sn"
+_KEY_VERSION = "version"
+_KEY_VER = "ver"
+_KEY_TYPE = "type"
+
+
+GenericResponseSchema = vol.All(
+    vol.Schema({vol.Required(_KEY_SERIAL): str}, extra=vol.ALLOW_EXTRA),
+    vol.Any(
+        vol.Schema({vol.Required(_KEY_VERSION): str}, extra=vol.ALLOW_EXTRA),
+        vol.Schema({vol.Required(_KEY_VER): str}, extra=vol.ALLOW_EXTRA),
+    ),
+    vol.Schema(
+        {
+            vol.Required(_KEY_TYPE): vol.Any(int, str),
+            vol.Required(_KEY_DATA): vol.Schema(contains_none_zero_value),
+        },
+        extra=vol.ALLOW_EXTRA,
+    ),
+)
+
 ProcessorTuple = Tuple[Callable[[Any], Any], ...]
 SensorIndexSpec = Union[int, PackerBuilderResult]
 ResponseDecoder = Dict[
@@ -55,7 +77,7 @@ def __init__(
         dongle_serial_number_getter: Callable[[Dict[str, Any]], Optional[str]],
         inverter_serial_number_getter: Callable[[Dict[str, Any]], Optional[str]],
     ) -> None:
-        self.schema = schema
+        self.schema = vol.And(GenericResponseSchema, schema)
         self.response_decoder = decoder
         self.dongle_serial_number_getter = dongle_serial_number_getter
         self.inverter_serial_number_getter = inverter_serial_number_getter
@@ -115,9 +137,9 @@ def handle_response(self, resp: bytearray) -> InverterResponse:
             raise
 
         return InverterResponse(
-            data=self.map_response(response["data"]),
+            data=self.map_response(response[_KEY_DATA]),
             dongle_serial_number=self.dongle_serial_number_getter(response),
-            version=response.get("ver", response.get("version")),
-            type=response["type"],
+            version=response.get(_KEY_VER, response.get(_KEY_VERSION)),
+            type=response[_KEY_TYPE],
             inverter_serial_number=self.inverter_serial_number_getter(response),
         )

solax/utils.py

@@ -1,4 +1,5 @@
-from typing import Protocol, Tuple
+from numbers import Number
+from typing import List, Protocol, Tuple
 
 from voluptuous import Invalid
 
@@ -88,3 +89,17 @@ def twoway_div100(val):
 
 def to_url(host, port):
     return f"http://{host}:{port}/"
+
+
+def contains_none_zero_value(value: List[Number]):
+    """Validate that at least one element is not zero.
+    Args:
+        value (List[Number]): list to validate
+    Raises:
+        Invalid: if all elements are zero
+    """
+
+    if isinstance(value, list):
+        if len(value) != 0 and any((v != 0 for v in value)):
+            return value
+    raise Invalid("All elements in the list are zero")

tests/conftest.py

@@ -1,5 +1,6 @@
 # pylint: disable=unused-import
 from tests.fixtures import inverters_fixture  # noqa: F401
+from tests.fixtures import inverters_fixture_all_zero  # noqa: F401
 from tests.fixtures import inverters_garbage_fixture  # noqa: F401
 from tests.fixtures import inverters_under_test  # noqa: F401
 from tests.fixtures import simple_http_fixture  # noqa: F401

tests/fixtures.py

@@ -1,4 +1,5 @@
 from collections import namedtuple
+from copy import copy
 
 import pytest
 
@@ -306,3 +307,27 @@ def inverters_garbage_fixture(httpserver, request):
         query_string=request.param.query_string,
     ).respond_with_json({"bingo": "bango"})
     yield ((httpserver.host, httpserver.port), request.param.inverter)
+
+
+@pytest.fixture(params=INVERTERS_UNDER_TEST)
+def inverters_fixture_all_zero(httpserver, request):
+    """Use defined responses but replace the data with all zero values.
+    Testing incorrect responses.
+    """
+
+    response = request.param.response
+    response = copy(response)
+    response["Data"] = [0] * (len(response["Data"]))
+
+    httpserver.expect_request(
+        uri=request.param.uri,
+        method=request.param.method,
+        query_string=request.param.query_string,
+        headers=request.param.headers,
+        data=request.param.data,
+    ).respond_with_json(response)
+    yield (
+        (httpserver.host, httpserver.port),
+        request.param.inverter,
+        request.param.values,
+    )

tests/test_smoke.py

@@ -66,3 +66,18 @@ def test_inverter_sensors_define_valid_units(inverters_under_test):
             f"is not a proper Unit on sensor '{name}' of Inverter '{inverters_under_test}'"
         )
         assert isinstance(unit, Measurement), msg
+
+
+@pytest.mark.asyncio
+async def test_smoke_zero(inverters_fixture_all_zero):
+    """Responses with all zero values should be treated as an error.
+    Args:
+        inverters_fixture_all_zero (_type_): all responses with zero value data
+    """
+    conn, inverter_class, _ = inverters_fixture_all_zero
+
+    # msg = 'all zero values should be discarded'
+    with pytest.raises(InverterError):
+        inv = await build_right_variant(inverter_class, conn)
+        rt_api = solax.RealTimeAPI(inv)
+        await rt_api.get_data()

tests/test_vol.py

@@ -1,7 +1,7 @@
 import pytest
 from voluptuous import Invalid
 
-from solax.utils import startswith
+from solax.utils import contains_none_zero_value, startswith
 
 
 def test_does_start_with():
@@ -23,3 +23,23 @@ def test_is_not_str():
     actual = 1
     with pytest.raises(Invalid):
         startswith(expected)(actual)
+
+
+def test_contains_none_zero_value():
+    with pytest.raises(Invalid):
+        contains_none_zero_value([0])
+
+    with pytest.raises(Invalid):
+        contains_none_zero_value([0, 0])
+
+    not_a_list = 1
+    with pytest.raises(Invalid):
+        contains_none_zero_value(not_a_list)
+
+    expected = [1, 0]
+    actual = contains_none_zero_value(expected)
+    assert actual == expected
+
+    expected = [-1, 1]
+    actual = contains_none_zero_value(expected)
+    assert actual == expected