Skip to content

Add asuswrt interface and dnsmasq location configuration#29834

Merged
springstan merged 4 commits intohome-assistant:devfrom
pkishino:asusfix
Feb 21, 2020
Merged

Add asuswrt interface and dnsmasq location configuration#29834
springstan merged 4 commits intohome-assistant:devfrom
pkishino:asusfix

Conversation

@pkishino
Copy link
Copy Markdown
Contributor

@pkishino pkishino commented Dec 11, 2019

Description:

This adds two new parameters to asuswrt configuration to specify dnsmasq location and interface to use for statistics.
This fixes the issue with padavan n56u-project sensors not reading statistics as well as having to create a soft link to dnsmasq file.

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

Example entry for configuration.yaml (if applicable):

# Example configuration.yaml entry
asuswrt:
  host: YOUR_ROUTER_IP
  username: YOUR_ADMIN_USERNAME
  ssh_key: /config/id_rsa
  interface: 'eth0'
  dnsmasq: '/var/lib/misc'
  sensors:
    - upload
    - download
    - upload_speed
    - download_speed

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 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. 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.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@probot-home-assistant
Copy link
Copy Markdown

Hey there @kennedyshead, mind taking a look at this pull request as its been labeled with a integration (asuswrt) you are listed as a codeowner for? Thanks!

@MartinHjelmare MartinHjelmare changed the title Added interface and dnsmasq location configuration Add asuswrt interface and dnsmasq location configuration Jan 7, 2020
@pkishino pkishino marked this pull request as ready for review February 3, 2020 09:29
@pkishino
Copy link
Copy Markdown
Contributor Author

pkishino commented Feb 3, 2020

aioasuswrt has been updated to 1.2.1 to include this update

@pkishino
Copy link
Copy Markdown
Contributor Author

pkishino commented Feb 3, 2020

not sure why it isn't finding the 1.2.1 release:
https://pypi.org/project/aioasuswrt/1.2.1/

@kennedyshead
Copy link
Copy Markdown
Contributor

not sure why it isn't finding the 1.2.1 release:
https://pypi.org/project/aioasuswrt/1.2.1/

That is strange ?!

Comment thread homeassistant/components/asuswrt/__init__.py Outdated
@springstan springstan added the dependency Pull requests marked as a dependency upgrade label Feb 3, 2020
@springstan
Copy link
Copy Markdown
Member

not sure why it isn't finding the 1.2.1 release:
https://pypi.org/project/aioasuswrt/1.2.1/

That is strange ?!

@kennedyshead I can confirm that downloading aioasuswrt==1.2.1 fails locally as well.

@kennedyshead
Copy link
Copy Markdown
Contributor

1.2.2 works, I just installed it locally... Have no idea why 1.2.0 and 1.2.1 fails

Comment thread homeassistant/components/asuswrt/__init__.py Outdated
Comment thread homeassistant/components/asuswrt/__init__.py Outdated
@pkishino pkishino requested a review from a team as a code owner February 4, 2020 10:48
@pkishino pkishino force-pushed the asusfix branch 3 times, most recently from cc13b6b to e2d3bed Compare February 4, 2020 11:33
Comment thread homeassistant/components/asuswrt/__init__.py Outdated
@springstan
Copy link
Copy Markdown
Member

Please add a test to reach the code coverage difference for this patch :)

@pkishino pkishino force-pushed the asusfix branch 3 times, most recently from d885af5 to 658bf0c Compare February 7, 2020 06:26
@pkishino pkishino force-pushed the asusfix branch 4 times, most recently from 40fd429 to 835532b Compare February 7, 2020 07:36
@pkishino
Copy link
Copy Markdown
Contributor Author

pkishino commented Feb 7, 2020

Please add a test to reach the code coverage difference for this patch :)

Yeah..trying to fix this but it is complaining about a Lint issue..I have fixed it and locally Lint passes fine, but when I push it to CI it fails...not sure how to fix this

Comment thread tests/components/asuswrt/test_device_tracker.py Outdated
@springstan springstan removed the request for review from a team February 7, 2020 20:02
@springstan
Copy link
Copy Markdown
Member

@pkishino I am not quite sure but has the codecoverage increased by adding this new test? It looks good but I think you will need to add another test that covers the interface as well.

@pkishino pkishino force-pushed the asusfix branch 2 times, most recently from 8328ec5 to 21db624 Compare February 8, 2020 08:07
@pkishino
Copy link
Copy Markdown
Contributor Author

pkishino commented Feb 8, 2020

@pkishino I am not quite sure but has the codecoverage increased by adding this new test? It looks good but I think you will need to add another test that covers the interface as well.

Hey,
not quite sure how to bump the coverage up to pass..tried looking at other test cases and such..
@kennedyshead suggestions?

@kennedyshead
Copy link
Copy Markdown
Contributor

@pkishino I'll have a look see

@kennedyshead
Copy link
Copy Markdown
Contributor

If you bump the tests for sensor.py I think you are done ;)

@pkishino
Copy link
Copy Markdown
Contributor Author

@kennedyshead yeah, can take a look at that but as there currently are 0 tests for sensor not sure where to start and exactly what needs to be covered there..

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 21, 2020

Codecov Report

Merging #29834 into dev will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #29834      +/-   ##
==========================================
+ Coverage   94.62%   94.73%   +0.11%     
==========================================
  Files         749      766      +17     
  Lines       54193    55568    +1375     
==========================================
+ Hits        51279    52644    +1365     
- Misses       2914     2924      +10     
Impacted Files Coverage Δ
homeassistant/components/demo/config_flow.py 62.85% <0.00%> (-37.15%) ⬇️
homeassistant/__init__.py 89.02% <0.00%> (-6.81%) ⬇️
homeassistant/components/google_assistant/http.py 79.16% <0.00%> (-6.67%) ⬇️
homeassistant/components/light/device_action.py 93.93% <0.00%> (-6.07%) ⬇️
homeassistant/components/hue/bridge.py 72.07% <0.00%> (-3.42%) ⬇️
homeassistant/components/zha/cover.py 93.93% <0.00%> (-2.03%) ⬇️
homeassistant/util/logging.py 95.83% <0.00%> (-1.61%) ⬇️
...omponents/homematicip_cloud/alarm_control_panel.py 98.52% <0.00%> (-1.48%) ⬇️
homeassistant/components/samsungtv/config_flow.py 98.24% <0.00%> (-0.75%) ⬇️
homeassistant/components/uk_transport/sensor.py 93.47% <0.00%> (-0.73%) ⬇️
... and 118 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 eb266d2...50e5e3c. Read the comment docs.

Comment thread tests/components/asuswrt/test_device_tracker.py Outdated
Comment thread tests/components/asuswrt/test_sensor.py Outdated
@pkishino
Copy link
Copy Markdown
Contributor Author

oh come on...this is nonsense...

@springstan
Copy link
Copy Markdown
Member

Please remove the following line which omits the specified file from code coverage testing:
https://github.com/home-assistant/home-assistant/blob/dd8597cb46c5e788995e1ea56b87dfbaf43ca51c/.coveragerc#L61

@springstan
Copy link
Copy Markdown
Member

springstan commented Feb 21, 2020

@pkishino no worries such a small difference is okay 😉

@kennedyshead please take a look again to approve this PR if everything is fine :)

@kennedyshead
Copy link
Copy Markdown
Contributor

Looks fine to me :)

@springstan springstan merged commit f32411e into home-assistant:dev Feb 21, 2020
@lock lock Bot locked and limited conversation to collaborators Feb 22, 2020
@pkishino pkishino deleted the asusfix branch February 28, 2020 12:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed dependency Pull requests marked as a dependency upgrade integration: asuswrt small-pr PRs with less than 30 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants