search

mkaiser / Sungrow-SHx-Inverter-Modbus-Home-Assistant

Sungrow SH Integration for Home Assistant for SH3K6, SH4K6, SH5K-20, SH5K-V13, SH3K6-30, SH4K6-30, SH5K-30, SH3.RS, SH3.6RS, SH4.0RS, SH5.0RS, SH6.0RS, SH5.0RT, SH6.0RT, SH8.0RT, SH10RT, SH5.0RT-20, SH6.0RT-20, SH8.0RT-20, SH10RT-20, SH5.0RT-V112, SH6.0RT-V112, SH8.0RT-V112, SH10RT-V112, SH5.0RT-V122, SH6.0RT-V122, SH8.0RT-V122, SH10RT-V122, SH4.6R
328 stars 86 forks source link

Simplify configuration by removing need for secrets.yaml #8

Closed johschmitz closed 2 years ago

mkaiser commented 2 years ago

I am not sure if this really simplifies the configuration, assuming that nearly every user already has a secrets.yaml file. Was your error message in #6 related to a missing slave variable?

Every modbus registers requires a slave port, which causes many adoptions /changes when someone does not use the default slave port (like me). From my programmers view with maintenance in mind, having all user-customizable parameters outside the modbus_sungrow.yaml config file is the best option to support easy future updates just by copying the file and without the need of any custom changes.

Consequently I should also export the name at the beginning of the file to the secrets.yaml.

What do you think?

johschmitz commented 2 years ago

I believe the secrets.yaml is for credentials and not for configuration. So we are mixing up some things here. Is this assumption wrong?

mkaiser commented 2 years ago

I still have no idea on how to make this more intuitive. Sure, my current implementation mixes secrets and configuration. But I have not figured out a way to keep the generic solution using variables in the secrets.yaml and not having to use fixed values in the sungrow config file.

Any suggestions? Yaml is easy but does unfurtonately not deliver that flexiblity :/

johschmitz commented 2 years ago

For this simple case I don't see the need for a second yaml at all, just let the user edit the 2 or 3 things at the beginning of the modbus_sungrow.yaml and that's it. Why overcomplicate things and not just KISS?

mkaiser commented 2 years ago

For the Ip adress and the port this is not a problem. They can be stored in the secrets.yaml or at the top of the sungrow.yaml file.

The issue it the modbus slave address (e.g. "slave: 100"), which needs to be set in every modbus register description. This is the reason I put all three parameters as variables into the secret file.

johschmitz commented 2 years ago

The point here is that the slave ID is always the same for Sungrow. At least as far as I understand. So there is no variable needed and it can be just hard-coded in order to avoid all this trouble and KISS.

mkaiser commented 2 years ago

In my inverter the slave address is set to 100.

It does not look like there is a sufficient workaround, Therefore I will close this PR :/