cdpuk / givenergy-local

Home Assistant integration for local access to GivEnergy inverter and battery systems
MIT License
47 stars 14 forks source link

Proof of concept - using givtcp modbus library #62

Closed reidjr closed 5 months ago

reidjr commented 11 months ago

I don't expect you to accept this, but just to start the conversation. I updated my Gen2 inverter to 911 ( pre-release) and givenergy -local then fails. 909 is still supported, but after that the crc changes. CRC and new features require an update to givenergy-modbus 10.1, which britkat has now forked to within givtcp. To get this working ( with my basic skills) I cloned givenergy-modbus-10.1, and replaced the modules with the britkat versions. There are then breaking changes, including requirement to add an AIO flag to some of the module calls. I have stubbed those out, and worked around the others. Changed the manifest to require my github version of givenergy modbus, and it all ( sort of ) works. I have moved my production HA to GivTCP, but wanted to get this working, and might be usefull if you want to do a major rewrite.

( I also added switch to reboot inverter, which was one of the new features)

cdpuk commented 11 months ago

Thanks for the prototype! I had a partial go at this myself a few weeks ago, but didn't get to the end of it. Your code will almost certainly be useful, but it's going to take me a while to find the time to polish off the changes and package everything up properly.

gavanfantom commented 11 months ago

To add to the conversation, I too make a fork of givenergy-modbus, but cherry picked the minimal changes to version 0.10.1 to work with the new inverter firmware as a stopgap.

Changing the givenergy-modbus requirement to "givenergy-modbus @ https://github.com/gavanfantom/givenergy-modbus/archive/refs/tags/v0.10.1b.zip" in manifest.json ought to work, although I did end up manually installing it with pip in the homeassistant docker container.

andynash commented 9 months ago

If I'm understanding this correctly, we shouldn't update to 911 firmware as this is a breaking change for this integration (currently), is that right?

andynash commented 7 months ago

Damn it, I now see I saw this issue and completely forgot about it. I can now confirm that yes, if you update beyond 909 givenergy-local will not work :-(

andynash commented 7 months ago

So - in desperation 😁 I've tried @gavanfantom's modbus version as described above which didn't work for me, I saw the same errors as with the original integration.

I've also then tried installing @reidjr's fork with changes for 911, and when that didn't work I tried forking my own version of that (something i've never done before) and merging the handful of changes to the official integration since then.

In both the latter cases (@reidjr's fork, and after I merged the latest @cdpuk changes) I got the following error when reloading the integration or restarting Home Assistant:

2023-12-08 11:08:12.301 ERROR (MainThread) [homeassistant.loader] Unexpected exception importing platform custom_components.givenergy_local.config_flow File "/home/homeassistant/.homeassistant/custom_components/givenergy_local/config_flow.py", line 25 2023-12-08 11:08:12.313 ERROR (MainThread) [homeassistant.config_entries] Error importing platform config_flow from integration givenergy_local to set up givenergy_local configuration entry: Exception importing custom_components.givenergy_local.config_flow

After I removed the isAIO=False from line 25 (reverting to the original) the error later changed to:

2023-12-08 11:33:37.214 ERROR (MainThread) [custom_components.givenergy_local.coordinator] Error fetching Inverter data: Error communicating with API: GivEnergyClient.refresh_plant() got an unexpected keyword argument 'isAIO'

I removed all other traces of this flag, and had the same error, so replaced them all again and still have this error (which doesn't make a whole lot of sense to me).

I don't suppose that is something anyone can help me easily fix?

cdpuk commented 5 months ago

I'm going to close this down as the discussion on #61 seems to be reaching a stable state. The current approach is to avoid using the givenergy_modbus 0.10.1 release, and instead use the pre-release code that's a complete rewrite of the original library in a much better way. This is currently available via the HACS beta release stream, but will soon be promoted to v2.0.0.

Once that's done, new issues to start bolting in extra features (such as those discussed here) are very much welcome.