-
-
Notifications
You must be signed in to change notification settings - Fork 36.5k
Hypontech micro invertors support via Hyponcloud #159442
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
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
@pantherale0 thank you for your review, it simplified greatly the error catching. |
|
No worries, the PR looks pretty good to be honest, there is a little bit around the device_registry handling that you could simplify. I assume the overview device is a sum of all the devices that a user may have in their account? |
| @dataclass | ||
| class HypontechSensorData: | ||
| """Representing different Hypontech sensor data.""" | ||
|
|
||
| # plant_data: PlantData | ||
| overview_data: OverviewData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if we currently don't then we currently don't have to wrap it
| def __init__(self, coordinator: HypontechDataCoordinator) -> None: | ||
| """Initialize the entity.""" | ||
| super().__init__(coordinator) | ||
| self._attr_device_info = DeviceInfo( | ||
| identifiers={(DOMAIN, coordinator.config_entry.entry_id)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do devices work in hypontech? What if I have 2 devices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Hypontech, you have one account where you have access to devices in different locations ("Plant"). In each location you can have multiple devices (a batterie, a few micro inverters etc.). To have an idea of what are the informations available you can look at this example script https://github.com/jcisio/hyponcloud/blob/main/example.py.
So, if you have 2 micro inverters, usually they are in the same location. You might put them in different locations like balcony, garden but they are basically the same location. Location is used for weather forecast, so if all devices are at your house normally you want to put them in a single location.
Then "Overview" is aggregation of all locations.
For the simplicity, in this first PR I want just a virtual "overview" device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would kinda opt to say that we should implement the plants first, as that will provide better data than the complete overview. As in
- If someone has 2 locations (let's say office and home), then the overview is useless
- If you only have a single location, then the overview is useless compared to the location total, as they are the same
So I would say entities that have the exact sum aren't really useful by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I'd like to support the simplest and the most popular use case first to keep the PR small. Very few HA users have Hypontech inverters, amongst them very few have multiple locations.
This is also my first integration so I avoided to make it complicated, and I can test only one location in my setup. I can guess for multiple location, but I haven't checked the API for multiple devices per location. Dongles are not useful I think, but Inverters could replace locations.

That said, if you think this change helps it getting merged then I'd be happy to implement it. Location seems to be a good intermediate solution, and I think we will keep it anyway even if we implement inverters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! I've added the Plant sensors.

Proposed change
Add an integration to support Hypontech micro inverters via Hyponcloud. It's a new company (established in 2018) growing fast and it's top 10 world wide.
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: