Missing radiobuttons on configuration

[gelse]

Describe the bug When creating a new config (after removing previous installation / upgrade), the radio buttons for the inverter type are missing, thus leading to invalid user input: User input malformed: value must be one of [] for dictionary value @ data['plugin']

Mandatory details

1. Integration Version: 2022.12.12 (also tried 2022.8 and main)
2. HA Version: 2022.12.7

Detailed Error Log there is no error log, it is an UI error.

[gelse]

as a fallback it would be nice to have a how-to somewhere how i can configure the integration via YAML.

[wills106]

Have you upgraded from 0.6.4 or earlier without removing the Integration from the Integration page, restarting HA and adding it back to the Integration page?

Edit: Possibly duplicate of #246

[gelse]

no, i followed the instructions by the book.
i removed the integration from HA i removed the integration from HACS i restarted HA i added the integration in HACS i restarted HA again i tried to add he integration in HA

[gelse]

and also, as i said in the description above, there are NO errors of solax in the logs of HA.

[wills106]

You are somehow missing the whole section from the config flow where you select which plugin you want.

On the Integration page have you tried forcing a refresh? Ctrl + F5 Then try adding it again?

[gelse]

yes, that was the first thing i tried (i am a developer myself ;-) ) - i just tried it again, same behaviour.

i also switched language, as it usually is german, but also that does not have any effect. as well as changing the theme. no difference.

developer console as well says it is not "just not displayed", those buttons are really not there.

i will try one last thing: remove i via HACS and then check the filesystem if there are any leftovers.

[wills106]

I have just done a fresh HACS download on a separate machine and I can't replicate this.

Has HACS downloaded the plugin_sofar plugin_solax files etc?

[gelse]

after removal via HACS:

root@omv:~/links/homeassistant/config/custom_components# ls
alexa_media  edgeos  elasticsearch  hacs  monitor_docker  myjdownloader  openmediavault  spotcast  xiaomi_cloud_map_extractor  xiaomi_miot

then adding it via HACS:

root@omv:~/links/homeassistant/config/custom_components# ls
alexa_media  edgeos  elasticsearch  hacs  monitor_docker  myjdownloader  openmediavault  solax_modbus  spotcast  xiaomi_cloud_map_extractor  xiaomi_miot

and the content of solax_modbus seems fine to me.

[wills106]

I'm on HA 2022.12.5 I'll try 2022.12.7 later today in case they made a change.

[gelse]

fyi - i also tried to downgrade and install 2022.8 and i had the same behaviour.

i mean - i am quite sure that it is not really a bug of this integration but a bug of my setup somehow, but nevertheless i want to find the reason, as i am a bit dependent on the plugin ...

and as i said above - a fallback to configure it via YAML would be really great. not sure if that is even possible.

[gelse]

aaaand another information: i just tried to configure it with edge browser (normally: brave), which i NEVER used before to access home assistant at all, and i had the exact same behaviour. as a last resort i try to configure it via Android in a minute.

update: same behaviour on the mobile app, now i start to believe more and more that it is indeed an issue with the integration somehow.

[wills106]

I'll upgrade to HA 2022.12.7 in a bit to see if I can replicate the issue.

[gelse]

regarding logs - i set solax_modbus to debug in the config.

all i get in the logs are the following 3 lines:

2022-12-21 17:41:34.733 DEBUG (MainThread) [custom_components.solax_modbus] using pymodbus library 2.x
2022-12-21 17:41:34.749 INFO (MainThread) [custom_components.solax_modbus.config_flow] detected HA core version 2022 12
2022-12-21 17:41:34.749 INFO (MainThread) [custom_components.solax_modbus.config_flow] starting configflow - domain = solax_modbus

and i just even recreated the docker container (using lscr.io/linuxserver/homeassistant:latest) - no difference.

i am at the end of my ideas now.

but if that helps - i know python and can on-the-fly edit the integration, if you have any ideas what i could change to test.

[gelse]

My guess from reading the code: somehow the dynamic loading of the plugins introduced in 469fb8082bf0344ae5aa110c00c056b1e09a1717 does not work. perhaps it actually HAS something to do with the fact that i am running it via docker ? some weird permissions maybe? i try to tinker around a bit with that ...

[wills106]

My main system is Core in Docker on Unraid.

The other system I tried was HAOS on PI4

[gelse]

i added a few lines of debug messages, and indeed - it does not load any plugins:

2022-12-21 20:02:02.471 DEBUG (MainThread) [custom_components.solax_modbus.config_flow] Plugin path: custom_components/solax_modbus/plugin_*.py
2022-12-21 20:11:58.159 DEBUG (MainThread) [custom_components.solax_modbus.config_flow] glob: []
2022-12-21 20:02:02.471 DEBUG (MainThread) [custom_components.solax_modbus.config_flow] Loaded plugins: []

the lines i added are in config_flow.py: L59 (before the PLUGINS= ): _LOGGER.debug(f"Plugin path: {PLUGIN_PATH}") L60 (before the PLUGINS= ): _LOGGER.debug(f"glob: {glob.glob(PLUGIN_PATH)}") L63 (directly after the PLUGINS= ): _LOGGER.debug(f"Loaded plugins: {PLUGINS}")

to make the information complete:

root@omv:/data/compose/6/homeassistant/config/custom_components/solax_modbus# ls plugin_*.py
plugin_sofar_old.py  plugin_sofar.py  plugin_solax.py  plugin_solis_old.py  plugin_solis.py

all of them are readable by everyone

seems to me that somehow on my system glob does not work. wtf...

[wills106]

When you do ls -l do you have read only for the different groups other than owner? Or has HACS removed read only some how?

I'm just going to upgrade HA to 2022.12.7

[gelse]

it looks exactly that way, all of them are 644 - but the user is different, ofc.

root@omv:/data/compose/6/homeassistant/config/custom_components/solax_modbus# ls -aln plugin_*.py
-rw-r--r-- 1 1000 1000  38306 Dec 21 18:35 plugin_sofar_old.py
-rw-r--r-- 1 1000 1000  73836 Dec 21 18:35 plugin_sofar.py
-rw-r--r-- 1 1000 1000 142331 Dec 21 18:35 plugin_solax.py
-rw-r--r-- 1 1000 1000  17800 Dec 21 18:35 plugin_solis_old.py
-rw-r--r-- 1 1000 1000  53338 Dec 21 18:35 plugin_solis.py

that's because i am using

      - PUID=1000
      - PGID=1000

in the environment setting of the docker-compose file.

[gelse]

i just realized - PLUGIN_PATH is a relative path, i guess glob tries to find it from the CWD. what if the CWD is not what we expect ?

[gelse]

i guess that's it! i added the following:

_LOGGER.debug(f"Current directory: {os.getcwd()}")

and got the following in the logs:

Current directory: /run/s6-rc:s6-rc-init:hjCanB/servicedirs/svc-homeassistant

i then attached to the container and checked what the directory is:

root@acbf3661c008:/run/s6-rc:s6-rc-init:hjCanB/servicedirs/svc-homeassistant# ls
down  event  notification-fd  run  supervise

which is NOT the root of the config (it should be /config). my guess: there is some way to get the actual absolute path of the home-assistant configuration directory path - and that is missing here.

i guess the reason is because i am using the linuxserver.io docker image

[wills106]

Is your config folder mapped to a drive on your NAS or however you are running this? Or is it all stored in your docker image?

[gelse]

and so it is:

what i changed - and i admit it is a dirty hack that only works temporarily, but i think it should point you in the right direction:

in solax_modbus/const.py i changed the following lines:

DEFAULT_PLUGIN   = "/config/custom_components/solax_modbus/plugin_solax.py"
PLUGIN_PATH      = "/config/custom_components/solax_modbus/plugin_*.py"     

which in MY case is the correct location.

i did open heart surgery here, so this bug is NOT closed, but i think it should be straightforward to implement now.

[wills106]

This is how my setup is mapped /config <-> /mnt/user/appdata/Home-Assistant-Core

[gelse]

this has nothing to do with mapping of volumes of the docker container, as HA is running inside and it does not know of anything of the outside world.

this has to do with the special way linuxserver.io "simplifies" their images, as they always somehow change the running software for easier volume mappings.

the only way you can solve that (and i learned that the hard way in my job) - never rely on the working directory. NEVER.

[infradom]

I think the absolute path might also work on most installations /config/custom...

[infradom]

Just tested, and it seems to work with the absolute path on my HAOS I had to reinstall the integration however ...

[wills106]

If people don't remove the Integration prior to updating they are greeted with:

2022-12-21 20:43:25.462 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry SolaX for solax_modbus
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 372, in async_setup
    result = await component.async_setup_entry(hass, self)
  File "/config/custom_components/solax_modbus/__init__.py", line 95, in async_setup_entry
    plugin = importlib.import_module(f".plugin_{plugin_name}", 'custom_components.solax_modbus')
  File "/usr/local/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1004, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'custom_components.solax_modbus.plugin_'

If you try to reconfigure the Integration you loose the plugins.

Deleting and adding the Integration back again doesn't work.

It needs a full removal of the Integration from the Integration page and restarting HA and adding it back in again. For the path changes to take effect.

We'll have a look into a solution that doesn't cause all of the other users pain.

[gelse]

i did not have to reinstall when i changed the path, altho i HAD to restart HA. if i read the logfiles correctly, the integration, and thus the init.py, is loaded either on startup when it is configured, or during the first attempt to add a integration configuration.

which actually i just confirmed on my machine, because i made a few more changes and now it will work for every installation: in const.py: add this as first line:

import pathlib

change this

DEFAULT_PLUGIN   = "custom_components/solax_modbus/plugin_solax.py"
PLUGIN_PATH      = "custom_components/solax_modbus/plugin_*.py"

to this

DEFAULT_PLUGIN   = f"{pathlib.Path(__file__).parent.absolute()}/plugin_solax.py"
PLUGIN_PATH      = f"{pathlib.Path(__file__).parent.absolute()}/plugin_*.py"

whoever wants it can take credit. ;-)

as these changes still needs to be implemented, i will not yet close the issue - i will try to create a PR, have to set up the IDE/env first, so ... whoever gets it first, wins it. ;-)

[infradom]

Thanks, but I am still not sure this survives an upgrade, as the result path is probably stored persistently in the config. I will examine in more detail to see if we need to adapt the string read from config also at startup This way old and new style will remain compatible

[infradom]

And why not use absolute paths immediately? The problem of incompatibility lies in the config file entry that needs to be adapted ... I will check the config entry first so see which kind of path it contains in old and new

[infradom]

checkedd: The config entry plugin contains a long path in the old version, probably relative, in the new version: absolute 'plugin': '/config/custom_components/solax_modbus/plugin_solax.py' So when starting up init.py reads this path and determines the name withj const.py getPluginName's function, which probable strips the name in a wrong way...

So we need carefull testing, and I am not sure adding this pathlib library solves the problem (as we can work as well with absolute paths). But there are more small changes needed is what I believe. I will make a proposal ..

[gelse]

Thanks, but I am still not sure this survives an upgrade, as the result path is probably stored persistently in the config.

thats a good concern, but my patch would at least fix the problem for people who have a custom installation directory and/or a different CWD. It of course helps nothing in terms of when you change the file location of the plugins in an upgrade.

why not use absolute paths immediately?

well my patch does that, but it calculates the absolute path depending on the location of the current file, which should be correct, wherever the HA config says that the configuration files reside. Unless, of course, in some future version of the integration, you change the location of the plugin files. Then you would have to change that code as well, but that's just obvious.

as we can work as well with absolute paths

no we can't because there is no guarantee that the configuration directory is /config/

[infradom]

OK thanks, all I wanted to say was that the pathlib patch is probably not sufficient if we want our users to be able to upgrade seamlessly

[gelse]

yes, i agree with that.

if you want i can take a look into it - calculating the (root-)directory on startup and using that throughout the integration should be sufficient, but i would have to read into the details to find every place where the stored path is used.

i am tbh just a little surprised that there is not already some global variable provided by HA itself for those type of things.

[infradom]

I am working on a solution... the const.py getPluginName() function is probably not needed anymore after another change last weeks

[gelse]

actually i think it's the opposite. if i read that code in init.py correctly, it maps any path from the config to a relative filename via getPluginName.

    # ================== dynamically load desired plugin
    _LOGGER.debug(f"Ready to load plugin {config[CONF_PLUGIN]}")
    plugin_path = config[CONF_PLUGIN]
    if not plugin_path: _LOGGER.error(f"plugin path invalid, using default {DEFAULT_PLUGIN}; config dict: {config}")
    plugin_name = getPluginName(plugin_path)
    plugin = importlib.import_module(f".plugin_{plugin_name}", 'custom_components.solax_modbus') 
    if not plugin: _LOGGER.error(f"could not import plugin {plugin_name}")
    setPlugin(name, plugin)
    # ====================== end of dynamic load

i just wonder, why the plugin path is stored in the config in the first place, instead of just the plugin name.

[gelse]

ModuleNotFoundError: No module named 'custom_components.solaxmodbus.plugin'

oh my ... that means, that getPluginName returns empty. would be interesting, what plugin_path is in line 94

okok, i leave you working on it... ;-)

[infradom]

The modifications I am currently making make sure that the config entry for plugin just contains 'solax' or 'solis', not a path anymore. I do want to keep supporting the old path based entry for a while to avoid yet another breaking update.

[infradom]

I used your contribution in a new PR, but I also made the plugin-config independent of paths, and added code to support old and new config entries. Tested in an upgrade context and tested in a clean new install context Further tests are welcome (see my PR)

[wills106]

The PR closed this down automatically.

I'm just going to test the code change.

[wills106]

@gelse does 2022.12.14b1 Work fine for you before I release the stable version?

I have tested it on my Docker install and it works smoothly as a code update.

I have also tested it on HAOS as much as I can without connecting to an actual Inverter.

[gelse]

i was able to upgrade (to 2022.12.14b1) even without removing the old configuration, integration still works and the GUI also still works.

[gelse]

btw you have a typo in the markdown of the index page:

homsassistant-solax-modbus

[wills106]

i was able to upgrade (to 2022.12.14b1) even without removing the old configuration, integration still works and the GUI also still works.

Thanks, I'll close the issue down then.

[gelse]

but ... i just saw the release-notes, and i did not find the error message about the old style plugin name yet... i keep checking my logs nevermind, it's a warning, not an error. found it.

[wills106]

It will be changed in the stable release.

[gelse]

great, thanks a lot!

but i am feeling tempted to make a full code review like i would at work.... ... must ... resist ... the ... urge ... ;-)