Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of concept - using givtcp modbus library #62

Closed
wants to merge 11 commits into from

Conversation

reidjr
Copy link
Contributor

@reidjr reidjr commented Jul 25, 2023

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
Copy link
Owner

cdpuk commented Aug 16, 2023

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
Copy link

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
Copy link

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
Copy link

andynash commented Dec 8, 2023

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
Copy link

andynash commented Dec 8, 2023

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
Copy link
Owner

cdpuk commented Jan 28, 2024

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.

@cdpuk cdpuk closed this Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants