Skip to content
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

Local ip address #9

Merged
merged 5 commits into from
Jun 28, 2017
Merged

Local ip address #9

merged 5 commits into from
Jun 28, 2017

Conversation

yarray
Copy link
Contributor

@yarray yarray commented Jun 23, 2017

This update added a "local_ip_address" option in config. This option is quite useful if multiple network interfaces exist and the one connecting to the target device is not the first. Also node's os.networkInterfaces cannot find all interfaces in some cases where providing a binding address may become a must (issue nodejs/node#498).

To make this patch work, updating https://github.com/smka/broadlinkjs-sm is mandatory (however it will not break either if the update does not happen). I also submitted a pull request there smka/broadlinkjs-sm#2.

Use/test case with homebridge-broadlink-platform:

My raspberry pi connected to a router via eth0 along with my iphone. The raspberry pi also has a wireless interface wlan0 which I set as an AP. The network on eth0 has ip 192.168.0.10 and the network on wlan0 has ip 192.168.42.1. An SP1 device connected to wlan0.

Now if without the "local_ip_address" option, when sending the broadcast message to 255.255.255.255, the packet will be sent through only one (random) interface (see https://stackoverflow.com/questions/683624/udp-broadcast-on-all-interfaces) and perhaps under the network 192.168.0. Thus , it will not reach my SP1. By this update we can provide local_ip_address as 192.168.42.1 to solve the problem.

Changes

The changes are simple: get the config and fill it to broadlink.discover. A method is extracted to remove duplicates.

@smka
Copy link
Owner

smka commented Jun 27, 2017

@yarray hi! thanks for this update!
Is the "local_ip_address" an option now? And if there is no "local_ip_address" in config - it will work by default?

@yarray
Copy link
Contributor Author

yarray commented Jun 27, 2017

@smka Yes. When the option does not present, the code is just broadlink.discover(undefined) which would be the same as with no parameter. It may be more complete to add a null-check before calling discover, though.

@smka smka merged commit d35e646 into smka:master Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants