pbix / HA-solark-PV

Home Assistant integration for the SolArk PV Inverter
Apache License 2.0
37 stars 9 forks source link

HomeAssistant 2023.2.0 / pymodbus #32

Closed EiNSTeiN- closed 1 year ago

EiNSTeiN- commented 1 year ago

I've just upgraded to 2023.2 and get the following error

Setup failed for custom integration solark_modbus: Unable to import component: No module named 'pymodbus.client.sync'

Seems like the issue is here https://github.com/pbix/HA-solark-PV/blob/94c16bf6087edcf1a9d0496607f26e2f9e6323af/custom_components/solark/hub.py#L10

The new syntax for pymodbus should be:

from pymodbus.client import ModbusSerialClient, ModbusTcpClient

I'll try to test this soon and open a pull request if I can.

EiNSTeiN- commented 1 year ago

I tested the change I suggested above locally in HA using File Editor and it worked, but there is an additional issue with: https://github.com/pbix/HA-solark-PV/blob/6e26aab9a49714d440952a98d7b51b81166f629e/custom_components/solark/hub.py#L83-L84 the unit argument was renamed to slave in pymodbus

With these 2 changes my integration is back up and running

poldim commented 1 year ago

I tested the change I suggested above locally in HA using File Editor and it worked, but there is an additional issue with:

https://github.com/pbix/HA-solark-PV/blob/6e26aab9a49714d440952a98d7b51b81166f629e/custom_components/solark/hub.py#L83-L84

the unit argument was renamed to slave in pymodbus

With these 2 changes my integration is back up and running

What's considered the argument in those lines?

Changing L83 to below has me back up and running: kwargs = {"slave": unit} if unit else {}

For reference, here are the changes to HA: https://github.com/home-assistant/core/commit/05c1aff0f692ae749acdcdf89a907bf727001105

pbix commented 1 year ago

Thanks for the work on these. The question I have in my mind is what type of backwards compatibility issues are we likely to have as a result of this breaking change in pymodbus?

Do we need to bump the required version in manifest.json?

I hope to have a moment to look at this this weekend, send me a pull request if you can.

Quattrohead commented 1 year ago

Same issue for me, I restored to the previous version I had backed up.

pbix commented 1 year ago

These changes break 2023.1 so I think we need to think more about how to implement this fix. It does not seem to matter what I put in the requirements file for the required version of pymodbus. Also I cannot figure out what version of pymodbus HA is currently using at any moment. I cannot even find any references anywhere that tell me with the manifest file is evaluated by HA so not sure when any change I put in it might take effect. Any help would be appreciated.

pbix commented 1 year ago

HA 2023.2 has moved to pymodbus 3.1.1 which has breaking changes compared to the previous 2.5 version. I have committed changes based on the above plus added logic to make sure that all versions of HA will be properly supported across this change to pymodbus.

Breaking changes, gotta love em. What a waste of a perfectly good weekend.