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.

Sign up for GitHub

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

Jump to bottom

bugfixes and testing of asuswrt device_tracker #11915

Merged
merged 1 commit into from
Jan 26, 2018
Merged

bugfixes and testing of asuswrt device_tracker #11915

merged 1 commit into from
Jan 26, 2018

Conversation

kennedyshead
Copy link
Contributor

Description:

Fixes bugs with slow detecting of left devices.
Adding tests to check the component after latest patch.

Related issue (if applicable): fixes #11848

Checklist:

  • The code change is tested and works locally.
  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

pvizeli
pvizeli reviewed Jan 25, 2018
View reviewed changes
homeassistant/components/device_tracker/asuswrt.py Outdated
return devices
ret_devices = {}
for key in devices:
print(devices[key])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

@kennedyshead
Copy link
Contributor Author

It have to be tested in AP mode by some one.

@balloob
Copy link
Member

balloob commented Jan 26, 2018

Do we know anyone running ASUS Router in AP mode?

@kennedyshead
Copy link
Contributor Author

if not Ill test it locally

@balloob
Copy link
Member

balloob commented Jan 26, 2018

If you can, that would be great. We can get it merged in time for 0.62.

houndci-bot
houndci-bot reviewed Jan 26, 2018
View reviewed changes
tests/components/device_tracker/test_asuswrt.py Outdated
self.assertEqual(WAKE_DEVICES_AP, scanner.get_asuswrt_data())


@mock.patch(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many blank lines (2)

@kennedyshead
Copy link
Contributor Author

I have created a test, there really is only 1 place the AP mode is checked, I'm almost confident 100% that this test is all that where needed to test the functionality.

Tonight I'll test this with my router in AP, that will however just be a brief test with 1 or 2 devices and not a "real life" test

@pvizeli pvizeli added this to the 0.62 milestone Jan 26, 2018
@pvizeli
Copy link
Member

pvizeli commented Jan 26, 2018

Ready to merge after tests

@kennedyshead
Copy link
Contributor Author

If I have til tonight I'll cover all component in tests

@kennedyshead
Copy link
Contributor Author

87% Don't think I'll have time for more at this point :/ There are some left for telnet and some _LOGGER stuff in general, but the core is now covered

@kennedyshead
Copy link
Contributor Author

kennedyshead commented Jan 26, 2018 •
edited
Loading

100% 97% 99.13% Covered, Will get back with my manual test of AP in 2h

@MagnusKnutas
fixes #11848
a1b9b8b 
Adding tests to check the component after latest patch
@kennedyshead
Copy link
Contributor Author

AP mode works for me, ready to merge

@balloob balloob merged commit 2d8ef36 into home-assistant:dev Jan 26, 2018
@balloob
Copy link
Member

balloob commented Jan 26, 2018

Thanks! 🐬

balloob pushed a commit that referenced this pull request Jan 27, 2018
@kennedyshead @balloob
fixes #11848 (#11915)
280c160 
Adding tests to check the component after latest patch
@balloob balloob mentioned this pull request Jan 27, 2018
Merged
matemaciek pushed a commit to matemaciek/home-assistant that referenced this pull request Jan 27, 2018
@matemaciek-9ld
Merge remote-tracking branch 'upstream/master' into 9ld
4c60a3c 
* upstream/master: (465 commits)
  Update pyhomematic to 0.1.38 (home-assistant#11936)
  Implement Alexa temperature sensors (home-assistant#11930)
  Fixed rfxtrx binary_sensor KeyError on missing optional device_class (home-assistant#11925)
  Allow setting climate devices to AUTO mode via Google Assistant (home-assistant#11923)
  fixes home-assistant#11848 (home-assistant#11915)
  Add "write" service to system_log (home-assistant#11901)
  Update frontend to 20180126.0
  Version 0.62
  Allow separate command and state OIDs and payloads in SNMP switch (home-assistant#11075)
  Add ERC20 tokens to etherscan.io sensor (home-assistant#11916)
  Report scripts and groups as scenes to Alexa (home-assistant#11900)
  Minor fix to configuration validation and related log line. (home-assistant#11898)
  Multi-Room Support for Greenwave Reality (home-assistant#11364)
  Added Xeoma camera platform (home-assistant#11619)
  Improve foscam library exception support (home-assistant#11701)
  Iota wallet (home-assistant#11398)
  New venstar climate component (home-assistant#11639)
  Update python-wink version and multiple wink fixes/updates. (home-assistant#11833)
  Use API to discover Hue if no bridges specified (home-assistant#11909)
  Clarify emulated hue warning (home-assistant#11910)
  ...
@kennedyshead kennedyshead deleted the asuswrt_bugfix_and_tests branch February 4, 2018 16:16
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@pvizeli pvizeli pvizeli approved these changes

Assignees
No one assigned
Labels
cla-signed integration: asuswrt
Projects
None yet
Milestone
0.62
Development

Successfully merging this pull request may close these issues.

ASUSWRT: Device still home even if not here
6 participants
@kennedyshead @balloob @pvizeli @houndci-bot @homeassistant @MagnusKnutas