bahamas10 / hue-cli

A command line interface to phillips hue
http://bahamas10.github.com/hue-cli/
221 stars 24 forks source link

update to new hue-js interface, storing username token in config file #14

Closed mattbucci closed 7 years ago

mattbucci commented 8 years ago

Before merging https://github.com/bahamas10/hue.js/pull/1 and https://github.com/bahamas10/hue.js/pull/2 must be merged

TwizzyDizzy commented 7 years ago

@mattbucci actually, when cloning your master of hue-cli (commit 0026cfa141f90f5f729a6d362c2c76bf6e118897), I'm still getting errors - though different ones:

When using hue search:

dgram.js:445
    throw errnoException(err, 'getsockname');
    ^

Error: getsockname EINVAL
    at exports._errnoException (util.js:1026:11)
    at Socket.address (dgram.js:445:11)
    at next (/opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/node_modules/hue.js/lib/Discoverer.js:27:25)
    at Object.module.exports [as discover] (/opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/node_modules/hue.js/lib/Discoverer.js:21:5)
    at Object.<anonymous> (/opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/hue-cli.js:265:9)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)

When using hue -H 192.168.1.205 register:

please go and press the link button on your base station
failed to pair to Hue Base Station 192.168.1.205

/opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/hue-cli.js:251
    throw err;
    ^
Error
    at Object.<anonymous> (/opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/node_modules/hue.js/lib/helpers.js:7:22)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/node_modules/hue.js/lib/Hue.js:5:20)
    at Module._compile (module.js:556:32)

NodeJS version: 6.4.0 (armv7).

bahamas10 commented 7 years ago

I will test this this weekend when I have access to a newer bridge, thanks!

Also, I created a (terribly named) PR a long time ago with good intentions, but it has unfortunately stagnated. Either way, it may be worth incorporating hue-sdk (my own creation) into this module instead of the seemingly abandoned hue.js.

mattbucci commented 7 years ago

@TwizzyDizzy assuming you dropped in my version of hue.js as well? Seems odd though that it's saying something about a socket error. I'll test with my version when I get home.

I'm on node v6.5.0 x86 osx

mattbucci commented 7 years ago

@bahamas10 I'd be glad to help out in anyway I can, even if that means refactoring to a new hue sdk or updating your hue sdk package to be compatible with the new auth flow

mattbucci commented 7 years ago

@TwizzyDizzy try changing your package.json to point to mattbucci/master for hue-js, run npm update and see if you have the same problem. My fixes aren't merged to his master yet.

TwizzyDizzy commented 7 years ago

Yeah, done that. But still... same error (though now just upraded to nodejs 6.6.0 that was released roughly an hour ago).

If you have anything I should test, don't hesitate to hit me up :)

Cheers Thomas

mattbucci commented 7 years ago

@TwizzyDizzy I'll break out the raspi today and see if I can reproduce it, I've got the newer hub btw

mattbucci commented 7 years ago

@TwizzyDizzy unfortunately my raspi seems dead so you're gonna have to test some untested stuff :+1:

So your first error seems to be a common issue with calling sever.address before the port has finished binding....now I can't see why this would ever get executed synchronously but perhaps @bahamas10 would know as it looks like he was the last to touch that.

I would edit the file /opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/node_modules/hue.js/lib/Discoverer.js changing lines 19-24 from. Your log clearly shows that line 21 is executed

if (client.bind.length) {
    client.bind();
    next(); // this fails because bind hasn't completed yet, not sure bind.length is being checked but it's clearly not working
  } else {
    client.bind(next);
  }

to

    client.bind(next);

Then report back. (For the sake of the reader I'm referencing the docs located here https://nodejs.org/api/dgram.html#dgram_socket_bind_port_address_callback)

As for your second error check out http://192.168.1.205/debug/clip.html and run through http://www.developers.meethue.com/documentation/getting-started to see if you can figure anything else out. are you running a new hub or an old hub? I'm on the new hub but your error seems not to be related to any code but rather a failed response of the hub. Maybe it's an older version and we have to account for both? dunno. You could also try adding a console.log(err) above line 251 of /opt/node-v6.4.0-linux-armv7l/lib/node_modules/hue-cli/hue-cli.js and let us know what the error is that hue is returning

TwizzyDizzy commented 7 years ago

Mhhh.. that kind of debugging seems to me to be rather cumbersome. If you like, I'd provide you with an unprivileged (read normal user) SSH account on my raspberry so you could test/debug as you like?

Send me your pubkey to [...]@[...] (redacted, after access was granted) if you want.

As for you change above: this lead to an EADDRINUSE error.

I'm running the v2 hub.

Cheers Thomas

mattbucci commented 7 years ago

I see where we got confused, @bahamas10 already fixed that issue in his dave branch. I merged that fix of his into my master so that it's a complete working implementation of hue.js and now discovery is working as expected:

mattbucci@behring5:~/hue-cli$ node hue-cli.js search
1 stations found

1: 192.168.1.205
TwizzyDizzy commented 7 years ago

Hi @mattbucci,

alright, since you can't physically press the register button on my bridge, I'm going to test the register functionality when I get home later (around 7PM, CEST). Should I test with the version in your home directory or your github master?

Cheers Thomas

mattbucci commented 7 years ago

they should both be the same now, but I'll add a console log to the one in my home dir so let's use that one

On Fri, Sep 16, 2016 at 2:33 AM, Thomas Dalichow notifications@github.com wrote:

Hi @mattbucci https://github.com/mattbucci,

alright, since you can't physically press the register button on my bridge, I'm going to test the register functionality when I get home later (around 7PM, CEST). Should I test with the version in your home directory or your github master?

Cheers Thomas

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bahamas10/hue-cli/pull/14#issuecomment-247558348, or mute the thread https://github.com/notifications/unsubscribe-auth/ADIcs0Yq4oM1ik7EAawwcIejZwr2-laEks5qqmJPgaJpZM4J9mkv .

TwizzyDizzy commented 7 years ago

Just tested the register function. It worked! As did the light functions and everything else hue-cli shall achieve :)

(for readers, this is debugging sample code and not to be used by you. Please refer to README.md instead)

mattbucci@behring5:~$ node /home/mattbucci/hue-cli/hue-cli.js -H 192.168.1.205 register please go and press the link button on your base station Hue Base Station paired! username: 4x55Sy492M3hfUCSNZKE8K-mBUwaXH-z8JvTW8-c config file written to /home/mattbucci/.hue.json

Cheers Thomas

jgladch commented 7 years ago

Can we get this merged, plz? 🎊

mattbucci commented 7 years ago

@jgladch @yadomi if you'd like to test this now please do the following and report back and let us know if it works. Will be much more helpful!

I'm sure he'll be much more comfortable merging if we have more people verifying it's fixed 👍

bahamas10 commented 7 years ago

hey everyone, thanks for all the debugging on this and interest in the project overall... i'm looking at the code now and will update this thread when everything is merged and working :D

TwizzyDizzy commented 7 years ago

Hi @bahamas10

My current proposal would be to drop the --save functionality completely, and make it so hue register will automatically write out the config IF ONE DOES NOT ALREADY EXIST. If one exists, hue register should print a warning, dump the current config, and exit non-zero, and leave it up to the user to remove it before registering a new bridge. What do you all think of that?

:+1: seems to be the wisest course of action!

yadomi commented 7 years ago

@mattbucci damn, i'm too late. I've made #15 but it seem you're faster than me :)

mattbucci commented 7 years ago

@yadomi I reviewed your work, funny we both chose to work on it at the same time 👍, Seems like I can do a bit of code cleanup if process.exit is preferred over throw

yadomi commented 7 years ago

@mattbucci yes that's funny, we've made almost the same change 👍 for the throw thing, I think for a CLI tool, it's more friendly to display a nice error message than throw the raw error to the console.

btw, should I close my PR (#15) since this one do almost the same things ?

mattbucci commented 7 years ago

@yadomi yeh that's fine. please close your PR and review mine. I'll make any changes needed (changing throw to process.exit(1)) and anything else you find and hopefully we can get this merged later today.

yadomi commented 7 years ago

@mattbucci this PR looks OK to me 🚀

bahamas10 commented 7 years ago

thank you everyone! I've merged this pr and have published it to npm

$ npm publish
+ hue-cli@0.2.0