core
core copied to clipboard
Add Sungrow solar inverter integration
Proposed change
Add modbus read out of Sungrow solar inverter. Tested with SG5.0RT but should work with: SG30KTL-M, SG30KTL-M-V31, SG33KTL-M, SG36KTL-M, SG33K3J, SG49K5J, SG34KJ, LP_P34KSG, SG50KTL-M-20, SG60KTL, SG80KTL, SG80KTL-20, SG60KU-M, SG5KTL-MT, SG6KTL-MT, SG8KTL-M, SG10KTL-M, SG10KTL-MT, SG12KTL-M, SG15KTL-M, SG17KTL-M, SG20KTL-M, SG80KTL-M, SG111HV, SG125HV, SG125HV-20, SG33CX, SG40CX, SG50CX, SG110CX, SG30KTL, SG10KTL, SG12KTL, SG15KTL, SG20KTL, SG30KU, SG36KTL, SG36KU, SG40KTL, SG40KTL-M, SG50KTL-M, SG60KTL-M, SG60KU
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] The code has been formatted using Black (
black --fast homeassistant tests
) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [x] Documentation added/updated for www.home-assistant.io https://github.com/home-assistant/home-assistant.io/pull/21472
If the code communicates with devices, web services, or third-party tools:
- [x] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest
. - [x] New or updated dependencies have been added to
requirements_all.txt
.
Updated by runningpython3 -m script.gen_requirements_all
. - [x] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to
.coveragerc
.
The integration reached or maintains the following Integration Quality Scale:
- [ ] No score or internal
- [x] 🥈 Silver
- [ ] 🥇 Gold
- [ ] 🏆 Platinum
To help with the load of incoming pull requests:
- [x] I have reviewed two other open pull requests in this repository.
Hi @DasBasti,
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
I think I managed to fulfill all the requirements in the tests. Could you please trigger a run?
I added a link to your documentation PR in the description.
The code change merged by github somehow mangled the import section of the test. The last commit fixes that.
Is there still something missing for the merge?
The pytests are currently failing. Please could you check why?
The tests failed in an unrelated component (history_stats/test_sensor.py::test_measure_sliding_window) and might have been realted to #81175. I rebased the PR to update to the current code.
This time the jobs fail in cache cleanup and recorder component tests. What is the correct way to deal with such issues?
Sorry that there are problems causing difficulty here.
If there are flaky/failing CI tests on the dev branch then probably you are not the only one to notice.
Rebase and retest in 24h often works in that case.
But I had a check on discord and couldn't see it mentioned.
Hi @02JanDal
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
@davet2001 I've been in contact with @DasBasti as we've both been working on an integration for Sungrow inverters in parallel. As he was further ahead with the integration itself and the process of getting it reviewed and merged while my backing library was more advanced we decided to work together already as part of this PR, since it was not yet merged. As such I have modified Bastians code to use my library.
A review of these changes would be greatly appreciated!
It looks like you have rebased and force pushed.
I can't easily see what has changed since I last reviewed and I would prefer not to re-read all >1000 lines.
Please could you describe what you changed?
@davet2001 Thanks for taking the time!
Here is a diff of just the changes to this integration I've made: https://gist.github.com/02JanDal/7c0d50e191e6bc74422a6c81ac6e1282 Note that it excludes changes in generated files like requirements and CODEOWNERS, as those also include changes from other commits so are harder for me to isolate.
The file with the most significant changes is __init__.py
, as my library is async (although not 100% until HA switches to pymodbus 3 but that has to wait for upstream and the modbus integration) I had to move some stuff out of the SungrowData constructor into async_setup_entry
. It also contains some aspects in SungrowCoordinatorEntity
previously found in sensor.py
, which is mostly just a preparation for more entity types (which I've got ready locally but will be part of a future PR).
config_flow.py
should be functionally equivalent.
const.py
lost the sensor definitions, I moved them into sensor.py
, again as it makes more sense to me once more entity types are there.
sensor.py
gained the sensor definitions, quite a few more as my library supports more, while also losing some details in them (like device class and unit) that I instead compute inside of SungrowCoordinatorEntity
based on the metadata in pysungrow
.
In the tests the changes are almost exclusively about changing what is mocked, I did however introduce a fixture that takes care of the patching that is done for most tests, as well as creating and registering the config entry.
Other than that it's of course quite a few import changes, as well as some adaptions to changes in the HA API, like using SensorDeviceClass
instead of strings and going from async_setup_platforms
to async_forward_entry_setups
.
Ok, this is quite a big job now.
It's close to a rewrite and using quite a new python library (so maybe that needs reviewing as well).
There is a big benefit to keeping a PR small. Faster to review, faster to re-review etc. Will re-review as soon as possible but it won't be immediately.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
@02JanDal can you please have a look into the changes I made, since I currently have no access to a sungrow inverter anymore
Yes, I need to make another push on this, need to finish the update to pymodbus 3 (which the modbus integration was updated to) and fix a few instabilities regarding the various ways to connect (WiFi, LAN, LAN via WiNet stick) and bugs found by a few testers in a Swedish group where I posted this. Hope to get to this early next week.
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.
Because there hasn't been any activity on this PR for quite some time now, I've decided to close it for being stale.
Feel free to re-open this PR when you are ready to pick up work on it again 👍
../Frenck