core icon indicating copy to clipboard operation
core copied to clipboard

Add photovoltaic sensors to ViCare integration

Open CFenner opened this issue 3 months ago • 11 comments

Proposed change

This adds further photovoltaic sensors for weekly, monthly, yearly and total solar production to ViCare.

Type of change

  • [ ] Dependency upgrade
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [x] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue: #91417
  • Link to documentation pull request:

Checklist

  • [x] The code change is tested and works locally.
  • [x] Local tests pass. Your PR cannot be merged unless tests pass
  • [x] There is no commented out code in this PR.
  • [x] I have followed the development checklist
  • [x] I have followed the perfect PR recommendations
  • [x] The code has been formatted using Ruff (ruff format homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • [ ] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

CFenner avatar Mar 17 '24 06:03 CFenner

@nero2465 @CHTHSCH please have a look.

CFenner avatar Mar 17 '24 06:03 CFenner

@nero2465 @CHTHSCH please have a look.

Last programming language learnt was FORTRAN 😂 - do not expect too much from me reviewing code.

However what I have looked at looks ok and fit for purpose.

CHTHSCH avatar Mar 17 '24 06:03 CHTHSCH

Could you also test in your system?

CFenner avatar Mar 17 '24 07:03 CFenner

Could you also test in your system?

Will do, but need the correct curl command line. I used this

cd /config curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d vicare -p 106600

and restarted HA. I did not see any new entities. I guess I need to use another command.

CHTHSCH avatar Mar 17 '24 08:03 CHTHSCH

Yes, the last parameter -p needs to match the number of this PR, so -p 113664.

CFenner avatar Mar 17 '24 08:03 CFenner

Done, but I get an "Fehler beim Einrichten", no entity displayed. ViCare integration not started.

CHTHSCH avatar Mar 17 '24 09:03 CHTHSCH

I See this in the logs on my side.

Bildschirmfoto 2024-03-17 um 13 38 51

Looks like there have been some changes to the general config flow in the dev version that are new to our last HA releases.

CFenner avatar Mar 17 '24 12:03 CFenner

I See this in the logs on my side.

Looks like there are some changes in the dev version that are incompatible with the last HA release.

I can install again and create some logfiles if you want. Just need an instruction what and how to do

CHTHSCH avatar Mar 17 '24 12:03 CHTHSCH

You need to change the content of the /vicare/config_flow.py with the one from this file: https://raw.githubusercontent.com/home-assistant/core/e06446d0fa0ad7b13482b3e0e50959a02f3e2299/homeassistant/components/vicare/config_flow.py

CFenner avatar Mar 17 '24 14:03 CFenner

Thanks for the effort and the hint. It works. See screenshot

Success

CHTHSCH avatar Mar 17 '24 14:03 CHTHSCH

Will this progress further and if so when?

CHTHSCH avatar Apr 27 '24 16:04 CHTHSCH