search

AlexGustafsson / homebridge-wol

A Wake on Lan plugin for Homebridge
MIT License
296 stars 30 forks source link

Enable pingsToChange for pingCommands #243

Open jackklink opened 1 year ago

jackklink commented 1 year ago

pingsToChange will now affect both pingCommands or normal pinging.

AlexGustafsson commented 8 months ago

Could you describe the use case a bit? As you've probably noted in the code, I've worked from the assumption that we should trust ping commands. The plugin can't tell if the command is expensive to run or even if it's idempotent, so leaving it up to the user (basically a for loop or other means of requiring multiple values before changing), ensures that we don't get in the way of some uses of the feature.

Edit: Perhaps I should've just read the issue first where I had already replied. Since the config is per device already it shouldn't be an issue either way. Sorry 😄

jackklink commented 6 months ago

Thanks so much for following up on this! It has been so long since I did this, I can't remember exactly how I left things.

I think pingCommandsToChange is a good idea, but also I do think this feature would benefit most users so up to you if you think making it work with pingsToChange is acceptable. I feel like it makes the plugin much more reliable and most would benefit.

The UI is the one thing I wasn't sure how to update. I was going to leave that and other tweaks up to someone with a little more experience with Homebridge! Ha. Let me know if there's anything I can help with or if you want me to test a build.

jackklink commented 6 months ago

Wait as I look now, I did get it working in the UI with pingsToChange being set to appear in the UI for both normal pings and pingCommands