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

Async version for asuswrt #17692

Merged
merged 2 commits into from Oct 23, 2018

Conversation

Projects
None yet
7 participants
@kennedyshead
Contributor

kennedyshead commented Oct 22, 2018

Description:

This is a new async-version for asuswrt.

It will hopefully fix a lot of other errors with duplicate ssh-channels as well

Related issue (if applicable):
fixes #13723, fixes #17063 fixes #16887 fixes #13757 fixes #13351 fixes #10568

Checklist:

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

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.

@kennedyshead kennedyshead force-pushed the kennedyshead:async_asuswrt branch from 170c8c3 to d0fccc5 Oct 22, 2018

@balloob

This comment has been minimized.

Member

balloob commented Oct 22, 2018

Could you extract it into a small lib that we can depend on instead?

@kennedyshead

This comment has been minimized.

Contributor

kennedyshead commented Oct 22, 2018

Yes I might be able to, do you mean a smaller ssh-lib or a small asuswrt-lib? @balloob

@@ -14,7 +14,7 @@
CONF_AWAY_HIDE)
from homeassistant.components.device_tracker.asuswrt import (
CONF_PROTOCOL, CONF_MODE, CONF_PUB_KEY, DOMAIN, _ARP_REGEX,
CONF_PORT, PLATFORM_SCHEMA, Device, get_scanner, AsusWrtDeviceScanner,
CONF_PORT, PLATFORM_SCHEMA, Device, async_get_scanner, AsusWrtDeviceScanner,

This comment has been minimized.

@houndci-bot

houndci-bot Oct 22, 2018

line too long (80 > 79 characters)

@@ -152,237 +100,9 @@ def _update_info(self):
return False

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 22, 2018

Member

Nothing is checking this return value.

@@ -152,237 +100,9 @@ def _update_info(self):
return False
_LOGGER.info('Checking Devices')
data = self.get_asuswrt_data()
data = await self.connection.async_get_connected_devices()
if not data:
return False

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 22, 2018

Member

See above.

@@ -152,237 +100,9 @@ def _update_info(self):
return False
_LOGGER.info('Checking Devices')
data = self.get_asuswrt_data()
data = await self.connection.async_get_connected_devices()
if not data:
return False
self.last_results = data
return True

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 22, 2018

Member

See above.

@kennedyshead kennedyshead force-pushed the kennedyshead:async_asuswrt branch 2 times, most recently from cb17c6b to f0f8988 Oct 22, 2018

@kennedyshead

This comment has been minimized.

Contributor

kennedyshead commented Oct 22, 2018

I don't think that redundant checks if data is returned is needed, the simpler the better ;)

@kennedyshead kennedyshead force-pushed the kennedyshead:async_asuswrt branch from f0f8988 to 02ca930 Oct 22, 2018

self.connection = AsusWrt(self.host, self.port,
False if self.protocol == 'ssh' else True,
self.username, self.password, self.ssh_key,
self.mode, self.require_ip)
self.last_results = {}

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 22, 2018

Member

Do we need to reset this here? It's already initialized in init.

This comment has been minimized.

@kennedyshead

kennedyshead Oct 22, 2018

Contributor

You are right, that is just sloppy coding

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 23, 2018

Member

This was left.

This comment has been minimized.

@balloob

This comment has been minimized.

@kennedyshead

kennedyshead Oct 23, 2018

Contributor

oops

This comment has been minimized.

@kennedyshead

@kennedyshead kennedyshead force-pushed the kennedyshead:async_asuswrt branch from 02ca930 to b0b2211 Oct 22, 2018

@kennedyshead

This comment has been minimized.

Contributor

kennedyshead commented Oct 22, 2018

I added all issues that are related to this as a possible fix for it, the lib issues should not be here anyway ;)

@kennedyshead kennedyshead force-pushed the kennedyshead:async_asuswrt branch 4 times, most recently from 0630c2b to af47959 Oct 22, 2018

@kennedyshead kennedyshead force-pushed the kennedyshead:async_asuswrt branch from af47959 to 9389c69 Oct 23, 2018

self.last_results = {}
self.success_init = False
self.connection = AsusWrt(self.host, self.port,
False if self.protocol == 'ssh' else True,

This comment has been minimized.

@kennedyshead

kennedyshead Oct 23, 2018

Contributor

LOL! I broke my own PEP request!!!

@balloob

This comment has been minimized.

Member

balloob commented Oct 23, 2018

amazing!

@balloob balloob merged commit 277a9a3 into home-assistant:dev Oct 23, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.597%
Details

@wafflebot wafflebot bot removed the in progress label Oct 23, 2018

@kennedyshead kennedyshead deleted the kennedyshead:async_asuswrt branch Oct 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment