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

Solax Inverter Sensor Component #22579

Merged
merged 15 commits into from May 26, 2019

Conversation

squishykid
Copy link
Contributor

@squishykid squishykid commented Mar 31, 2019

Description:

This component adds support for Solax solar inverters. These inverters may be connected to a home Wi-Fi network and expose a REST endpoint. This component polls that endpoint and exposes the information as sensor data to home-assistant.

Future pull requests will add support for exposing specific attributes, but I want to keep the first contribution small 😊.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#9082

Example entry for configuration.yaml (if applicable):

- platform: solax
  ip_address: '192.168.0.123'

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.

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

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

  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@homeassistant
Copy link
Contributor

Hi @squishykid,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@awarecan
Copy link
Contributor

awarecan commented Apr 4, 2019

It is harsh time for new integration as we are doing refactoring, we are now requesting all components has to have a manifest file.

Please rebase you branch against current dev branch, and create a manifest.json file under your component's folder.

You may need to do more rebase in the next few days as we are still in the progress of refactoring.

@squishykid
Copy link
Contributor Author

Ok, thanks for the info. I might wait I a few days before continuing work on this then.

@codecov

This comment has been minimized.

@squishykid
Copy link
Contributor Author

@rohankapoorcom @awarecan Review comments and manifest have been addressed, should be ready to merge now 😊

@rohankapoorcom
Copy link
Member

@squishykid A couple more (minor) comments, otherwise this is looking good!

@codecov
Copy link

codecov bot commented May 6, 2019

Codecov Report

Merging #22579 into dev will decrease coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #22579      +/-   ##
==========================================
- Coverage   93.89%   93.83%   -0.06%     
==========================================
  Files         464      448      -16     
  Lines       37877    36583    -1294     
==========================================
- Hits        35566    34329    -1237     
+ Misses       2311     2254      -57
Impacted Files Coverage Δ
homeassistant/components/mqtt/fan.py 74.33% <0%> (-23.64%) ⬇️
homeassistant/components/mqtt/light/schema_json.py 73.06% <0%> (-20.65%) ⬇️
homeassistant/bootstrap.py 58.08% <0%> (-17.21%) ⬇️
homeassistant/components/mqtt/lock.py 91.83% <0%> (-8.17%) ⬇️
homeassistant/components/axis/device.py 93.02% <0%> (-6.98%) ⬇️
homeassistant/components/hassio/ingress.py 71.3% <0%> (-6.75%) ⬇️
homeassistant/helpers/template.py 95.54% <0%> (-4.46%) ⬇️
homeassistant/components/deconz/config_flow.py 97.77% <0%> (-2.23%) ⬇️
homeassistant/components/zha/core/registries.py 85% <0%> (-1.96%) ⬇️
homeassistant/components/hue/bridge.py 73.07% <0%> (-1%) ⬇️
... and 202 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3925b7...ca43123. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented May 6, 2019

Codecov Report

Merging #22579 into dev will decrease coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #22579      +/-   ##
==========================================
- Coverage   93.89%   93.83%   -0.06%     
==========================================
  Files         464      448      -16     
  Lines       37877    36583    -1294     
==========================================
- Hits        35566    34329    -1237     
+ Misses       2311     2254      -57
Impacted Files Coverage Δ
homeassistant/components/mqtt/fan.py 74.33% <0%> (-23.64%) ⬇️
homeassistant/components/mqtt/light/schema_json.py 73.06% <0%> (-20.65%) ⬇️
homeassistant/bootstrap.py 58.08% <0%> (-17.21%) ⬇️
homeassistant/components/mqtt/lock.py 91.83% <0%> (-8.17%) ⬇️
homeassistant/components/axis/device.py 93.02% <0%> (-6.98%) ⬇️
homeassistant/components/hassio/ingress.py 71.3% <0%> (-6.75%) ⬇️
homeassistant/helpers/template.py 95.54% <0%> (-4.46%) ⬇️
homeassistant/components/deconz/config_flow.py 97.77% <0%> (-2.23%) ⬇️
homeassistant/components/zha/core/registries.py 85% <0%> (-1.96%) ⬇️
homeassistant/components/hue/bridge.py 73.07% <0%> (-1%) ⬇️
... and 202 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3925b7...ca43123. Read the comment docs.

@squishykid
Copy link
Contributor Author

@rohankapoorcom should be ready to go again. Attempt 2!

@rohankapoorcom
Copy link
Member

Sorry for the delay, we're good to go here!

@rohankapoorcom rohankapoorcom merged commit 7959c04 into home-assistant:dev May 26, 2019
@balloob balloob mentioned this pull request Jun 4, 2019
@MJP-76
Copy link

MJP-76 commented Jun 6, 2019

Nice work here. Let me know if you need any testing. I have two Solax inverters, 1 x Hybrid with Batteries and 1 x X1 inverter

Happy to do beta testing etc

@kprzeb
Copy link

kprzeb commented Jun 14, 2019

Hello Is this sensor working witch solax X3 inverter ?

@squishykid squishykid deleted the solax-inverter branch June 14, 2019 23:39
@squishykid
Copy link
Contributor Author

@kprzeb Give it a go and let us know if it works with your inverter.

I have tested it with a Solax SK-TL5000E

@MJP-76
Copy link

MJP-76 commented Jun 18, 2019

I use a Solax X1 SK-SU5000E and Solax X1-2.5-S-D (X1-Air) and both work

@kprzeb
Copy link

kprzeb commented Jun 20, 2019

Sorry for delay
I have SolaX Power X3-8.0 I connect it to my WiFi network and add HA sensor:

- platform: solax
  ip_address: '192.168.7.15'

HA dont read any values. IP is correct.
On host 192.168.7.15 I cant see any web interface, pings replay, port 80 is open
Can I help develop this problem ?
What can I check?
Thenks

@MartinHjelmare
Copy link
Member

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jun 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants