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

Missing code for show_phase + enlighten checkbox #3

Closed posixx closed 1 year ago

posixx commented 1 year ago

I cannot test the integration as i don't have D7 firmware, but there are 2 issues:

the initial show_phase is missing on EnvoyReader (lines should be inserted on line numbers):

init.py line 52: show_phase=config.get(CONF_SHOW_PHASE, False),

envoy_reader.py: line 104: show_phase=False, line 119: self.show_phase = show_phase

Also as this integration only wotks with D7 fimrware the option "Use Enlighten" should be removed from the initial config page as this will be always true

vincentwolsink commented 1 year ago

Hi @posixx. The show_phase config flow to envoy_reader is not needed because I just always read the production.json?details=1 url. This is what the envoy homepage itself also does.

The code is actually tested, see #2

I still need to check compatibility with the D5 firmware, but I need someone to work with me on that one. Thats why the checkbox is not gone yet. But if I cannot get it to work I will remove it. Thanks for the suggestion.

posixx commented 1 year ago

Hello @vincentwolsink. Regardless of using the same URL still these changes are needed in order to correctly setup EnvoyReader.

Without these (trying without "use enlighten") i get:

2023-05-05 15:05:21.281 ERROR (MainThread) [custom_components.enphase_envoy.config_flow] Unexpected exception Traceback (most recent call last): File "/config/custom_components/enphase_envoy/config_flow.py", line 151, in async_step_user envoy_reader = await validate_input(self.hass, user_input) File "/config/custom_components/enphase_envoy/config_flow.py", line 28, in validate_input envoy_reader = EnvoyReader( TypeError: EnvoyReader.init() got an unexpected keyword argument 'show_phase'

The class EnvoryReader needs the show_phase argument in order to have it configured. Don't know why this works for the user on #2 though..

vincentwolsink commented 1 year ago

Thanks for the error. Probably it was not spotted before since this code is only used during configuration of the integration during installation.

I removed it from the call there. Could you try when installing the plugin from branch main @posixx ?

posixx commented 1 year ago

Still the same error. I don't see what you are trying to achieve by removing. It is the option for showing the phase sensors or not. So this is a mandatory option and needs to be there,

vincentwolsink commented 1 year ago

I'll do some more tests (and maybe rewrites) and will get back on this soon.

vincentwolsink commented 1 year ago

@posixx @bes-r would be nice if you both could test 0.1.8-beta1

Next step will be implementing tests against static data, so I don't need others for that 😄

bes-r commented 1 year ago

It's not working: lifetime_production_phase' is an invalid keyword argument for int()

posixx commented 1 year ago

I've asked enphase support to push D7 firmware to my device. When upgraded i will test.

bes-r commented 1 year ago

0.1.8-beta2 is working. But I'm not sure what has been changed.

vincentwolsink commented 1 year ago

Hi @bes-r thanks for checking! Nothing much changed for the user. The checkbox during installation/configuration is no longer needed. And I rewrote a lot of code under the hood.

bes-r commented 1 year ago

@vincentwolsink if you need more testing, let me know!

vincentwolsink commented 1 year ago

Also removed all obsolete code that was there for older firmwares. Adjusted the configuration flow accordingly. See release 0.1.9