fsantini / python-e3dc

Python API for querying E3/DC systems through the manufacturer's portal
MIT License
71 stars 23 forks source link

add type hints and static type checking #76

Closed vchrisb closed 8 months ago

vchrisb commented 1 year ago

Type hinting is now available in all supported Python versions.

This should help further improve code quality and make it more readable

vchrisb commented 1 year ago

@torbennehmer would you mind trying this PR out? You're currently actively working with it. 🥇

torbennehmer commented 1 year ago

@vchrisb Of course, will do, but it'll probably take until Sunday to get around to it.

torbennehmer commented 1 year ago

@vchrisb I came around to give this a try, most things work, however there are at least to places, where I've got regressions. I didn't have time to analyse it deeper (I'm preparing for a business-trip, a large software rollout next week). I can look deeper into it if you need it (just say) afterwards. The problems I saw:

print(json.dumps(e3dc.poll_switches(), indent=2, default=str))

Traceback (most recent call last):
  File "/home/torben/src/python-e3dc/test.py", line 18, in <module>
    print(json.dumps(e3dc.poll_switches(), indent=2, default=str))
                     ^^^^^^^^^^^^^^^^^^^^
  File "/home/torben/src/python-e3dc/e3dc/_e3dc.py", line 448, in poll_switches
    descList = switchDesc[2]  # get the payload of the container
               ~~~~~~~~~~^^^
IndexError: list index out of range

The standard branch returns an empty list here ([]).

Similarily, this fails too:

print(json.dumps(e3dc.get_pvis_data(), indent=2, default=str))

  File "/home/torben/src/python-e3dc/test.py", line 19, in <module>
    print(json.dumps(e3dc.get_pvis_data(), indent=2, default=str))
                     ^^^^^^^^^^^^^^^^^^^^
  File "/home/torben/src/python-e3dc/e3dc/_e3dc.py", line 1789, in get_pvis_data
    self.get_pvi_data(
  File "/home/torben/src/python-e3dc/e3dc/_e3dc.py", line 1577, in get_pvi_data
    voltageMonitoring = rscpFindTag(req, "PVI_VOLTAGE_MONITORING")
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/torben/src/python-e3dc/e3dc/_rscpLib.py", line 66, in rscpFindTag
    raise KeyError("There is no Tag named {} in the message.".format(tag))
KeyError: 'There is no Tag named PVI_VOLTAGE_MONITORING in the message.'

The standard code lists this:

[
  {
    "acMaxApparentPower": 4000.0,
    "cosPhi": {
      "active": null,
      "value": null,
      "excited": null
    },
    "deviceState": {
      "connected": true,
      "working": true,
      "inService": false
    },
    "frequency": {
      "under": null,
      "over": null
    },
    "index": 0,
    "lastError": "3 0x0",
    "maxPhaseCount": 3,
    "maxStringCount": 2,
    "onGrid": true,
    "phases": {
      "0": {
        "power": 1321.0,
        "voltage": 231.1,
        "current": 5.71,
        "apparentPower": 1324.0,
        "reactivePower": 0.0,
        "energyAll": 22787022.0,
        "energyGridConsumption": 22257.0
      },
      "1": {
        "power": 0.0,
        "voltage": 231.7,
        "current": 0.0,
        "apparentPower": 0.0,
        "reactivePower": 0.0,
        "energyAll": 8551523.0,
        "energyGridConsumption": 850.0
      },
      "2": {
        "power": 0.0,
        "voltage": 234.2,
        "current": 0.0,
        "apparentPower": 0.0,
        "reactivePower": 0.0,
        "energyAll": 8674676.0,
        "energyGridConsumption": 953.0
      }
    },
    "powerMode": 1,
    "serialNumber": "E3<redacted>66",
    "state": "0xb32715",
    "strings": {
      "0": {
        "power": 22.0,
        "voltage": 244.0,
        "current": 0.09,
        "energyAll": 45005419.0
      }
    },
    "systemMode": 2,
    "temperature": {
      "max": 130.0,
      "min": -30.0,
      "values": [
        28.9,
        29.3,
        32.6,
        32.6
      ]
    },
    "type": 3,
    "version": " MAIN HW06 2.048",
    "voltageMonitoring": {
      "thresholdTop": null,
      "thresholdBottom": null,
      "slopeUp": null,
      "slopeDown": null
    }
  }
]

I have not yet tested (e.g. directly called):

In addition:

The two known bugs above are no direct problem, as soon as I give my local test environment another run, I'll use your branch for further tests.

vchrisb commented 1 year ago

Thank you for your effort. Interesting that you don't get values for cosPhi, frequency, and voltageMonitoring I would have expected these to be present in any e3dc system. If only there would be some documentation...

I missed testing poll_switches(), I'l look into it.

torbennehmer commented 12 months ago

Hi @vchrisb, so, finally I got some more time for HA: poll_switches works with the latest version of the branch, I get an empty array as I have no switches paired to E3DC.Can't really say what happens with switches though. get_pvis_data still fails as before, failing to find PVI_VOLTAGE_MONITORING.

Also as a further successful test, get_db_data_timestamp and get_db_data both work as well. And of course get_system_info_static, as this is called during init.

vchrisb commented 8 months ago

@torbennehmer , @eikowagenknecht and @bullitt186 would you mind trying this PR? 🥇 There are quite a few changes, but behavior should not have changed! Adding pyright also did surface a few bugs. 👍

eikowagenknecht commented 8 months ago

running mypy against it shows some more typing issues:

e3dc\_e3dc_rscp_web.py:116: error: Incompatible types in assignment (expression has type "str", variable has type "bytes")  [assignment]
e3dc\_e3dc_rscp_web.py:118: error: Incompatible types in assignment (expression has type "str", variable has type "bytes")  [assignment]
e3dc\_e3dc_rscp_web.py:126: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
e3dc\_e3dc_rscp_web.py:131: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
e3dc\_e3dc_rscp_web.py:135: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
e3dc\_e3dc.py:134: error: Incompatible types in assignment (expression has type "E3DC_RSCP_web", variable has type "E3DC_RSCP_local")  [assignment]
e3dc\_e3dc.py:144: error: Incompatible types in assignment (expression has type "int", variable has type "None")  [assignment]
e3dc\_e3dc.py:147: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:149: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:150: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:152: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:153: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:157: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:158: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:159: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:163: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:164: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:165: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:169: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:170: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:171: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:175: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:176: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:177: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:181: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:182: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:183: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:187: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:188: error: "None" has no attribute "startswith"  [attr-defined]
e3dc\_e3dc.py:189: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:193: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:195: error: Incompatible types in assignment (expression has type "str", variable has type "None")  [assignment]
e3dc\_e3dc.py:326: error: Incompatible types in assignment (expression has type "dict[str, Any]", variable has type "None")  [assignment]
e3dc\_e3dc.py:327: error: Incompatible types in assignment (expression has type "float", variable has type "int")  [assignment]
e3dc\_e3dc.py:468: error: Need type annotation for "idlePeriods"  [var-annotated]
tools\tests.py:118: error: "str" not callable  [operator]

I only checked some of those, but they were valid.

vchrisb commented 8 months ago

pyright and mypy don't have the same set of rules. For this PR pyright is configured with the following:

typeCheckingMode = "strict"
reportUnknownVariableType = "warning"
reportUnknownArgumentType = "warning"
reportUnknownMemberType = "warning"
reportUnknownParameterType = "warning"

I would be interested, if there is any change in behavior of the API compared to latest release of pye3dc with your E3DC.

eikowagenknecht commented 8 months ago

pyright and mypy don't have the same set of rules.

Yeah, I was thinking maybe mypy would catch some extra issues like it's done before in other projects. But sticking with pyright's feedback for now is of course a valid choice.

I would be interested, if there is any change in behavior of the API compared to latest release of pye3dc with your E3DC.

A quick run of tests.py and diff against master looked good, no changes to see.

torbennehmer commented 8 months ago

@torbennehmer , @eikowagenknecht and @bullitt186 would you mind trying this PR? 🥇

Will do, but probably not before the start of next week, unfortunately it's rather hectic here at the moment.

bullitt186 commented 8 months ago

I did manual back-to-back testing of all the get_*() calls this morning comparing the output of this PR compared to 0.8.2 and could not find any differences / oddities / performance differences. However I did not test the set_*() calls so far. But for the first ones it looks good to me!

vchrisb commented 8 months ago

I did clean the commit history