emhass icon indicating copy to clipboard operation
emhass copied to clipboard

Standalone and Add-on merge

Open GeoDerp opened this issue 1 year ago β€’ 14 comments

Related to https://github.com/davidusb-geek/emhass/issues/269

GeoDerp avatar Aug 02 '24 12:08 GeoDerp

~~This is still in early development~~

It's here to show progress. A lot has changed, and a lot will still need to be. Will edit this comment as the PR progresses.

Edits:

  • Merged/Simplified EMHASS/Add-on Dockerfile
  • Added functionality to auto detect EMHASS on HAOS (choose to whether to use Supervisor API)
  • Suggested edit to migrate container registry from DockerHub to Github Projects. (Similar to Home Assistants, should functionally be the same) @davidusb-geek feel free to comment on this.
  • Simplified EMHASS-Add-on Github repo to bare minimum: https://github.com/davidusb-geek/emhass-add-on/pull/100
    • Copies the docker image from the EMHASS container registry
  • Simplified build_params with associations (similar to https://github.com/davidusb-geek/emhass/pull/181)
  • Dev Container now built from the main Dockerfile
  • Added dedicated function for checking parameters against deferrable load amount

Parameter / Config changes:

  • New configuration page has been added to the webserver to modify and save to config.json
  • config.json file has been added as the new config (centralized config for all users)
    • options.json is now only used for secrets (HAOS version of secrets_emhass.yaml)
      • config parameter naming conventions/definitions almost identical to options.json
        • EMHASS Add-on users can copy and past their existing json into the configuration page to generate the updated config.json file.
    • Removed config_emhass.yaml
    • Support for config_emhass.yaml still exists under legacy_config_path and will convert the parameter definitions for you (press save on the configuration page to save to config.json)
  • config_defaults.json added:
    • test reference
    • default state on configuration page
    • missing parameters in config.json (adding some support for optional params)

TODO:

  • [x] Finalize EMHASS HAOS auto detect
    • [x] Test HAOS auto detect
  • [x] Update all documentation to match parameter definitions of config.json
    • [x] EMHASS
    • [x] EMHASS-add-on
  • [ ] Update all documentation to match the new method to run, dev & test EMHASS
    • [ ] EMHASS
    • [ ] EMHASS-add-on
  • [x] Modify GitHub Actions to publish images to github container registry
    • [x] EMHASS
    • [x] EMHASS-add-on
      • Example: https://github.com/GeoDerp/emhass/pkgs/container/emhass
  • [x] Plan and implement parameter defaults location
  • [x] Update unittests
    • [ ] Update unittest to test new functionality
  • [ ] Thorough Test EMHASS/EMHASS-Add-on functionality
    • [ ] Thorough Test configuration functionality
  • [x] Finalize build_params associations
  • [x] Fix functionality of Dev Container

Secret and Config Example

EMHASS methods

configuration page

List view example

Screenshot 2024-09-03 at 19-16-27 EMHASS Energy Management Optimization for Home Assistant

Box view example

Screenshot 2024-09-03 at 19-16-40 EMHASS Energy Management Optimization for Home Assistant

GeoDerp avatar Aug 02 '24 12:08 GeoDerp

I am currently unsure how we should approach the parameter naming conventions moving forward. This PR is currently set up with the EMHASS code as the original parameter naming convention, and the options.json the add-on naming conversation (options.json being converted in build_parms with the associations list. However, we could also change all the parameters in the options.json to match the original. Moreover, we could change the code to match the options.json parameter naming. Either way I'm planning on making a script that will help users to convert from one to the other for an easier transition.

GeoDerp avatar Aug 03 '24 00:08 GeoDerp

It would be probably best to change the code to match the options.json, those names are way more explicit

davidusb-geek avatar Aug 03 '24 08:08 davidusb-geek

work in progress In the process of creating an configuration page. Will explain the changes once this has been uploaded.

GeoDerp avatar Aug 18 '24 06:08 GeoDerp

Screenshot 2024-08-23 at 16-21-28 EMHASS Energy Management Optimization for Home Assistant Slowly getting there.

GeoDerp avatar Aug 22 '24 13:08 GeoDerp

bulk of configuration page done. Just need to test and add redundancy. Later Ill make a video and/or some diagrams the explain the approaches this PR aims, for running EMHASS in docker/Home Assistant.

I haven't added any unitests, changed any documentation or done any serious tests yet. Once I have release the media explaining this PR any, help would be appreciated.

GeoDerp avatar Aug 27 '24 12:08 GeoDerp

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

The hope is to do some testing then create a video tomorrow showing the progress and how this PR works. Im also hoping that we can find some people that can help with things like testing, unitest of new functionality and refreshing the documentation.

GeoDerp avatar Aug 30 '24 11:08 GeoDerp

Codecov Report

Attention: Patch coverage is 60.55227% with 200 lines in your changes missing coverage. Please review.

Project coverage is 68.33%. Comparing base (207711f) to head (81d0182). Report is 26 commits behind head on master.

Files with missing lines Patch % Lines
src/emhass/web_server.py 0.00% 89 Missing :warning:
src/emhass/utils.py 73.23% 72 Missing :warning:
src/emhass/command_line.py 67.44% 28 Missing :warning:
src/emhass/optimization.py 80.00% 7 Missing :warning:
src/emhass/forecast.py 85.71% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #334      +/-   ##
==========================================
+ Coverage   67.84%   68.33%   +0.48%     
==========================================
  Files           8        8              
  Lines        2709     2814     +105     
==========================================
+ Hits         1838     1923      +85     
- Misses        871      891      +20     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 01 '24 11:09 codecov[bot]

Greetings @davidusb-geek, hope you are doing well.

Here is a video for an overview of the PR.

If you approve, I would appreciate it if you could reply toΒ this post with a thumbs up. Then ill send the vid to LinkedIn & Community form.

The video isn't amazing, but I hope it demonstrates the PR well enough. Regards.

EMHASS - PR #334 - Standalone and Add-on merge

GeoDerp avatar Sep 08 '24 15:09 GeoDerp

Hi @GeoDerp, I've just now had the time to check on all this. I just saw the video. Great work as always this will be really helful as you said in the introductory part of the video. To be honest I totally understood the motivation but did not had the time until now to really understand the work that you have done. That graph summarize it very well and it is just great to have just one unified interface to input the configuration. So big thumbs up for all this.

davidusb-geek avatar Sep 08 '24 18:09 davidusb-geek

No problem for me about migrating to github registry.

davidusb-geek avatar Sep 08 '24 18:09 davidusb-geek

One thing on my side will be to completely understand the workflow of something that happens all the time, which is the need to add a new controlling parameter in the configuration. So with all these changes I will to get used to this new workflow and understad what is needed to add (or remove) a new parameter

davidusb-geek avatar Sep 08 '24 18:09 davidusb-geek

One thing on my side will be to completely understand the workflow of something that happens all the time, which is the need to add a new controlling parameter in the configuration. So with all these changes I will to get used to this new workflow and understad what is needed to add (or remove) a new parameter

Ah yeah good point, So the steps of appending a new parameter would be:

Example parameter = this_parameter_is_amazing

Append a line into associations.csv : So that build_params() knows what config catagorie to allocate the parameter

...
retrieve_hass_conf,,this_parameter_is_amazing
  • Alternatively if you want to support this parameter with the yaml conversion (Ie. allow the parameter to be converted from config_emhass.yaml)
    ...
    retrieve_hass_conf,his_parameter_is_amazing,this_parameter_is_amazing
    

Append a line into the config_defaults.json To set a default value for the user if none is provided in config.json

"...": "...",
  "this_parameter_is_amazing": [
    0.1,
    0.1
  ]

Lastly, to support the configuration website to generate the parameter in the list view, append the param_definitions.json file:

"this_parameter_is_amazing": {
      "friendly_name": "This parameter is amazing",
      "Description": "This parameter functions as you expect. It makes EMHASS AMAZING!",
      "input": "array.float",
      "default_value": 0.777
    }

Note: The default_value in this case acts (or should act) as last resort fallback if default_config.json is not found. It also acts as the default value when you append (press plus) to an array.* parameter

Screenshot from 2024-09-09 16-45-32

GeoDerp avatar Sep 09 '24 06:09 GeoDerp

Thanks for that detailed answer. It seems clear for now but of course I will need to test that workflow. I'll let you tell me when this PR is ready to merge. And thank again for this! This will solve a lot of the difficulties when configuring EMHASS for the first time ;-)

davidusb-geek avatar Sep 17 '24 18:09 davidusb-geek

Hi, from the TODO list above I understand that there are still some tests to be implemented + documentation, is that right?

davidusb-geek avatar Oct 04 '24 10:10 davidusb-geek

Also CodeQL is complaining, I'm not sure if we should dismiss those

davidusb-geek avatar Oct 04 '24 10:10 davidusb-geek

Hi, from the TODO list above I understand that there are still some tests to be implemented + documentation, is that right?

That's correct. The unittests aren't necessary ( more for code cover) but the documentation kinda need a rewrite. Focusing on the new methods of running EMHASS and the parameter naming conventions.

GeoDerp avatar Oct 04 '24 10:10 GeoDerp

Also CodeQL is complaining, I'm not sure if we should dismiss those

I'll have a re-review of them, from my previous observations the complaints aren't anything new, from how EMHASS used to operate. I just change the methods of calling the secrets.

GeoDerp avatar Oct 04 '24 11:10 GeoDerp

Ill see how mutch documentation I can complete tonight. Probably mainly focus on the parameter conversion. With the documentation to run EMHASS to do later be me or someone else.

GeoDerp avatar Oct 05 '24 07:10 GeoDerp

Hi @GeoDerp, should we proceed to merge or not yet?

davidusb-geek avatar Oct 22 '24 20:10 davidusb-geek

Hi @GeoDerp, should we proceed to merge or not yet?

@davidusb-geek I'll see if I can smash out the bulk of the documentation today. Then we can merge and fine-tune it later. πŸ‘

GeoDerp avatar Oct 22 '24 20:10 GeoDerp

Alright, I believe the bulk of the documentation is complete (could be merged now). Would likely need to refine it later. I didn't manage to get to the Emhass-Add-on docs yet. Will hopefully get some time tomorrow to complete the Add-on.

GeoDerp avatar Oct 23 '24 12:10 GeoDerp

Ok I will wait for you to complete the add-on docs part and then merge

davidusb-geek avatar Oct 24 '24 15:10 davidusb-geek

Ok I will wait for you to complete the add-on docs part and then merge

@davidusb-geek , this should be done by the end of today πŸ‘

GeoDerp avatar Oct 24 '24 20:10 GeoDerp

Bulk of the Add-on is done πŸ‘ (can tweak it later). Trying to work out these test failing now.

GeoDerp avatar Oct 25 '24 08:10 GeoDerp

Looks like one of those tests that fails randomly. Maybe related to daylight saving time πŸ€”

GeoDerp avatar Oct 25 '24 08:10 GeoDerp

@davidusb-geek, could you have a look at f754cf9 , this is just my simple "fix" for the DST issue. However I wouldn't be surprised if this is implemented wrong and snuffles the time incorrectly. (Feel free to revert) Besides that feel free to test both repos and merge.

GeoDerp avatar Oct 25 '24 10:10 GeoDerp

@davidusb-geek, could you have a look at f754cf9 , this is just my simple "fix" for the DST issue. However I wouldn't be surprised if this is implemented wrong and snuffles the time incorrectly. (Feel free to revert) Besides that feel free to test both repos and merge.

Maybe we can check but at the very beginning of the code all timestamps were handled like you have done, using UTC, which seems to me as the best option to deal with DST changes. But then this was shifted to other methods after failures on DST events. That's an open issue so let's try this and see how it goes, I have reviewed your changes an it seems all logic to me.

I will merge and test. If everything goes fine I will release a new version hopefully solving DST failures. The DST change is due for tomorrow night!

davidusb-geek avatar Oct 25 '24 18:10 davidusb-geek

Amazing! Let me know if you need any clarification on anything, or need me to fix something. πŸ‘

GeoDerp avatar Oct 25 '24 19:10 GeoDerp