alisdairjsmyth / node-red-contrib-lifx-api

A collection of Node-RED nodes to control LIFX globes using the HTTP Remote Control API
Apache License 2.0
8 stars 5 forks source link

Set State node should not require power, brightness and duration parameters #4

Closed bland328 closed 6 years ago

bland328 commented 6 years ago

The documentation for the Set State node says: "If you don't supply a parameter, the node will leave that value untouched."

My experience, however, is that if brightness, duration or power parameters are missing, nothing happens, and an error message like this is the result: "brightness does not have a valid value, duration does not have a valid value"

My configuration appears otherwise correct, since providing all three parameters results in the bulb behaving as expected.

Are my expectations wrong, or is the node misbehaving?

bland328 commented 6 years ago

Some additional information:

While Set State doesn't accept this payload: {"power": "on"}

...it does accept this payload: {"power": "on", "brightness": null, "duration": null}

alisdairjsmyth commented 6 years ago

Try v1.0.6 on npm. (I have not been able to fully test this fix as I am remote from my bulbs and they are all offline.) Let me know how you go.

bland328 commented 6 years ago

Thanks for the quick response, @alisdairjsmyth! Sadly, v1.0.6 didn't help.

In my quick test, I attempted to change a bulb color without changing the power state, but got a "power does not have a valid value" error.

When I then added {"power": null}, it worked.

So, there's a workaround, but it does complicate use somewhat.

To be clear for anyone else who is running into this, my experience is that power, brightness, and duration all must be provided, even if null.

alisdairjsmyth commented 6 years ago

I have access to the globes this evening. With v1.0.6 have been able to successfully process the following msg.payload objects: {power: "on"}, {color:"kelvin:5000"}, {brightness:1.0} where selector had been specified in the node configuration. I have also been able to successfully process msg.payload objects containing combinations of the above.

bland328 commented 6 years ago

Thanks again, @alisdairjsmyth.

To be clear, what I'm reporting is that the set state node appears to always require power and duration and brightness, but it should not.

The set state documentation for node-red-contrib-lifx-api says:

All parameters (except for the selector) are optional. If you don't supply a parameter, the node will leave that value untouched.

That makes perfect sense to me--if only power is changing, I should need to pass only power.

However, for me, this msg.payload doesn't work: {"power": "on"}

To make it work, I have to add null duration and brightness, like so: {"power": "on", "brightness": null, "duration": null}

I hope this helps, and sorry if I've over-explained! ;)

alisdairjsmyth commented 6 years ago

@bland328 I understand the issue you are describing (and you are not over-explaining), and my problem is that I have not been able to reproduce it with the new version of the Set State node (^1.0.6). As I mentioned yesterday, I have been able to successfully process msg.payload with single and multiple parameters - including a msg.payload of {power: "on"}.

In my tests I have not specified any values for power, brightness or duration in the configuration of the node. I will have another look this evening.

alisdairjsmyth commented 6 years ago

I have prepared a sample flow with examples of using the Set State node with only a single parameter and selector (label:Lamp). All of these scenarios work on my laptop - details of my Node-RED configuration below.

11 Jan 21:31:49 - [info] Node-RED version: v0.17.5
11 Jan 21:31:49 - [info] Node.js  version: v6.9.5
11 Jan 21:31:49 - [info] Windows_NT 10.0.14393 x64 LE

... with v1.1.0 of node-red-contrib-lifx-api

You should be able to copy and import this flow into your Node-RED editor. Update token details and selector within function nodes in the editor to reflect your details

alisdairjsmyth commented 6 years ago

@bland328 Still not working for you?

bland328 commented 6 years ago

Sorry, @alisdairjsmyth, was away from this for a few days!

Good news--I just now confirmed that passing just a selector and one parameter like this DOES now work: {"selector":"id:d0xxxxxxxxd8f","power":"on"}

I no longer need to "pad" the payload with null brightness and duration.

Thank you for all your great and generous work on this!

alisdairjsmyth commented 6 years ago

No problem. I am glad to hear it is working for you.