Added support for alternate SSH ports in AsusWRT (#4832) #5198

swbradshaw · 2017-01-06T04:55:36Z

Description:
My Asus router doesn't run SSH on port 22. This allows Home Assistant to work now by exposing this the SSH port as a parameter.

Related issue (if applicable): fixes #4832

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

Example entry for configuration.yaml (if applicable):

device_tracker:
  - platform: asuswrt
    host: YOUR_ROUTER_IP
    username: YOUR_ADMIN_USERNAME
    password: YOUR_ADMIN_PASSWORD
    protocol: ssh
    port: 22222

Checklist:

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

[] Documentation added/updated in home-assistant.github.io

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

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

mention-bot · 2017-01-06T04:55:37Z

@swbradshaw, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kellerza, @balloob and @linjef to be potential reviewers.

houndci-bot · 2017-01-06T04:55:41Z

homeassistant/components/device_tracker/asuswrt.py

@@ -16,7 +16,7 @@

 from homeassistant.components.device_tracker import (
    DOMAIN, PLATFORM_SCHEMA, DeviceScanner)
-from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
+from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME, CONF_PORT


line too long (82 > 79 characters)

swbradshaw · 2017-01-06T05:41:38Z

Travis builds are failing but in real life these tests work on actual configs. The default port config is defined at the top of the file in the PLATFORM_SCHEMA. I copied this method from usage in other files (netgear.py, cisco_ios.py, etc). This method works, but fails Travis tests. It turns out Netgear and Cisco IOS don't have tests written - otherwise they would be failing, too. Go figure. :-)

So what is the correct way to write this to pass the tests? If you could point me to an example, I will correct.

fabaff · 2017-01-06T06:46:52Z

The tests are failing because of the new configuration variable. Add the CONF_PORT to test_asuswrt.py.

…ation. Fixed long line

houndci-bot · 2017-01-06T19:33:23Z

homeassistant/components/device_tracker/asuswrt.py

@@ -16,7 +16,8 @@

 from homeassistant.components.device_tracker import (
    DOMAIN, PLATFORM_SCHEMA, DeviceScanner)
-from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
+from homeassistant.const import (CONF_HOST, CONF_PASSWORD,
+    CONF_USERNAME, CONF_PORT)


continuation line under-indented for visual indent

Newline immediately after opening the bracket ( and then 4 spaces indented on next line start with first one

kellerza · 2017-01-07T04:11:06Z

@swbradshaw you can run tests locally for quicker feedback. Some shortcuts for tox (what travis runs) below...

The following will show style errors (like the hound-bot)

$ script/lint --changed

The following will show you the first failure & what test you still need to work on

$ py.test -x -k asus

houndci-bot · 2017-01-08T20:58:01Z

tests/components/device_tracker/test_asuswrt.py

@@ -173,7 +173,7 @@ def test_ssh_login_without_password_or_pubkey(self):  \

        conf_dict = {
            CONF_PLATFORM: 'asuswrt',
-            CONF_HOST: 'fake_host',
+            CONF_HOST: 'fake_host',            


trailing whitespace

houndci-bot · 2017-01-08T20:58:01Z

tests/components/device_tracker/test_asuswrt.py

@@ -146,7 +146,7 @@ def test_ssh_login_with_password(self):
        self.addCleanup(ssh_mock.stop)
        conf_dict = PLATFORM_SCHEMA({
            CONF_PLATFORM: 'asuswrt',
-            CONF_HOST: 'fake_host',
+            CONF_HOST: 'fake_host',            


trailing whitespace

houndci-bot · 2017-01-08T20:58:02Z

tests/components/device_tracker/test_asuswrt.py

@@ -121,7 +121,7 @@ def test_ssh_login_with_pub_key(self):
        self.addCleanup(ssh_mock.stop)
        conf_dict = PLATFORM_SCHEMA({
            CONF_PLATFORM: 'asuswrt',
-            CONF_HOST: 'fake_host',
+            CONF_HOST: 'fake_host',            


trailing whitespace

houndci-bot · 2017-01-08T20:58:02Z

tests/components/device_tracker/test_asuswrt.py

@@ -97,7 +97,7 @@ def test_get_scanner_with_pubkey_no_password(self, asuswrt_mock):  \
        conf_dict = {
            device_tracker.DOMAIN: {
                CONF_PLATFORM: 'asuswrt',
-                CONF_HOST: 'fake_host',
+                CONF_HOST: 'fake_host',                


trailing whitespace

houndci-bot · 2017-01-08T20:58:02Z

tests/components/device_tracker/test_asuswrt.py

@@ -72,7 +72,7 @@ def test_get_scanner_with_password_no_pubkey(self, asuswrt_mock):  \
        conf_dict = {
            DOMAIN: {
                CONF_PLATFORM: 'asuswrt',
-                CONF_HOST: 'fake_host',
+                CONF_HOST: 'fake_host',                


trailing whitespace

houndci-bot · 2017-01-08T20:58:02Z

homeassistant/components/device_tracker/asuswrt.py

            elif self.password:
-                self.ssh_secret = {'password': self.password}
+                self.ssh_args['password'] = self.password;                


statement ends with a semicolon
trailing whitespace

houndci-bot · 2017-01-08T20:58:02Z

homeassistant/components/device_tracker/asuswrt.py

            if self.ssh_key:
-                self.ssh_secret = {'ssh_key': self.ssh_key}
+                self.ssh_args['ssh_key'] = self.ssh_key;                


statement ends with a semicolon
trailing whitespace

houndci-bot · 2017-01-08T20:58:03Z

homeassistant/components/device_tracker/asuswrt.py

+
+            if self.port != DEFAULT_SSH_PORT:
+                self.ssh_args['port'] = self.port
+


blank line contains whitespace

swbradshaw · 2017-01-08T21:03:40Z

@kellerza Thanks for the info. Unforunately, I tried getting these tools to run on 2 different machines, and was not successful. This simple change has taken me several hours to commit. I'll get it working, but this will probably be my last contribution to the project. I don't have the time to get my dev environment working correctly to pass the CI tests :-(

houndci-bot · 2017-01-08T21:09:44Z

homeassistant/components/device_tracker/asuswrt.py


        if self.protocol == 'ssh':
+
+            if self.port != DEFAULT_SSH_PORT:
+                self.ssh_args['port'] = self.port     


trailing whitespace

swbradshaw · 2017-01-08T21:47:17Z

Unfortunately my time and Python skills aren't up to completing this change. The code works but I don't have the capability to complete this to pass the CI task. Closing this request. I'll close the doc PR, too.

Added support for alternate SSH ports in AsusWRT (home-assistant#4832)

5fc2ebc

houndci-bot reviewed Jan 6, 2017

View reviewed changes

swbradshaw mentioned this pull request Jan 6, 2017

Added SSH port config (#4832) home-assistant/home-assistant.io#1721

Merged

Updated AsusWrt tests to reflect default port being added to configur…

0c82d8b

…ation. Fixed long line

houndci-bot reviewed Jan 6, 2017

View reviewed changes

Rewrote SSH port to help pass tests (home-assistant#4832)

70caa26

houndci-bot reviewed Jan 8, 2017

View reviewed changes

Fixing whitespace

e385110

houndci-bot reviewed Jan 8, 2017

View reviewed changes

swbradshaw closed this Jan 8, 2017

home-assistant locked and limited conversation to collaborators Apr 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for alternate SSH ports in AsusWRT (#4832) #5198

Added support for alternate SSH ports in AsusWRT (#4832) #5198

swbradshaw commented Jan 6, 2017 •

edited

mention-bot commented Jan 6, 2017

houndci-bot Jan 6, 2017

swbradshaw commented Jan 6, 2017 •

edited

fabaff commented Jan 6, 2017 •

edited

houndci-bot Jan 6, 2017

kellerza Jan 7, 2017

kellerza commented Jan 7, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

houndci-bot Jan 8, 2017

swbradshaw commented Jan 8, 2017

houndci-bot Jan 8, 2017

swbradshaw commented Jan 8, 2017


		if self.port != DEFAULT_SSH_PORT:
		self.ssh_args['port'] = self.port

Added support for alternate SSH ports in AsusWRT (#4832) #5198

Added support for alternate SSH ports in AsusWRT (#4832) #5198

Conversation

swbradshaw commented Jan 6, 2017 • edited

mention-bot commented Jan 6, 2017

Choose a reason for hiding this comment

swbradshaw commented Jan 6, 2017 • edited

fabaff commented Jan 6, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellerza commented Jan 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swbradshaw commented Jan 8, 2017

Choose a reason for hiding this comment

swbradshaw commented Jan 8, 2017

swbradshaw commented Jan 6, 2017 •

edited

swbradshaw commented Jan 6, 2017 •

edited

fabaff commented Jan 6, 2017 •

edited