search

jangxx / node-magichome

An incomplete implementation of the functionality of the "Magic Home" app. Partially a port of https://github.com/Danielhiversen/flux_led to Node.js
ISC License
124 stars 26 forks source link

Discovery Feature #2

Closed edeuxk closed 6 years ago

edeuxk commented 6 years ago

Create a child process with python. Created python script to discover lights in the local network. Discovery.scan() : Promise, return an array. Compatible python version 2/3+

Need improvement : should use Node.JS sockets instead of spawning a python script. (https://nodejs.org/api/dgram.html)

Current result:

screen shot 2018-05-15 at 1 46 51 pm
jangxx commented 6 years ago

Hey, thanks a lot for your contribution, the Discovery Mode is a pretty important feature of the original python tool and it would be great to have it in the node version as well. Before I merge it however, I would like you to do two things:

  1. Please port the python component to Javascript, so this module can stay a pure Javascript implementation. Mixing different languages in a node module is not nice.
  2. Please have the scan method take a callback in addition to returning a Promise, so that we can keep the API somewhat consistent, in that every method takes a callback. In the future I (or someone else :D) will update the methods of the Control class to return Promises as well.

If you do these two things, I'll happily merge your work into this module and submit an updated version to npm :)

edeuxk commented 6 years ago

Totally agree with the fact that mixing different language in a node module is not nice but it was the fastest way to do a POC.

I'll do the two things and update the pull request :)

edeuxk commented 6 years ago

@jangxx Everything is done.