Redshift-gtk doesn't recognize changes to conf

[pik]

Hello - firstly a great thanks for making this - I wish I had found this years ago.

So redshift-gtk does not appear to recognize any changes made to the redshift.conf without quiting & restarting the app. I'm not sure this entirely warranted new issue since the dbus integration would fix this as well (mentioned in #31). If there is a preferred approach I can work on this - I guess there would be a few things on this list which are inter-related:

a) Check if config file has been updated and reload b) Add a GTK config panel accessible from the icon c) Implement dbus support in redshift

[jonls]

Yes, have a look at #54 for the current D-Bus work. Since redshift-gtk is only a thin GUI wrapper around redshift we really can't fix this issue purely in redshift-gtk.

Long-term I think the D-Bus approach is best. Unfortunately I have not had much time lately to work on it. With the D-Bus approach I'm not entirely sure that a configuration file should even be supported anymore. If it is supported, redshift would need to be able to write out and synchronize settings that are changed through the D-Bus interface. There was also an idea of using gsettings as the configuration backend instead of using a configuration file.

[maandree]

I think SIGUSR2 should be added. Then received, redshift should transition to the new settings in the conf file. For automatic updating inotifywait could be used:

set -e
exec 2>/dev/null
while true; do
    inotifywait -e modify,close_write,move_self -- "${XDG_CONFIG_HOME}/redshift.conf"
    pkill -USR2 '^redshift$'
done

[pik]

The signaling might be a little weird; imho in terms of user friendliness reload should either be a polling mechanism or a redshift command-line flag / gui button - e.g. without having to google which kill signal to send to redshift.

[maandree]

Personally I would never want redshift, or any other software, automatically reload configurations. I prefer the traditional way of sending HUP, USR1 or USR2. Of course, I am not opposed to a --reload option in redshift that causes the process to execlp("pkill", "pkill", "-USR2", "^redshift$", NULL);.

[pik]

Fair enough. But I also would really prefer a desktop application that I don't have to terminate to alter config-settings. A redshift --reload command sounds good - but I'm not sure how neat it would be without a redshift dbus server.

[maandree]

I am not saying we should not have a D-Bus server. But that this issue should be implement without D-Bus. Sending a singal to redshift using pkill or redshift --reload should be enough to reload update configurations without terminating redshift. This would make it simple to do so without having to run D-Bus, and it would also make it easy enough to have it autoreload if you want to. Still, D-Bus could be used for this too.

[pik]

Тhat makes sense and for people building without --enable-dbus a change to allow reloading config settings would still be a useful patch, I guess my main point was that app/program functionality should be easily discoverable.

[maandree]

Okey. So the ease of discoverablity should not be a problem. Document both the signal and the flag in the manpage, as well as adding a menu item in redshift-gtk.

[pik]

@jonls Currently all the setting loading and default assignment is one giant main() function (not sure what the original reason was for this was?) - to do this neatly without using longjmp it looks like a lot of it would have to be broken up into separate functions that could be called on if (reload) from run_continual_mode() ?

[pik]

There are also somethings that I think are kind of weird in there:
Why does redshift -O parse the config file? Why does redshift -p print the color temperature set in config and not the one currently set? In that case perhaps there should be separate flags e.g. print-profile and print-current.

If D-Bus support is going to be merged at some-point it might make sense to over-haul a lot of this first -- particularly with all the one-shot actions requiring a different behaviour depending on whether redshift was compiled with --enable-dbus or not.

[jonls]

@pik Agree. Feel free to refactor. The original reason was that we only had command line options (no config file) and very few options to set. Over the years we had a bunch of options and special modes (like -O) tacked on without any thorough clean up of these parts.

[pik]

@maandree Do you think execlp is better than using kill?

[maandree]

You could only use kill if you now the PID of redshift. If so, kill is better. But if you don't, you have to use something like pkill, which is invoked by execlp:ing "pkill".

[pik]

I agree, looping through sysctl output to look for a pid isn't nice, but redshift could write a locked file to /var/run - which might have other advantages (e.g. informing the user if redshift is already running).

[maandree]

A locked file is not too good. There must be support for multiple display instances. For example, I often have two X servers running (:0 and :1), with redshift running in both of them. But that is not enough, sometimes I actually have two instances of redshift running inside the same display.

I think the best solution is to have redshift --reload update all instances of redshift by running pkill. More advanced users could use kill to update a specific instance, and pkill to update all instances (just like redshift --reload). I have also written a tool, dpkill, which could be used to update all instances inside the display.

[maandree]

I found that I have a patch for reloading the settings: #96

[pik]

Ahh I should have looked at that before I started on: https://github.com/jonls/redshift/pull/200

[pik]

@maandree I pushed what I'm running at the moment to #200 , if you had mentioned your branch earlier I would have cleaned up the main() method in there rather than starting a separate one.

I do think switching from the mode enum to flags is important if the command-line program is going to selectively apply d-bus updates via d-bus at some point. But I guess that and the clean-up to main() could be completely separate PR's?

Not sure where to go with this at the moment :O

[vasilakisfil]

any update on this ?