Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lots of unnecessarily required config fields #262

Open
werdnum opened this issue Apr 23, 2024 · 8 comments
Open

Lots of unnecessarily required config fields #262

werdnum opened this issue Apr 23, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@werdnum
Copy link
Contributor

werdnum commented Apr 23, 2024

Describe the bug
When I tried to start up EMHASS for the first time, I deleted all the parts of the configuration file that didn't apply (I have no solar and no battery). EMHASS refused to start up and/or do any optimisation until I added them all back.

This makes config migrations harder when you add new required options (as also happened later on)

To Reproduce
Run EMHASS with a configuration file that includes no parameters related to solar and batteries and try to do a forecast.

Expected behavior

EMHASS should ignore the absence of fields that are not relevant to the current configuration.

If EMHASS has required fields, it should throw a user friendly error on startup instead of KeyError on doing a forecast.

Screenshots
N/A

Home Assistant installation type
Container, but not relevant.

Your hardware
Linux Kubernetes arm64

EMHASS installation type
Container in Kubernetes

Additional context
Maybe worth using whatever HA uses for configuration validation?

@davidusb-geek
Copy link
Owner

When was this first time that you tried? There has been a lot of improvements for these types of issues thanks to the work of @GeoDerp earlier this year. There is now a treatment for setting default values when they are not specified by the user.
The configuration file in the add-on was also updated and rearranged for better treatment of optional and non-optional params. But there is probably more room for improvement. Although this is foe the add-on installation method and not the docker standalone, so that may not be completely irrelevant

@GeoDerp
Copy link
Contributor

GeoDerp commented May 1, 2024

When was this first time that you tried? There has been a lot of improvements for these types of issues thanks to the work of @GeoDerp earlier this year. There is now a treatment for setting default values when they are not specified by the user.
The configuration file in the add-on was also updated and rearranged for better treatment of optional and non-optional params. But there is probably more room for improvement. Although this is foe the add-on installation method and not the docker standalone, so that may not be completely irrelevant

Hey @werdnum, you probably have some good ideas, as @davidusb-geek already mentioned we have done some parameter defaulting in the add-on version of EMHASS.
Standalone so far we just assumed people left the defaults in. (not saying we can't change this)

If you like, try running the add-on version of EMHASS and see what you think. You will need to convert your parameters (including their respective parameters name changes) from the config.yaml to the options.json. (note you should be able to remove all the options.json parameters and it should still function). You will need to revert the config.yaml to it defaults in order for the add-on mode to work.

I'll update this comment Tomorrow with more details on this (currently writing this on bed with 1% battery life). But for now have a look at https://emhass.readthedocs.io/en/latest/develop.html#docker-run-add-on-via-with-local-files for docker run command difference, and https://emhass.readthedocs.io/en/latest/differences.html for the parameter name differences between standalone and add-on.
Hope that helped. 😁
Sorry in advance for all the spelling errors.

@GeoDerp
Copy link
Contributor

GeoDerp commented May 2, 2024

If you with to grab the image via a container registry: (not build the image locally)

  • EMHASS amd64 add-on: https://hub.docker.com/r/davidusb/image-amd64-emhass
  • Docker run example:
    docker run -it -p 5000:5000 --name emhass-container -v $(pwd)/options.json:/app/options.json  davidusb/image-amd64-emhass:latest 
    This assumes secrets are passed via options.json. Alternately, you can mount the secrets file also and EMHASS should reference it.
    docker run -it -p 5000:5000 --name emhass-container -v $(pwd)/options.json:/app/options.json  -v $(pwd)/secrets_emhass.yaml:/app/secrets_emhass.yaml davidusb/image-amd64-emhass:latest 

@werdnum
Copy link
Contributor Author

werdnum commented May 2, 2024

I'm not sure I understand the difference between the 'addon' and the regular version of EMHASS. I thought addons were just for HAOS, and I'm not using HAOS, so I am just using the normal version. Is the addon version better supported or something?

@GeoDerp
Copy link
Contributor

GeoDerp commented May 2, 2024

I'm not sure I understand the difference between the 'addon' and the regular version of EMHASS. I thought addons were just for HAOS, and I'm not using HAOS, so I am just using the normal version. Is the addon version better supported or something?

Thats correct, add-on was designed with HAOS in mind, It has better support for validating parameters. Standalone and Add-on mode aren't that mutch different in the way they operate besides the parameters.

In saying this this is the first time trying out the container via DockerHub and I seem to be running into some issues (I know why , i'm just not sure whats the best outcome ). Having a look at it now.

@werdnum
Copy link
Contributor Author

werdnum commented May 2, 2024

As a meta question, what's the point in having a separate standalone type if the add-on version doesn't actually require running under HAOS? Would it make sense to maintain only the add-on version, or at least to strongly encourage preferring it over the standalone one?

@GeoDerp
Copy link
Contributor

GeoDerp commented May 2, 2024

I think the best way to use add-on mode is via passing in the arguments via the standalone mode Image

docker run -it -p 5000:5000 --name emhass-container -v $(pwd)/options.json:/app/options.json davidusb/emhass-docker-standalone --addon true --no_response True

@GeoDerp
Copy link
Contributor

GeoDerp commented May 2, 2024

As a meta question, what's the point in having a separate standalone type if the add-on version doesn't actually require running under HAOS? Would it make sense to maintain only the add-on version, or at least to strongly encourage preferring it over the standalone one?

Good question. The one main difference is that add-on mode calls HA via its local API to gain location and timezone information.
In reality, it wouldn't be impossible to create an alternative method to check if its running in HAOS (to use the local API or not). Then standalone and add-on could be merged.
You could ask it to check via If the Local API supervisor key exists.

Since everyone is already using one or the other. This may be a good change to implement on a large version release. And it may be good to make sure legacy users are not impacted.
PS. Up to recently, there has been more differences between the two. We have been slowly merging them together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants