dingo35 / ha-SmartEVSEv3

Integrate SmartEVSEv3 with HomeAssistant through custom component
13 stars 8 forks source link

Empty screen shown when adding integration #15

Closed Ivesvdf closed 1 year ago

Ivesvdf commented 1 year ago

Hi,

When adding the integration to HA, I enter the serial number, click the confirmation button and then the following screen is shown: image

After clicking close I don't have the integration listed and cannot use it. Nothing related is shown in the HA logs.

I'm using the 1.0.5 version of this project and SERKRI-1.5.5 on the SmartEVSE. However previous versions of the SmartEVSE SW caused the same issue.

If I enter a different (incorrect) serial number, the system does allow me to add it by IP, which does work. However, due to unrelated issues with the SmartEVSE FW, I have difficulty getting the static IP to stick (lost after power loss).

Best, Ives

calamarain commented 1 year ago

Is it's web interface reachable from http://smartevse-####.local/, with #### the serial number?

Ivesvdf commented 1 year ago

Yes, it works fine that way.

Met vriendelijke groeten, Ives

On Tue, Apr 11, 2023 at 1:16 PM calamarain @.***> wrote:

Is it's web interface reachable from http://smartevse-####.local/, with

the serial number?

— Reply to this email directly, view it on GitHub https://github.com/dingo35/ha-SmartEVSEv3/issues/15#issuecomment-1503143751, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACAR72GRFAKGT5BN6D3VT3XAU4Q7ANCNFSM6AAAAAAW2DMFQA . You are receiving this because you authored the thread.Message ID: @.***>

dingo35 commented 1 year ago

What HomeAssistant version are you running?

Ivesvdf commented 1 year ago

Home Assistant 2023.4.0, in a docker container. The previous .3.0 release showed the same issue.

I would think the issue isn't related to the name lookup, because otherwise how could the system differentiate between a good serial number (showing the blank dialog) and a bad serial number (asking for an IP address)?

Met vriendelijke groeten, Ives

On Tue, Apr 11, 2023 at 1:25 PM dingo35 @.***> wrote:

What HomeAssistant version are you running?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

dingo35 commented 1 year ago

Ok I cannot reproduce this problem myself, tried a total homeassistant reinstall but no blank dialog here.

However, it is reported by other users in the past. I understand that in most cases this was the problem that they were trying to configure the integration while it was already auto-configuring itself.

My suggestion would be to uninstall the integration from HomeAssistant, it will then still remain in HACS; restart HA AND your browser and see if it autoconfigures (give it some time), without entering the serial number. If it hasn't discovered your smartevse, try the manual configuration, make sure you use the four digit serial number, and NOT the five digit one.

If that doesn't work, you can always use the ip address. Even if it is not a static address, your SmartEVSE will be always on, so it will only change ip address if you swap your router....

EDIT: you might even try SmartEVSE-xxxx.local as an ip address, if it is pingable it should work too!

Ivesvdf commented 1 year ago

I tried your steps but without result. The suggestion of adding the mDNS name as an IP won't work, because inside of the HA container ping does not support mDNS resolution, instead HA uses some python library for this.

However, I did make some progress. When the device is online when booting HA, an error is logged:

Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/discovery_flow.py", line 95, in _async_start
    await gather_with_concurrency(
  File "/usr/src/homeassistant/homeassistant/util/async_.py", line 178, in gather_with_concurrency
    return await gather(
  File "/usr/src/homeassistant/homeassistant/util/async_.py", line 176, in sem_task
    return await task
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 840, in async_init
    flow, result = await task
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 868, in _async_init
    result = await self._async_handle_step(flow, flow.init_step, data)
  File "/usr/src/homeassistant/homeassistant/data_entry_flow.py", line 373, in _async_handle_step
    if not isinstance(result["type"], FlowResultType):
TypeError: 'NoneType' object is not subscriptable

However at this point there was no evidence that this is related to your HACS extension. I then added some debug code (while I am a decent Python programmer I don't know anything about how HA works) in this file:

        try:
            result: FlowResult = await getattr(flow, method)(user_input)
        except AbortFlow as err:
            result = _create_abort_data(
                flow.flow_id, flow.handler, err.reason, err.description_placeholders
            )

        # Added block for debugging
        if result == None:
            report( ( f"Fail = flow{flow} {step_id} {user_input}" ), error_if_core=False)                       
            result = _create_abort_data(
                flow.flow_id, flow.handler, err.reason, err.description_placeholders
            )

        if not isinstance(result["type"], FlowResultType):
            result["type"] = FlowResultType(result["type"])  # type: ignore[unreachable]
            report(
                (
                    "does not use FlowResultType enum for data entry flow result type. "
                    "This is deprecated and will stop working in Home Assistant 2022.9"
                ),
                error_if_core=False,
            )

(yeah err is not defined, doesn't matter, we just want the report statement)

This gives the following report:

Detected code that Fail = <custom_components.smartevse.config_flow.SmartEVSEConfigFlow object at 0x7fd88c004bb0> zeroconf ZeroconfServiceInfo(host='192.168.0.226', addresses=['192.168.0.226'], port=80, hostname='SmartEVSE-64xx.local.', type='_http._tcp.local.', name='SmartEVSE-64xx._http._tcp.local.', properties={'_raw': {}}). Please report this issue.

(serial number censored). So to my untrained eye it looks like

Any idea why this may be?

dingo35 commented 1 year ago

Well you know definitely your way better around Python than I do :-)

The problem is that HA is IMHO poorly documented; I just specify that the zeroconf routines should be used (via manifest.json and specifying a async_step_zeroconf function in config_flow.py ), I gained the knowledge needed to build this integration by peeking at other integrations and a lot of experimenting, not the best way to build software :-(

So this error seems to be generated by the insides of the HA beast; with your python knowledge it would be probably be easiest to install HA-core in a test environment and see where it goes wrong; but you would need to be very determined to do that .....

dingo35 commented 1 year ago

I can reproduce the problem, it occurs with the "container" type of installation of HomeAssistant. There are issues with zeroconf in the (docker) container, the container is not as transparent as I would expect....

Ivesvdf commented 1 year ago

Yeah I agree, I looked into it a bit yesterday evening and I found a few things.

  1. Cause is that the base container for HA does not support mDNS, instead they use a library. So what happens is that the requests library resolution fails because it uses the mDNS name.
  2. At the end of the async_step_zeroconf function you have a if no error, return this. You don't return anything if there are errors. This is what causes the empty box. Instead I'd do something like return self.async_abort(reason="whatever"). Returning None is not allowed.
  3. My issue is well known, and described here: https://github.com/home-assistant/home-assistant.io/issues/14153
  4. Best practice for a solution is apparently to store the host string using the IP address. If the IP changes, then the next zeroconf initialisation will update the IP address.
  5. For correct handling, look at what they did in https://github.com/home-assistant/core/blob/dev/homeassistant/components/elgato/config_flow.py
dingo35 commented 1 year ago

Thanks Ives, that helps a lot. I noticed that, although from the container you cannot ping the mdns reported entries, you CAN ping the hostname! If you run netstat from your container you can see that.

So I pushed an update that fixes the white box, and also tries different suffixes. I also have a fix ready for the smartevse-firmware, currently the hostname is derived from the mac address; I am changing this so as a hostname also the serial number is used, so it will be detected within the container, but also should be accessible from the container....

Ivesvdf commented 1 year ago

I tried your fix on master, but it didn't work for me. I cannot resolve the SmartEVSE-xxxx hostname from within the container.

ives@DockerHost1 [ ~/home_assistant ]$ docker-compose exec homeassistant /bin/bash
bash-5.1# ping smartevse-64xx
ping: bad address 'smartevse-64xx'
bash-5.1# ping SmartEVSE-64xx.local
ping: bad address 'SmartEVSE-64xx.local'
bash-5.1# ping SmartEVSE-64xx
ping: bad address 'SmartEVSE-64xx'
bash-5.1# ping SmartEVSE-64xx.lan
ping: bad address 'SmartEVSE-64xx.lan'

while on the host only the .local address resolves. Not the pure hostname.

The fact that you can probably has to do with your network configuration and search domain. In the thread I linked to in my previous post they also tried to resolve the hostname based on the IP address but determined this was not reliable.

I gave it a bit of a shot to reproduce what they did for elgato (included in HA), and ended up with something like this: https://github.com/Ivesvdf/ha-SmartEVSEv3/blob/main/custom_components/smartevse/config_flow.py

So in the autoconfiguration, use the IP and not the hostname, then use the updates= parameter when calling _abort_if_unique_id_configured, so that if the IP changes over time the system will self fix.

I tested this by rebooting HA as normal, the autodiscovery immediately discovered the device and the communciation check passed. Then I set the SmartEVSE to wifi config mode, changed the IP to some other address, gave it a few seconds.

This gave the following logging output:

2023-04-12 18:46:38.028 INFO (MainThread) [custom_components.smartevse.config_flow] Host is 192.168.0.49
2023-04-12 18:46:38.079 INFO (MainThread) [homeassistant.components.sensor] Setting up sensor.smartevse
2023-04-12 18:46:38.079 INFO (MainThread) [homeassistant.components.select] Setting up select.smartevse
2023-04-12 18:46:38.080 INFO (MainThread) [homeassistant.components.switch] Setting up switch.smartevse
2023-04-12 18:46:38.080 INFO (MainThread) [homeassistant.components.number] Setting up number.smartevse

I then connected a vehicle simulator to the smartEVSE to confirm the connection was still working on this new IP, which it was (I could see the change in HA). So this does seem to update the HA host correctly through autodiscovery when the serial number is known.

I didn't test the manual config flow. Let me know your thoughts.

dingo35 commented 1 year ago

I will look into your work tomorrow, but for my version to work you would need this version of the smartevse firmware, which has a correct hostname: hostname.zip

(first flash firmware.bin, then spiffs.bin).

dingo35 commented 1 year ago

Liked your solution Ives, so I committed it c7e9008fa8578f6bc75733e6a0867b85c953f841 and released it in 1.0.6 .

dingo35 commented 1 year ago

I have an OT question, it seems easier for you to get through the caverns of HomeAssistant/python: I introduced the switch in the integration, on request, so it is easy to switch the SmartEVSE on/off in an automation or script. Of course it sort of doubles the "mode" selector. So when you turn the switch from OFF to ON, it takes a full update cycle (max 1 minute) for the selector to update; there must be a smarter way to update the selector in realtime (without consulting the REST API).

I've tried adding a self.async_write_ha_state() at different places, I can't find the assignment statement I have to do to update the display immediately, because I don't know the relationships between the objects in the integration and the objects in HASS.

Would you have any clue?

Ivesvdf commented 1 year ago

I'm actually not that convinced of the purpose of the on/off switch.

I suppose the intended purpose is that it makes switching the device on/off manually somewhat easier. For automations it definitely won't matter, creating 2 helpers in HA with the associated automation action

      - device_id: 5f109cf3ca1fc3449d37bb6c7a7b5c59
        domain: select
        entity_id: select.smartevse_mode_selector
        type: select_option
        option: "OFF"

isn't that difficult either but okay, more difficult than clicking a button.

How do we want it to behave? I suppose it should be another level above the mode selector. But

Even ignoring that, I agree it's annoying that the mode selector doesn't update when you use the on/off button until the next refresh cycle. However, it's also annoying that all of the other fields don't update. Things like SmartEVSE access should change when turning the switch on/off, and they don't either. So I'm afraid the issue of the mode selector taking a long time to change is just a part of the problem.

I wonder if a better solution would be to force a data refresh (GET request) through the coordinator after a button is pressed or selector is changed. At least in scenario's where the user is interacting with the UI through HA this will give the impression of a responsive UI.

dingo35 commented 1 year ago

The switch goes to Smart mode if you switch ON, because if you want it on, you dont want to wait for solar; and if no meters are installed Smart mode will behave the same as Normal mode.

Forcing a new GET would be the easy road, but I keep thinking there must be a smarter way inside HA.

Thanks for your help!

Ivesvdf commented 1 year ago

I tested your new changes and they seem to work fine for my setup. SmartEVSE is autodetected fine.

Are you sure that smart & solar are identical to normal if no meters are installed by the way? Perhaps there's something wrong with my setup but when I set mainsmeter and EV meter to disabled and set mode to Smart (either through the on-device web UI or through HA), it almost immediately goes to Communication error mode, with "check serial com" on the LCD.

dingo35 commented 1 year ago

You are correct in that when MainsMeter is disabled, you should not even be allowed to switch to Smart and or Solar. I believe those checks are made in the LCD screen but not (yet) in the web/API. Im going to look into that.

The HA switch should switch to Normal in that case, IMHO that would be "expected user behaviour".

Ivesvdf commented 1 year ago

Agreed, through the LCD it's not possible but through the API it is.

Closing this ticket as completed because the issue has been resolved in v1.0.6.

Cheers!