Support Multiple inverter api for inverter type (#96)

* Support Multiple inverter api for inverter type

The main problem is that inverters apis are different in two ways:

- The api end point to get the data
- Some aspects of parsing the non-"data" attributes of the response
  (like SN, version, etc...)

Each single inverter could expose (for example) two end points in
general. as in, before version x it exposes an end point with query
parameters, but after version x it exposes the end point with params in
the request body.

The inheritance model in the project is short handed
here. As it is based on the assumption that a single inverter will
always support a single api end point contract.

This patch is basically (and only for now), refactoring in favor of
composition (vs. inheritance).

Each inverter class is initiated with two dependencies/components:

- InverterHttpClient
- ResponseParser

To support multiple variants for each inverter, we only build the
inverter with proper variations of those dependencies.

So, After this patch, the discovery process is all about asking the
inverter class to build all possible variants this inverter knows it
supports. The discovery then goes on, as was before, trying to use that
variant to get_data, until one variant is successful.

The patch tries as much as possible to continue providing sane and most
common defaults for inveter's implementations.

At this moment, the patch does not add (or subtract) any feature/support
on top of the baseline (before the patch). This is just opening the
opportunity to easily add missing variants.

Closes #88

WIP

* Add a verify script to be run locally

This will be helpful for new contributors to check their code against
the github workflow checks without back and forth communication with the
maintainers to run the pipeline manually for them.

Co-authored-by: Robin Wohlers-Reichel <me@robinwr.com>

.gitignore

@@ -1,4 +1,5 @@
 /.vscode
+/.idea/**
 *.pyc
 /dist
 /solax.egg-info

solax/discovery.py

@@ -34,12 +34,12 @@ class DiscoveryError(Exception):
 async def discover(host, port, pwd="") -> Inverter:
     failures = []
     for inverter in REGISTRY:
-        i = inverter(host, port, pwd)
-        try:
-            await i.get_data()
-            return i
-        except InverterError as ex:
-            failures.append(ex)
+        for i in inverter.build_all_variants(host, port, pwd):
+            try:
+                await i.get_data()
+                return i
+            except InverterError as ex:
+                failures.append(ex)
     msg = (
         "Unable to connect to the inverter at "
         f"host={host} port={port}, or your inverter is not supported yet.\n"

solax/inverter.py

@@ -1,32 +1,18 @@
-import json
-from collections import namedtuple
-from typing import Any, Callable, Dict, Tuple, Union
+from typing import Dict, Tuple
 
 import aiohttp
 import voluptuous as vol
-from voluptuous import Invalid, MultipleInvalid
-from voluptuous.humanize import humanize_error
 
-from solax.units import Measurement, SensorUnit, Units
-from solax.utils import PackerBuilderResult
+from solax import utils
+from solax.inverter_http_client import InverterHttpClient, Method
+from solax.response_parser import InverterResponse, ResponseDecoder, ResponseParser
+from solax.units import Measurement, Units
 
 
 class InverterError(Exception):
     """Indicates error communicating with inverter"""
 
 
-InverterResponse = namedtuple("InverterResponse", "data, serial_number, version, type")
-
-SensorIndexSpec = Union[int, PackerBuilderResult]
-ResponseDecoder = Dict[
-    str,
-    Union[
-        Tuple[SensorIndexSpec, SensorUnit],
-        Tuple[SensorIndexSpec, SensorUnit, Callable[[Any], Any]],
-    ],
-]
-
-
 class Inverter:
     """Base wrapper around Inverter HTTP API"""
 
@@ -41,33 +27,53 @@ def response_decoder(cls) -> ResponseDecoder:
     # pylint: enable=C0301
     _schema = vol.Schema({})  # type: vol.Schema
 
-    def __init__(self, host, port, pwd=""):
-        self.host = host
-        self.port = port
-        self.pwd = pwd
+    def __init__(
+        self, http_client: InverterHttpClient, response_parser: ResponseParser
+    ):
         self.manufacturer = "Solax"
+        self.response_parser = response_parser
+        self.http_client = http_client
+
+    @classmethod
+    def _build(cls, host, port, pwd="", params_in_query=True):
+        url = utils.to_url(host, port)
+        http_client = InverterHttpClient(url, Method.POST, pwd)
+        if params_in_query:
+            http_client.with_default_query()
+        else:
+            http_client.with_default_data()
+
+        schema = cls.schema()
+        response_decoder = cls.response_decoder()
+        response_parser = ResponseParser(schema, response_decoder)
+        return cls(http_client, response_parser)
+
+    @classmethod
+    def build_all_variants(cls, host, port, pwd=""):
+        versions = [
+            cls._build(host, port, pwd, True),
+            cls._build(host, port, pwd, False),
+        ]
+        return versions
 
     async def get_data(self) -> InverterResponse:
         try:
-            data = await self.make_request(self.host, self.port, self.pwd)
+            data = await self.make_request()
         except aiohttp.ClientError as ex:
             msg = "Could not connect to inverter endpoint"
             raise InverterError(msg, str(self.__class__.__name__)) from ex
-        except ValueError as ex:
-            msg = "Received non-JSON data from inverter endpoint"
-            raise InverterError(msg, str(self.__class__.__name__)) from ex
         except vol.Invalid as ex:
             msg = "Received malformed JSON from inverter"
             raise InverterError(msg, str(self.__class__.__name__)) from ex
         return data
 
-    @classmethod
-    async def make_request(cls, host, port, pwd="", headers=None) -> InverterResponse:
+    async def make_request(self) -> InverterResponse:
         """
         Return instance of 'InverterResponse'
         Raise exception if unable to get data
         """
-        raise NotImplementedError()
+        raw_response = await self.http_client.request()
+        return self.response_parser.handle_response(raw_response)
 
     @classmethod
     def sensor_map(cls) -> Dict[str, Tuple[int, Measurement]]:
@@ -92,113 +98,9 @@ def sensor_map(cls) -> Dict[str, Tuple[int, Measurement]]:
             sensors[name] = (idx, unit)
         return sensors
 
-    @classmethod
-    def _decode_map(cls) -> Dict[str, SensorIndexSpec]:
-        sensors: Dict[str, SensorIndexSpec] = {}
-        for name, mapping in cls.response_decoder().items():
-            sensors[name] = mapping[0]
-        return sensors
-
-    @classmethod
-    def _postprocess_map(cls) -> Dict[str, Callable[[Any], Any]]:
-        """
-        Return map of functions to be applied to each sensor value
-        """
-        sensors: Dict[str, Callable[[Any], Any]] = {}
-        for name, mapping in cls.response_decoder().items():
-            processor = None
-            (_, _, *processor) = mapping
-            if processor:
-                sensors[name] = processor[0]
-        return sensors
-
     @classmethod
     def schema(cls) -> vol.Schema:
         """
         Return schema
         """
         return cls._schema
-
-    @classmethod
-    def map_response(cls, resp_data) -> Dict[str, Any]:
-        result = {}
-        for sensor_name, decode_info in cls._decode_map().items():
-            if isinstance(decode_info, (tuple, list)):
-                indexes = decode_info[0]
-                packer = decode_info[1]
-                values = tuple(resp_data[i] for i in indexes)
-                val = packer(*values)
-            else:
-                val = resp_data[decode_info]
-            result[sensor_name] = val
-        for sensor_name, processor in cls._postprocess_map().items():
-            result[sensor_name] = processor(result[sensor_name])
-        return result
-
-
-class InverterPost(Inverter):
-    # This is an intermediate abstract class,
-    #  so we can disable the pylint warning
-    # pylint: disable=W0223,R0914
-    @classmethod
-    async def make_request(cls, host, port=80, pwd="", headers=None):
-        if not pwd:
-            base = "http://{}:{}/?optType=ReadRealTimeData"
-            url = base.format(host, port)
-        else:
-            base = "http://{}:{}/?optType=ReadRealTimeData&pwd={}&"
-            url = base.format(host, port, pwd)
-        async with aiohttp.ClientSession() as session:
-            async with session.post(url, headers=headers) as req:
-                req.raise_for_status()
-                resp = await req.read()
-
-        return cls.handle_response(resp)
-
-    @classmethod
-    def handle_response(cls, resp: bytearray):
-        """
-        Decode response and map array result using mapping definition.
-
-        Args:
-            resp (bytearray): The response
-
-        Returns:
-            InverterResponse: The decoded and mapped interver response.
-        """
-
-        raw_json = resp.decode("utf-8")
-        json_response = json.loads(raw_json)
-        response = {}
-        try:
-            response = cls.schema()(json_response)
-        except (Invalid, MultipleInvalid) as ex:
-            _ = humanize_error(json_response, ex)
-            raise
-        return InverterResponse(
-            data=cls.map_response(response["Data"]),
-            serial_number=response.get("SN", response.get("sn")),
-            version=response["ver"],
-            type=response["type"],
-        )
-
-
-class InverterPostData(InverterPost):
-    # This is an intermediate abstract class,
-    #  so we can disable the pylint warning
-    # pylint: disable=W0223,R0914
-    @classmethod
-    async def make_request(cls, host, port=80, pwd="", headers=None):
-        base = "http://{}:{}/"
-        url = base.format(host, port)
-        data = "optType=ReadRealTimeData"
-        if pwd:
-            data = data + "&pwd=" + pwd
-        async with aiohttp.ClientSession() as session:
-            async with session.post(
-                url, headers=headers, data=data.encode("utf-8")
-            ) as req:
-                req.raise_for_status()
-                resp = await req.read()
-
-        return cls.handle_response(resp)

solax/inverter_http_client.py

@@ -0,0 +1,73 @@
+from enum import Enum
+
+import aiohttp
+
+
+class Method(Enum):
+    GET = 1
+    POST = 2
+
+
+class InverterHttpClient:
+    def __init__(self, url, method: Method = Method.POST, pwd=""):
+        """Initialize the Http client."""
+        self.url = url
+        self.method = method
+        self.pwd = pwd
+        self.headers = None
+        self.data = None
+        self.query = ""
+
+    @classmethod
+    def build_w_url(cls, url, method: Method = Method.POST):
+        http_client = cls(url, method, "")
+        return http_client
+
+    def with_headers(self, headers):
+        self.headers = headers
+        return self
+
+    def with_default_data(self):
+        data = "optType=ReadRealTimeData"
+        if self.pwd:
+            data = data + "&pwd=" + self.pwd
+        return self.with_data(data)
+
+    def with_data(self, data):
+        self.data = data
+        return self
+
+    def with_query(self, query):
+        self.query = query
+        return self
+
+    def with_default_query(self):
+        if self.pwd:
+            base = "optType=ReadRealTimeData&pwd={}&"
+            query = base.format(self.pwd)
+        else:
+            query = "optType=ReadRealTimeData"
+
+        return self.with_query(query)
+
+    async def request(self):
+        if self.method is Method.POST:
+            return await self.post()
+        return await self.get()
+
+    async def get(self):
+        url = self.url + "?" + self.query if self.query else self.url
+        async with aiohttp.ClientSession() as session:
+            async with session.get(url, headers=self.headers) as req:
+                req.raise_for_status()
+                resp = await req.read()
+        return resp
+
+    async def post(self):
+        url = self.url + "?" + self.query if self.query else self.url
+        data = self.data.encode("utf-8") if self.data else None
+        async with aiohttp.ClientSession() as session:
+            async with session.post(url, headers=self.headers, data=data) as req:
+                req.raise_for_status()
+                resp = await req.read()
+        return resp

solax/inverters/qvolt_hyb_g3_3p.py

@@ -1,12 +1,12 @@
-import aiohttp
 import voluptuous as vol
 
-from solax.inverter import InverterPost
+from solax import utils
+from solax.inverter import Inverter, InverterHttpClient, Method, ResponseParser
 from solax.units import Total, Units
 from solax.utils import div10, div100, pack_u16, to_signed, twoway_div10, twoway_div100
 
 
-class QVOLTHYBG33P(InverterPost):
+class QVOLTHYBG33P(Inverter):
     """
     QCells
     Q.VOLT HYB-G3-3P
@@ -42,8 +42,10 @@ def battery_modes(value):
                 3: "Feed-in Priority",
             }.get(value, f"unmapped value '{value}'")
 
-    def __init__(self, host, port, pwd=""):
-        super().__init__(host, port, pwd)
+    def __init__(
+        self, http_client: InverterHttpClient, response_parser: ResponseParser
+    ):
+        super().__init__(http_client, response_parser)
         self.manufacturer = "Qcells"
 
     _schema = vol.Schema(
@@ -159,13 +161,16 @@ def response_decoder(cls):
         }
 
     @classmethod
-    async def make_request(cls, host, port=80, pwd="", headers=None):
+    def _build(cls, host, port, pwd="", params_in_query=True):
+        url = utils.to_url(host, port)
+        http_client = InverterHttpClient(url, Method.POST, pwd).with_default_data()
 
-        url = f"http://{host}:{port}/"
-        data = f"optType=ReadRealTimeData&pwd={pwd}"
+        schema = cls._schema
+        response_decoder = cls.response_decoder()
+        response_parser = ResponseParser(schema, response_decoder)
+        return cls(http_client, response_parser)
 
-        async with aiohttp.ClientSession() as session:
-            async with session.post(url, headers=headers, data=data) as req:
-                resp = await req.read()
-
-        return cls.handle_response(resp)
+    @classmethod
+    def build_all_variants(cls, host, port, pwd=""):
+        versions = [cls._build(host, port, pwd)]
+        return versions

solax/inverters/x1.py

@@ -1,11 +1,11 @@
 import voluptuous as vol
 
-from solax.inverter import InverterPost
+from solax.inverter import Inverter
 from solax.units import Total, Units
 from solax.utils import startswith
 
 
-class X1(InverterPost):
+class X1(Inverter):
     # pylint: disable=duplicate-code
     _schema = vol.Schema(
         {