-
-
Notifications
You must be signed in to change notification settings - Fork 540
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
Revise device (broadcast-using) discovery process #437
Conversation
This is the initial commit to rework the handshake/discovery process, separating them to separate entities. The discovery part from Device class is moved into MiIODiscovery class usable for broadcast discovery of available devices. Also, instead of printing out the details from discovered devices, a list of Devices is being returned. The discover() method of the class is extended to allow discovery on all network interfaces (instead of using 255.255.255.255), making it easier to perform the initial configuration of non-provisioned devices and/or work with devices in other subnets. Although the change cause some duplicate code (mainly the handshake payload), this will make the responsibilities of corresponding classes clearer and code more usable for 3rd party developers. There will be cleanups and (likely also) renamings of some of the parts, but I wanted to make this available for others to comment. TODO: * Upcasting the Device to a corresponding implementation class (miIO.info model information could be useful, too..) * Decide how to handle the returning of discovered devices. Use a separate DiscoveredDevice class which can be used to initialize the real device, or use the existing Device and do the casting trick? Related to #152 (and maybe #422).
"the device should connect to the given network") | ||
except DeviceException as ex: | ||
click.echo("Unable to configure the wifi connection: %s" % ex) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line contains whitespace
continue | ||
all_devices.extend(devices) | ||
|
||
return all_devices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line at end of file
_LOGGER.debug("Broadcast addreses: %s", ipv4_broadcasts) | ||
for broadcast in ipv4_broadcasts: | ||
_LOGGER.debug("Sending discovery to %s with timeout of %ss..", | ||
broadcast, timeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation line under-indented for visual indent
break | ||
|
||
return devices | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line contains whitespace
import socket | ||
from . import Message | ||
|
||
class MiIODiscovery: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expected 2 blank lines, found 1
@@ -161,3 +158,83 @@ def discover_mdns() -> Dict[str, Device]: | |||
browser.cancel() | |||
|
|||
return listener.found_devices | |||
|
|||
import socket | |||
from . import Message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module level import not at top of file
@@ -161,3 +158,83 @@ def discover_mdns() -> Dict[str, Device]: | |||
browser.cancel() | |||
|
|||
return listener.found_devices | |||
|
|||
import socket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expected 2 blank lines after class or function definition, found 1
module level import not at top of file
@@ -1,7 +1,9 @@ | |||
import binascii |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'binascii' imported but unused
The codebase has changed so much that improving the discovery process should be done from scratch. |
This is the initial commit to rework the handshake/discovery process,
separating them to separate entities.
The discovery part from Device class is moved into MiIODiscovery class usable for broadcast discovery of available devices.
Also, instead of printing out the details from discovered devices, a list of Devices is being returned.
The discover() method of the class is extended to allow discovery on all network interfaces (instead of using 255.255.255.255),
making it easier to perform the initial configuration of non-provisioned devices and/or work with devices in other subnets.
Although the change cause some duplicate code (mainly the handshake payload),
this will make the responsibilities of corresponding classes clearer and code more usable for 3rd party developers.
There will be cleanups and (likely also) renamings of some of the parts, but I wanted to make
this available for others to comment.
TODO:
to initialize the real device, or use the existing Device and do the casting trick?
Related to #152 (and maybe #422).