Add growatt server url https://openapi.growatt.com/ #103542
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @DerMuffin
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!
home-assistant
bot
added
bugfix
cla-needed
integration: growatt_server
small-pr
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Hey there @muppet3000, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
home-assistant
bot
added
Quality Scale: No score
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
|
Please fill out the PR template completely. |
MartinHjelmare
changed the title
This comment was marked as abuse.
This comment was marked as abuse.
I'm happy to help with this if you don't have the time. Otherwise, remember raise a issue 6 months before removing it completely like e.g. #98828 |
The Home Assistant project does not support, recommend, or endorse the use of custom integrations. As a matter of fact, these are not checked or reviewed by the Home Assistant
This is a non-reason. More than half of the integrations of Home Assistant use such constructs.
From our point of view, this integration is in active maintenance and we very much welcome PRs to improve the integration shipped in core. Again, if you don't want to contribute @muppet3000, that is fine and up to you. But you are now spreading misinformation. TL;DR: We welcome any PR that improves this integration, including this one. ../Frenck |
|
I was simply trying to follow on the recommendations of the HA Core team that I was given last year by @MartinHjelmare here: #81951 (comment) ("If that's not possible, it may be better to move the integration to a custom integration."). I didn't realise the stance from the core maintainers had changed We (I) have played cat & mouse with Growatt over the last 2 years as they have repeatedly ignored all attempts to engage and have now actively sought methods to block any external API integrations. Hence my reluctance to spend any more time on it. This PR will be a sticking plaster (at best) until Growatt block API calls to that URL as well (if they haven't already). The problem that needs to be solved here is in the upstream library that this integration leverages: https://github.com/indykoning/PyPi_GrowattServer To avoid spreading any more 'misinformation' (I'm not going to even go try and articulate how strongly I disagree with this statement given how hard I've worked to keep this integration alive for the last 2 years), I'll submit a PR to remove myself as the codeowner and I wish whoever decides to take it up the very best of luck and I hope they're able to succeed where I have failed. |
| @@ -8,6 +8,7 @@ | |||
| DEFAULT_NAME = "Growatt" | |||
|
|
|||
| SERVER_URLS = [ | |||
| "https://openapi.growatt.com/", | |||
There was a problem hiding this comment.
This used to be the default, and was previously put into the config entries as well.
Should we migrate existing config entries to update the stored URL from https://server-api.growatt.com/ to https://server.growatt.com/?
../Frenck
There was a problem hiding this comment.
This has never been the default, it is a new entry.
I believe this comment has been copy/pasted from the other PR.
With regards to migrating existing config entries, I don't really mind, it's not something I've ever done historically for this integration as the only way people have ever been able to reconfigure it is by deleting it and then re-adding it anyway (which is generally the first thing people do when they get problems with it).
It's worth remembering that I don't even know if this will fix the issue, that's down to @DerMuffin to confirm as the person that raised the PR.
There was a problem hiding this comment.
@frenck I agree that adding this URL without migrating existing entries would be enough for now. That way not all users will switch at once, which probably increases the chance it will quickly be blocked again.
|
Just made the changes by hand to see if this would solve my issue, I was able to connect after. So looks like this indeed solved the issue. Thank you for the work, looking forward to seeing this into master :) |
|
Just to confirm, this has been working for me for 2 weeks stable now. |
Where do i change it myself? |
I am using the docker image running on kubernetes so for you it is maybe different /usr/src/homeassistant/homeassistant/components/growatt_server/const.py |
|
@DerMuffin could you convert the draft into a real PR and complete the nessecary steps so this can be merged? |
|
@DerMuffin I contacted Growatt IT department, they provide these URLs: "https://openapi-cn.growatt.com", # Chinese server |
Breaking change
Proposed change
Type of change
Add URLAdditional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: