doudz / zigate

python lib for zigate
MIT License
46 stars 22 forks source link

Fix network discovery #159

Closed pdecat closed 4 years ago

pdecat commented 4 years ago

This PR improves network discovery by switching to an iterative scan of known zigbee devices.

UI changes originally in this were moved to #161 (and merged into dev). TODO:

pdecat commented 4 years ago

A quick glance of how it is shaping up right now:

image

pdecat commented 4 years ago

Rebased and cleaned-up.

pdecat commented 4 years ago

image

ruimarinho commented 4 years ago

Looking good @pdecat 👍

doudz commented 4 years ago

good job ! thanks is it ready to review ?

pdecat commented 4 years ago

Thanks!

Certainly ready for a first pass. Most notably, I'm wondering if timeout should be passed as an option to API calls so that the default can be kept as 3s, but overridden if needed/desired.

pdecat commented 4 years ago

Just noticed I broke the test_build_neighbours_table() unit test:

======================================================================
FAIL: test_build_neighbours_table (tests.test_core.TestCore)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/patrick/workspaces/homeassistant/doudz-zigate/tests/test_core.py", line 722, in test_build_neighbours_table
    ('0000', '1234', 182)])
AssertionError: Lists differ: [('0000', 'abcd', 182)] != [('0000', 'abcd', 182), ('abcd', '9876', 182), ('0000', '1234', 182)]

Second list contains 2 additional elements.
First extra element 1:
('abcd', '9876', 182)

- [('0000', 'abcd', 182)]
+ [('0000', 'abcd', 182), ('abcd', '9876', 182), ('0000', '1234', 182)]

----------------------------------------------------------------------
Ran 58 tests in 7.534s

FAILED (failures=1)
pdecat commented 4 years ago

I'm going to split this PR in two: one for the UI, and one for the network table building.

pdecat commented 4 years ago

I've submitted https://github.com/doudz/zigate/pull/161 to include only UI related changes.

Will rebase this one once #161 is merged.

doudz commented 4 years ago

You can now rebase

pdecat commented 4 years ago

Rebased. I still need to fix the test cases, will try to do so ASAP.