Closed Gregory-K closed 7 years ago
Hi, Thanks for the contribution.
A few things:
.editorconfig
isn't needed, and standard is actually more specific. My editor is just configured that way, so I don't need to worry about it..gitignore
It's a ton of editor-specific stuff for editors/OS I don't use. Was this a specific use need that you had, for some reason?status
command should be covered by details
(and more) is there something that was missing from this?tplight on -b BRIGHTNESS
, and I'd rather keep the number of commands to a minimum, if it makes sense. Since the first param to set
is on/off, it makes a lot of sense to me that it's just a param for on
. What is the use-case for setting off + brightness?Hi, nice to talk to.
First, I am admitting that this pull request was a late hours mistake.
.gitignore
covered above.status
command is indeed covered by details
. I find it useful in times you just want to check if the light bulb is on or off and with what settings (temp, brightness etc). details
is returning a lot to scroll-up for just to find the light_state
.tplight on -b
. I understand that "tplink-lightbulb" is more of a paradigm than a full blown controller suite and you are right about keeping the number of commands to a minimum.
on -b
argument but I think it's a little awkward that the only option to alter brightness is to "set the lights on". bright
is turning the bulb off. That is the second of my mistakes. Behind that move was the notion that the bright
and later the temp, hex, hsb
commands should be indifferent to the light-bulb on/off state. I haven't yet searched if this is possible.Thanks for listening to me ... now we can close this pull request :)
forgot to mention: transitions are actually in milliseconds and not seconds (talking about the command examples)
Totally understand , I have made many late-night PRs myself. 😄
Maybe we should include something about standard in readme/scripts
to help people follow the style. As I said, I just configure my editor, but it should probly be documented somewhere, in the very least. I made an issue for myself: #15
For details
, I see your point, but I might recommend jq It does nice syntax-highlighting, let's you pull out just the parts of JSON you want, and lets you do -r
(raw) to pull out a value and use it in other things (I use this for bash scripts, etc.) Lately, I have been using it for all kinds of things (it's rad with aws cli tools, for example,) it's really handy.
I see your use-case for bright
, but the bulb only changes state with on
/off
message, so maybe we could just give off
all the same params, so you can set them either way. I made an issue for myself: #16
Thanks again for your contribution. Happy to work on these things, I think they are good suggestions.
Thanks for the follow up.
I most certainly will try jq. As for the off
params, i have a slight intuition that the bulb doesn't accept more status changes while turning-off. We'll see. I hope it does.
p.s. I have made a minor correction from .example('$0 on -t 10 10.0.0.200', 'Take 10 seconds to turn on a light')
to .example('$0 on -t 10000 10.0.0.200', 'Take 10 seconds to turn on a light')
but I don't to bother you with a pull request for something like that.
Ah, yeah, good catch. I will update the code. Maybe I'll wait till I have time for the other changes for a proper version release.
"bright" command -- set only brightness without setting the bulb on "status" command -- return the bulb's light state .editorconfig coding style -- misc additions to .gitignore