johnholbrook / streamdeck-pihole

Streamdeck plugin for monitoring & controlling Pi-hole
https://apps.elgato.com/plugins/us.johnholbrook.pihole
32 stars 5 forks source link

Temporary disable no longer works #7

Closed CorneliousJD closed 2 years ago

CorneliousJD commented 2 years ago

With some recent pi-hole update, the temp disable no longer actually works, it will disable it indefinitely until you re-enable it again.

Any chance this could be updated and fixed? i assume something w/ the API changed?

johnholbrook commented 2 years ago

Hmm, thanks for the report - I can have a look into this this weekend.

CorneliousJD commented 2 years ago

Hmm, thanks for the report - I can have a look into this this weekend.

rgr that, no rush on my end, for now I changed it to toggle which will also work fine for the time being. if you need me to test anything let me know :)

johnholbrook commented 2 years ago

for now I changed it to toggle which will also work fine for the time being

That's interesting – the plugin calls the same pi-hole API endpoints for both actions, so it's odd that p-h update would break the 'temporarily disable' action but not the 'toggle' action. Maybe the problem is in my code somewhere.

CorneliousJD commented 2 years ago

for now I changed it to toggle which will also work fine for the time being

That's interesting – the plugin calls the same pi-hole API endpoints for both actions, so it's odd that p-h update would break the 'temporarily disable' action but not the 'toggle' action. Maybe the problem is in my code somewhere.

if it is the code that's the problem it would likely specifically be with the time, e.g. i had 900 seconds as my time to temp disable, but when using it, it just disables it completely and no longer passes the time anymore, but it USED to work for sure, as I used it regularly :)

Firecul commented 2 years ago

At the moment it's disabling using the indefinite mode I think then the script reenables when ready, rather than having pi-hole do the reenabling I think.

johnholbrook commented 2 years ago

At the moment it's disabling using the indefinite mode I think then the script reenables when ready, rather than having pi-hole do the reenabling I think.

This is correct.

I guess it's conceivable that if you set the disable time to a sufficiently large number of seconds, the plugin disables p-h but for some reason 'forgets' to send the API request to enable before the specified amount of time has elapsed. I'll bet if the plugin asks p-h directly to disable for the specified duration instead, that would likely solve this issue.

(For my own reference when I get around to fixing this in the future, the API reference says "Temporal disabling is possible by specifying the amount of seconds, e.g. api.php?disable=10&auth=...")

Firecul commented 2 years ago

I'll bet if the plugin asks p-h directly to disable for the specified duration instead, that would likely solve this issue.

It would and it would also make it more foolproof. Using your example, if someone put the time quite high and then shut down the computer (before the program sent the reenable command) it would be permanently disabled until they remembered/noticed. So I definitely think that would be a worthwhile change.

p1r473 commented 2 years ago

@johnholbrook when will this make it into a new release? Also- I tested it with the current version, and was not having any problems, but agree we should definitely use the built in pihole timer for disabling.

johnholbrook commented 2 years ago

Funny you should ask that: https://github.com/johnholbrook/streamdeck-pihole/releases/tag/v1.1

I've submitted the updated plugin file to Elgato via email, so hopefully it will be available on the App Store within a few days, but it can also be downloaded and installed manually from the releases page.