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

Add optimizer data to solaredge_local #26708

Merged
merged 38 commits into from Sep 21, 2019

Conversation

@scheric
Copy link
Contributor

commented Sep 18, 2019

Description:

Added sensors for optimizer data.

  • Average optimizer temperature
  • Average optimizer input voltage
  • Average optimizer input current
  • Average optimizer power (calculated with the above averages)

Added sensor for inverter temperature.

Info

This is all the data received by the API

maintenance.log
power_control.log
status.log
information.log

My to-do list:

  • Update lib.
  • Add sensors.
  • Add debug info.
  • Add proper units.
  • Fix Cl.
  • Waiting for feedback.

What to test

  • What happens when no optimizes are connected.
  • What happens when inverter is offline.
  • What happens when something went wrong during setup.
  • What happens when more then one inverter connected.
  • What happens to the optimizer voltage when it is night.

Problems

The solaredge-local api only gives current input in integers.

help needed for

Possible stuff people want to see.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.
scheric added 9 commits Sep 18, 2019
@MartinHjelmare MartinHjelmare moved this from Needs review to Incoming in Dev Sep 18, 2019
scheric added 15 commits Sep 18, 2019
@scheric scheric requested a review from MartinHjelmare Sep 20, 2019
@scheric

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2019

Should the units for current and voltage be added in const.py

scheric added 2 commits Sep 20, 2019
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

We shouldn't add units in homeassistant/const.py in this PR. That could be done in another PR if there are many integrations that use the same units.

Copy link
Member

left a comment

Looks good!

Dev automation moved this from Review in progress to Reviewer approved Sep 20, 2019
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

Is it ready to merge?

@scheric

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2019

Not yet program fails when there was a bad connection to the inverter.
If that happens everything goes to 0.

@scheric

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2019

Not a functional problem but would anyone hate having these large numbers or should i round the values.
large numbers

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

We usually want to present the data as is from the device, but if the device sends data that isn't following best practices regarding significant figures, it's ok to round. Ie we shouldn't present numbers with false precision.

scheric added 9 commits Sep 20, 2019
@scheric

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2019

There are no errors anymore when the inverter is not available.
There are no errors when the sun is under.

@scheric

This comment has been minimized.

Copy link
Contributor Author

commented Sep 21, 2019

I think it is ready to be merged

This is the final result:
final result

@scheric scheric requested a review from MartinHjelmare Sep 21, 2019
Copy link
Member

left a comment

Good!

@MartinHjelmare MartinHjelmare merged commit 9d0cb89 into home-assistant:dev Sep 21, 2019
11 checks passed
11 checks passed
CI Build #20190920.50 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing 886d8bd...0d1435a
Details
codecov/project 94.04% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Sep 21, 2019
@lock lock bot locked and limited conversation to collaborators Sep 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
4 participants
You can’t perform that action at this time.