vincentwolsink / home_assistant_enphase_envoy_installer

This is a HACS custom integration for Enphase Envoys with firmware version 7 and up.
Apache License 2.0
54 stars 10 forks source link

Updated envoy_reader with smarter fetching and fetching properties based on jsonpaths #41

Closed mnederlof closed 11 months ago

mnederlof commented 12 months ago

And other updates:

I also included test data, so we can more easily test the EnvoyData parser, without waiting for output of the envoy, so it is easier to test the jsonpath values.

mnederlof commented 12 months ago

I have been running this version for some days now, works great (for me at least 😄 )

The way this is set up makes more sense to me, as the counter values can be defined separately for separate scenario's of a envoy. Also, all values are now placed in a EnvoyData class (e.g. EnvoyMetered or EnvoyStandard), while all fetching and token logic is stored in the EnvoyReader class.

This envoy_reader.py is fully backward compatible with the current way data is updated in __init__.async_update_data. Also adding more sensors should be easier in the future, as we only have to define the sensor for HA, and then define the jsonpath in the dataclass; as the __getattr__ methods in the EnvoyReader and EnvoyData class will resolve the value configured in the attribute holding the jsonpath.

mnederlof commented 12 months ago

(i just noticed that the escalation to installation account does not work properly yet, working on it) Fixed.

vincentwolsink commented 11 months ago

Wow, this is a huge improvement in code! 😃 Also love having the json fixtures included. We might want to use them to do unit testing later.

Also love the caching system. But the cache time per endpoint seems a bit arbitrary to me? At least for my Envoy most of the endpoints only update every 5 minutes. Why to choice to not cache some endpoints and others for 20 seconds?

Thanks for all your contributions. I will also set you as code owner in the manifest and add you as collaborator to the repo, if you like. 🙂

vincentwolsink commented 11 months ago

@mnederlof This change does throw quite a lot of warnings into my Home Assistant logs:

Logger: custom_components.enphase_envoy.envoy_reader
Source: custom_components/enphase_envoy/envoy_reader.py:1223
Integration: Enphase Envoy (Installer) ([documentation](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/), [issues](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/issues))
First occurred: 15:47:49 (84 occurrences)
Last logged: 15:50:52

Attribute daily_consumption_l2 unknown
Attribute lifetime_consumption_l2 unknown
Attribute consumption_l3 unknown
Attribute daily_consumption_l3 unknown
Attribute lifetime_consumption_l3 unknown
Logger: custom_components.enphase_envoy.envoy_reader
Source: custom_components/enphase_envoy/envoy_reader.py:617
Integration: Enphase Envoy (Installer) ([documentation](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/), [issues](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/issues))
First occurred: 15:47:49 (16 occurrences)
Last logged: 15:50:52

Attribute consumption unknown
Attribute daily_consumption unknown
Attribute lifetime_consumption unknown
Attribute voltage unknown
mnederlof commented 11 months ago

Maybe the detect_model comes up with EnvoyMetered, instead of EnvoyStandard. Could you post some json fixtures from you EnvoyStandard, then maybe i can find some other things to differentiate on

mnederlof commented 11 months ago

And we should make that _LOGGER.warning in __getattr__ in EnvoyReaderdo _LOGGER.debug maybe 😇

mnederlof commented 11 months ago

Wow, this is a huge improvement in code! 😃 Also love having the json fixtures included. We might want to use them to do unit testing later.

Also love the caching system. But the cache time per endpoint seems a bit arbitrary to me? At least for my Envoy most of the endpoints only update every 5 minutes. Why to choice to not cache some endpoints and others for 20 seconds?

Thanks for all your contributions. I will also set you as code owner in the manifest and add you as collaborator to the repo, if you like. 🙂

Well, yes, the inverters only update every 5 minutes, but they do not do this synchronously (so with a lot of inverters, it could yield a different output every X seconds. Since users can set the poll_interval now via configurable options, it would make sense to do this arbitrary value of 20 seconds

But, yes, i did choose that arbitrary value there.

Also, when using a CT, you could do polling a lot, which should reflect the realtime usage of that seconds (when not using the /stream/meter endpoint), so this 20 seconds should alleviate the pressure on the envoy a bit.

We could make that cache setting be configurable too though 😄

mnederlof commented 11 months ago

@mnederlof This change does throw quite a lot of warnings into my Home Assistant logs:

Logger: custom_components.enphase_envoy.envoy_reader
Source: custom_components/enphase_envoy/envoy_reader.py:1223
Integration: Enphase Envoy (Installer) ([documentation](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/), [issues](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/issues))
First occurred: 15:47:49 (84 occurrences)
Last logged: 15:50:52

Attribute daily_consumption_l2 unknown
Attribute lifetime_consumption_l2 unknown
Attribute consumption_l3 unknown
Attribute daily_consumption_l3 unknown
Attribute lifetime_consumption_l3 unknown
Logger: custom_components.enphase_envoy.envoy_reader
Source: custom_components/enphase_envoy/envoy_reader.py:617
Integration: Enphase Envoy (Installer) ([documentation](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/), [issues](https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/issues))
First occurred: 15:47:49 (16 occurrences)
Last logged: 15:50:52

Attribute consumption unknown
Attribute daily_consumption unknown
Attribute lifetime_consumption unknown
Attribute voltage unknown

see https://github.com/vincentwolsink/home_assistant_enphase_envoy_installer/pull/43