jesserizzo / envoy_reader

MIT License
37 stars 26 forks source link

Convert API to use ConfigFlow in Home Assistant #41

Closed gtdiehl closed 3 years ago

gtdiehl commented 3 years ago

Home Assistant is letting previously implemented integrations continue development if they have implemented monitored_conditions in the configuration.yaml. For me as a user this was helpful as I could display/hide which sensors I was interested in. Now with ADR-0010 Home Assistant will not approve any code changes for an integration if the monitored_conditions is added to/removed from.

Currently Home Assistant communicates with envoy_reader API via one method update().

Users of the envoy_reader API are wanting more attributes to be monitored, such as Battery Storage, 3-Phase information, and individual CT Production and Consumption data.

The current implementation of envoy_reader.update() will not work moving forward. Something else probably needs to be implemented.

Maybe individual methods that return data. Or maybe a single update method but in a JSON format which can be looked up on the Home Assistant side. Not sure of the correct approach.

As well, when this change occurs a change will have to occur on the Home Assistant side to support ConfigFlow. Have to look into this but maybe during setup the user selects what they want to monitor?

This is the start of a long process and discussion.

lnlp commented 3 years ago

The current implementation of envoy_reader.update() will not work moving forward. Something else probably needs to be implemented.

Maybe individual methods that return data. Or maybe a single update method but in a JSON format which can be looked up on the Home Assistant side. Not sure of the correct approach.

As well, when this change occurs a change will have to occur on the Home Assistant side to support ConfigFlow. Have to look into this but maybe during setup the user selects what they want to monitor?

This is the start of a long process and discussion.

Unfortunately I lack the knowledge about the internals of Home Assistant and how sensors and the updating process are implemented. And there is hardly any documentation about how the 'Enphase Envoy' reader works, how it is implemented.

Can you please elaborate about what will not work and why?

It would be nice if any additional additions to envoy_reader/Enphase Envoy could just be configured by adding them to the yaml configuration (just like the currently supported parameters). However, I understand this is current not possibly.

Where is the problem and why does the problem exist?

Is this caused by a lack in the Enphase Envoy add-in or in Home Assistant itself?

What would be needed to get this fixed?

_The envoyreader repo may not be the most suitable place for discussions about HA but is this is probably the place where we get response from Enphase Envoy users.

gtdiehl commented 3 years ago

@lnlp Sorry for the late reply

The main issue really revolves around adding Envoy details into Home Assistant. About 8 months ago it was approved on the Home Assistant side that new integrations have to support a User Interface configuration (called config flow) rather than using the configuration.yaml file. Also existing integrations cannot add additional support to the already supported monitored_conditions section of the configuration.yaml

To add details regarding the Enphase Encharge battery storage system or 3-Phase details from en Enphase Envoy, we have to drop support for YAML and move to the UI configuration method. Personally I don't mind I like easy to use point and click configurations I just don't have any experience coding the UI configuration portion. It doesn't seem too bad, I have actually started on doing it for another integration I'm working on.

Than once the UI portion is done, I feel the envoy_reader library will have to change a bit so that when Home Assistant retrieves information it does it in a smart way. Currently the library grabs all the data than parses it based on what is returned by the device. Supporting more details the matrix grows and the code might have to be changed to minimize the introduction of bugs.

All of our issues/enhancements can be resolved by changing the envoy_reader library code and the Enphase Envoy add-in sensor code. I don't see any issues with Home Assistant itself.

I have started down the path of converting the Enphase Envoy add-in from YAML support to the Configuration Flow way. There are plenty of examples and like I mentioned above I do have some code to pull from an already started integration that is using it as well. Maybe by this weekend I'll have an initial commit to share with others.

I know GitHub now supports Discussions but the owner of the repository has to enable it. Discussions would allow us to discuss items without opening issues. @jesserizzo Can you enable Discussions for the envoy_reader repository? It should be under Settings -> Options.

gtdiehl commented 3 years ago

Closing issue. @bdraco converted the Home Assistant sensor to use config flow.