Disabling timer with xidlehook-client has no effect - Githubissues

jD91mZM2 / xidlehook

GitLab: https://gitlab.com/jD91mZM2/xidlehook

MIT License

387 stars 33 forks source link

Disabling timer with xidlehook-client has no effect #35

Closed desbma closed 4 years ago

desbma commented 4 years ago

Start xidlehook with a timer to run date after 1s of inactivity:

$ xidlehook --socket /tmp/socket --timer 1 "date" ""

Disable the timer:

$ xidlehook-client --socket /tmp/socket control --timer 0 --action disable
Empty

Check it is disabled:

$ xidlehook-client --socket /tmp/socket query
QueryResult(
    [
        QueryResult {
            timer: 0,
            time: 1s,
            activation: [
                "/bin/sh",
                "-c",
                "date",
            ],
            abortion: [],
            deactivation: [],
            disabled: true,
        },
    ],
)

However the timer still fires after 1s of inactivity....

I'm using xidlehook version 0.8.1 on Arch Linux.

jD91mZM2 commented 4 years ago

Oh my, I can reproduce this. That's not very great, clearly I have too few tests :(

jD91mZM2 commented 4 years ago

The issue seems to be that it's the very first timer! It opens some interesting questions about functionality:

Should the first timer then be ignored even in the cases where it's normally used to calculate the maximum time allowed to sleep (if the first timer activates after 1 hour, and the second timer after 2, the maximum is still 1 because the user could interrupt the 2 hour wait and therefore re-set at 1).
What about when all timers are disabled? Should xidlehook complain, sleep for all eternity, or keep looping like crazy?

jD91mZM2 commented 4 years ago

There we go, feel free to try the latest git commit :)

desbma commented 4 years ago

Well it now "works", but not the way I would have expected.

Disabling the timer makes xidlehook exit, and I would have prefered if it continued to run until I re-enable the timer.

Otherwise I could have just killed xidlehook, which is much simpler than using the socket API.

jD91mZM2 commented 4 years ago

I guess that's a good point, sleeping forever it is!

jD91mZM2 commented 4 years ago

How about https://github.com/jD91mZM2/xidlehook/commit/ad0cc349edffa19e9a93e71b5c48e6a09335dfc6?

desbma commented 4 years ago

Works perfectly, thank you very much !

desbma commented 4 years ago

@jD91mZM2 Can you please make a 0.8.2 release, so that the fix is available in a tagged release? Thanks !

jD91mZM2 commented 4 years ago

Ah yeah, I forgot :| Will do ASAP, although this weekend I am without computer :(

jD91mZM2 commented 4 years ago

And of course I forgot. Again :|

Sorry @desbma!! Will do

jD91mZM2 commented 4 years ago

Released!