Update luci.py device tracker to work with OpenWRT 18.06#15792
Update luci.py device tracker to work with OpenWRT 18.06#15792fbradyirl wants to merge 7 commits into
Conversation
ghost
added
the
| error_message = result['error']['message'] | ||
| error_code = result['error']['code'] | ||
| _LOGGER.warn("method: '%s' returned an error '%s' (code: '%s)." % (method, error_message, error_code)) | ||
| if error_code == -32601: |
There was a problem hiding this comment.
indentation is not a multiple of four
| # On 18.06, we want to check for error 'Method not Found' | ||
| error_message = result['error']['message'] | ||
| error_code = result['error']['code'] | ||
| _LOGGER.warn("method: '%s' returned an error '%s' (code: '%s)." % (method, error_message, error_code)) |
There was a problem hiding this comment.
indentation is not a multiple of four
line too long (116 > 79 characters)
| else: | ||
| # On 18.06, we want to check for error 'Method not Found' | ||
| error_message = result['error']['message'] | ||
| error_code = result['error']['code'] |
There was a problem hiding this comment.
indentation is not a multiple of four
| return result_value | ||
| else: | ||
| # On 18.06, we want to check for error 'Method not Found' | ||
| error_message = result['error']['message'] |
There was a problem hiding this comment.
indentation is not a multiple of four
| if result_value is not None: | ||
| return result_value | ||
| else: | ||
| # On 18.06, we want to check for error 'Method not Found' |
There was a problem hiding this comment.
indentation is not a multiple of four (comment)
| _LOGGER.warn('method net.arptable is not found. Going to try ip neighbors next time instead...') | ||
| self.use_ip_neighbors_table = True | ||
| return False | ||
|
|
|
|
||
| except LuciRpcMethodNotFoundError: | ||
| # This is normal for newer OpenWRT releases (18.06+) | ||
| _LOGGER.warn('method net.arptable is not found. Going to try ip neighbors next time instead...') |
There was a problem hiding this comment.
line too long (108 > 79 characters)
| if self.use_ip_neighbors_table: | ||
| # Newer OpenWRT releases (18.06+) | ||
| result = _req_json_rpc( | ||
| ip_url, 'neighbors', {"family": 4}, params={'auth': self.token}) |
There was a problem hiding this comment.
line too long (84 > 79 characters)
trailing whitespace
| """When an invalid method is called.""" | ||
|
|
||
| pass | ||
|
|
|
|
||
| pass | ||
|
|
||
| class LuciRpcMethodNotFoundError(HomeAssistantError): |
| except LuciRpcMethodNotFoundError: | ||
| # This is normal for newer OpenWRT releases (18.06+) | ||
| _LOGGER.warn('method net.arptable is not found. ' | ||
| 'Going to try ip neighbors next time instead...') |
There was a problem hiding this comment.
continuation line under-indented for visual indent
|
|
||
| if result_value is not None: | ||
| return result_value | ||
|
|
| except LuciRpcMethodNotFoundError: | ||
| # This is normal for newer OpenWRT releases (18.06+) | ||
| _LOGGER.error('method net.arptable is not found. ' | ||
| 'Going to try ip neighbors next time instead...') |
fbradyirl
changed the title
| self.assertEqual('home', self.hass.states.get('device_tracker.mydevicename').state) | ||
|
|
||
| self.assertEqual('home', self.hass.states.get('device_tracker.b8e8995c6e15').state) | ||
| self.assertEqual('home', self.hass.states.get('device_tracker.b827eb117c22').state) |
| self.assertIsNone(self.hass.states.get('device_tracker.188b400814b2')) | ||
| self.assertEqual('home', self.hass.states.get('device_tracker.mydevicename').state) | ||
|
|
||
| self.assertEqual('home', self.hass.states.get('device_tracker.b8e8995c6e15').state) |
| self.setup_luci_component(); | ||
|
|
||
| self.assertIsNone(self.hass.states.get('device_tracker.188b400814b2')) | ||
| self.assertEqual('home', self.hass.states.get('device_tracker.mydevicename').state) |
| """Test for successfully setting up the Luci platform on v18.06+""" | ||
| mock_responses_version_v18(mock) | ||
|
|
||
| self.setup_luci_component(); |
| self.assertIsNone(self.hass.states.get('device_tracker.b8e8995c6e12')) | ||
| self.assertIsNone(self.hass.states.get('device_tracker.b8e8995c6e10')) | ||
|
|
||
| self.assertEqual('home', self.hass.states.get('device_tracker.b8e8995c6e11').state) |
|
|
||
| self.setup_luci_component(); | ||
|
|
||
| self.assertEqual('home', self.hass.states.get('device_tracker.raspberrypi3').state) |
| """Test for successfully setting up the Luci platform on pre-v18.""" | ||
| mock_responses_version_pre_18(mock) | ||
|
|
||
| self.setup_luci_component(); |
| @requests_mock.Mocker() | ||
| def add_devices(self, devices, mock): | ||
| """Mock add devices.""" | ||
| mock_responses(mock) |
| '{}/cgi-bin/luci/rpc/uci?auth={}'.format(base_url, token), | ||
| text=load_fixture('luci_dhcp.json')) | ||
|
|
||
| def mock_responses_version_v18(mock): |
|
|
||
| import requests_mock | ||
|
|
||
| from homeassistant.components.device_tracker.luci import LuciDeviceScanner |
There was a problem hiding this comment.
'homeassistant.components.device_tracker.luci.LuciDeviceScanner' imported but unused
fbradyirl
changed the title
| self.assertTrue(setup_component(self.hass, | ||
| device_tracker.DOMAIN, { | ||
| device_tracker.DOMAIN: LUCI_CONFIG | ||
| })) |
There was a problem hiding this comment.
continuation line missing indentation or outdented
| def setup_luci_component(self): | ||
| self.assertTrue(setup_component(self.hass, | ||
| device_tracker.DOMAIN, { | ||
| device_tracker.DOMAIN: LUCI_CONFIG |
There was a problem hiding this comment.
continuation line missing indentation or outdented
|
|
||
|
|
||
|
|
||
| def mock_responses_version_v18(mock): |
Updating to use new APIs in OpenWRT 18.06. The old net.arptable method is no longer available.
| def setup_luci_component(self): | ||
| self.assertTrue(setup_component(self.hass, | ||
| device_tracker.DOMAIN, { | ||
| device_tracker.DOMAIN: LUCI_CONFIG |
There was a problem hiding this comment.
continuation line missing indentation or outdented
|
|
||
| def setup_luci_component(self): | ||
| self.assertTrue(setup_component(self.hass, | ||
| device_tracker.DOMAIN, { |
There was a problem hiding this comment.
continuation line under-indented for visual indent
lkollar
left a comment
lkollar
left a comment
There was a problem hiding this comment.
Would it make sense to create two subclasses which inherit from LuciDeviceScanner and implement the pre-18.06 and post-18.06 logic by overriding methods in the base class? get_scanner could then invoke a factory method on LuciDeviceScanner to return the right implementation.
|
@lkollar Yes I like that idea. I'll try and get time to implement that over the coming days. Thanks for looking! |
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
Please break out device specific code, requests and data parsing, into a standalone library published on pypi.
We shouldn't make any changes to this module until that is done.
|
Can somebody please comment on home-assistant/home-assistant.io#6050? UBUS vs. RPC for retrieving details. Thanks |
|
Just a side-note, I was working to externalize this but found out that there's a push-based solution which is much nicer way for device tracking (openwrt/packages#5701). As part of that effort I created https://github.com/rytilahti/python-ubus for communicating with the ubus-rpc, which may be interesting for the externalized luci tracker lib. |
|
Noticed this bug this weekend as well, present on HA 0.81.4 / Openwrt 18.06.1 as well. |
|
i have last version of openwrt and im having this issues my device iss allways home :( any fix |
|
any news? |
|
@fbradyirl why did you close it? |
|
@disrupted I closed it because I don't have the time at present to implement the work as a pip module. I can reopen if I ever get the chance, but right now, it is not happening, so best to close the PR. |
This comment has been minimized.
This comment has been minimized.
|
@fbradyirl We waiting for you reopen it; |
|
I spent two days looking a solution for luci device tracker (after updating my router openwrt to 18.06.01), It just works! Please implement this |
This comment has been minimized.
This comment has been minimized.
Why should it be a pip module? |
|
@flipreverse Converting my branch into a pip module as that's the way external APIs are to be interacted with from HA. I've set up a template project for the pip module here: I've added you as a collab, so if you can help that would be good. I am just busy at present but might eventually get to it. |
@flipreverse see the above PR feedback...
|
Did you see the link of the rpc I posted above, or is this another kind of RPC API? If you didn't check it out yet, it may save some effort. I was working on that to convert luci tracker to use it at some point before I simply started to use https://github.com/mueslo/openwrt_hass_devicetracker . |
|
Yes as I said earlier, my device doesn’t have wifi so any tracker based off that won’t work for everyone. Thanks anyway! |
|
@fbradyirl - true |
|
Agreed. Falling back to the arp table seems to be the next best thing without WiFi. |
|
Just another vote/request for a 18.06 compliant tracker (I also dont use the WiFi so arp table ideal) |
|
Opened new PR with the fix. |
|
Thank you @fbradyirl for fixing this! |
|
I'm going to lock this thread now. The new PR is merged. If there are bugs please open new issues. |
15 participants
Description:
Updating to use new APIs in OpenWRT 18.06 (which was released yesterday).
The old
net.arptablemethod is no longer available so device tracking will be broken in HA for users who upgrade their router to the latest.Related issue (if applicable): fixes #8508
Checklist:
tox.If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices: