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

Add methods to retrieve uptime and reboot bulb #63

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jasoncodes
Copy link
Contributor

Tested with LIFX Original.

I use these to monitor device uptime and reboot the bulb when connection to the cloud is lost and does not automatically reconnect.

The uptime call is officially documented. The reboot request packet was found via the no longer supported Ruby gem. The reboot response packet was found via a packet capture.

@ristomatti
Copy link
Collaborator

ristomatti commented Nov 29, 2017

@jasoncodes this looks awesome, thanks! I'll give it a try when I have the time. I wasn't aware about this functionality myself.

Please bear with us as it might take quite some time until this progresses any further since @MariusRumpf has been taking time off the project. I've been given access to merge PR's but to be perfectly honest I don't feel comfortable doing that as I don't have enough information on if the develop branch should be part of the next npm release. And even if I did, I don't have the required keys to publish an npm release anyhow.

The tests seem to be failing though due to an unexpected character and a linting error. You'll spot the errors with npm run lint.

Copy link
Collaborator

@ristomatti ristomatti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment on the code diff but also check my previous comment. If possible, please also add some unit tests for the functionality. Although with the current project activity level I think they can be added later.

var low = buf.readUInt32LE(0);
var high = buf.readUInt32LE(4);

return (high * 2**32 + low) / 1.0E9;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double ** here is giving issues on the tests. Please also make this line a separate function to help understand what happens here.

@ristomatti
Copy link
Collaborator

I tested this and both uptime and reboot seem to work. In case you're interested I merged this to master on my own fork and rebased my open PR's #60 and #61 on top of it. I also updated TypeScript typings and the interactive-cli.js example with the new functions. The stuff can be found here: https://github.com/ristomatti/node-lifx/tree/es2015-dev

Note that this is a branch for my own use of the library mainly with Node-RED.

@ristomatti
Copy link
Collaborator

ristomatti commented Nov 29, 2017

@jasoncodes Forgot to ask if you know what the uptime figure returned is? I get a floating point number which did not seem to make much sense.

Also in case you want to try out my branch mentioned in my previous comment, note that it's based on my es2015 branch which introduces Babel and requires npm run build before using. If you have a lot of lights, I believe you'll find the changes on #60 quite nice. Changes to multiple lights are almost instant then. :)

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