Closed anuvc closed 5 years ago
I am following up on your last posts in #81
Yes, you can subscribe to TEMP_UPD to receive current user-setted temperature, eg: 6500 during the DAY; then you can store it to a global variable; moreover, you can set daytime = -1 in your request, this way it will use current daytime.
Yeah, I saw that. But this feels quite error prone. As far as I understand, I will only get updates as soon as the state changes. So initially, I do not know the user's settings. Why can't I use dbus
to read the settings from within the module? Is it because I cannot communicate with myself via dbus?
Obviously you can merge this custom module with the inhibit_bl one.
Was thinking about that. But first needs to work :)
And please, if you don't mind, share your feedback with custom modules, ie: if you find them hard or whatever :)
So far, it makes sense to me and seems like an easy to understand interface.
I will think of a better approach to inhibit modules, eg: a request with a list of modules to be inhibited.
This sounds exacly like what I need :)
On a second thought, i actually think that inhibiting GAMMA is not really good for your eyes; i don't understand the reason behind that! I can see inhibiting DIMMING and DPMS (obviously...otherwise your screen would switch off/dim during a movie) useful; inhibiting BACKLIGHT already seems quite exaggerated to me, but i can understand that one would wish to set maximum backlight and leave it untouched until the end of a movie. But i cannot see any real use case for GAMMA inhibition. I think you should "fallback" to a custom module for that functionality; the one provided above should work fine.
Well, you are right. We use gamma for a reason. We want to reduce the blue light emission to not simulate daylight for the brain at night. However, watching a movie with gamma 4500 feels a little bit vintage... I prefer removing the blue light cancellation to see 'real'-colors. This might be just a personal thing. But this is why, we can implement as a personal module. :)
As far as I understand, I will only get updates as soon as the state changes. So initially, I do not know the user's settings.
You will get new temp as soon as GAMMA starts and set screen temperature; i think it won't take more than a couple of seconds :)
Why can't I use dbus to read the settings from within the module? Is it because I cannot communicate with myself via dbus?
I decided to hide both conf and state for custom modules to avoid having them changed in wrong ways by custom modules.
But you should be able to parse busctl get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf ...
commands with something like popen and sscanf.
But this is why, we can implement as a personal module. :)
:muscle:
You will get new temp as soon as GAMMA starts and set screen temperature; i think it won't take more than a couple of seconds :)
But do I get both, day and night values? Or only the current one? I need both settings to implement my logic.
I decided to hide both conf and state for custom modules to avoid having them changed in wrong ways by custom modules. But you should be able to parse busctl get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf ... commands with something like popen and sscanf.
I cannot get it working here. if I call busctl
with popen
in init()
, I am getting this:
Failed to get property DayTemp on interface org.clight.clight.Conf: Process org.clight.clight exited with status 1
My assumption is, that it is because clight is not yet fully up and running in init() state. So, I tried to make the calls in my receive()
callback. However, it does not seem to work either:
Failed to get property NightTemp on interface org.clight.clight.Conf: Connection timed out
Code demonstrating both cases:
#include <clight/public.h>
#include <stdlib.h>
CLIGHT_MODULE("INHIBIT_GAMMA");
DECLARE_MSG(temp_req, TEMP_REQ);
int night_temp = -1;
int day_temp = -1;
static void init(void) {
if(night_temp == -1) {
FILE *f;
f = popen ("busctl --user get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf NightTemp", "r");
fscanf(f, "i %d", night_temp);
printf("Night Temp: %d\n", night_temp);
}
if(day_temp == -1) {
FILE *f;
f = popen ("busctl --user get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf DayTemp", "r");
fscanf(f, "i %d", day_temp);
printf("Day Temp: %d\n", day_temp);
}
M_SUB(INHIBIT_UPD);
}
static void receive(const msg_t *msg, const void *userdata) {
switch (MSG_TYPE()) {
case INHIBIT_UPD: {
inhibit_upd *up = (inhibit_upd *)MSG_DATA();
if(night_temp == -1) {
FILE *f;
f = popen ("busctl --user get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf NightTemp", "r");
fscanf(f, "i %d", night_temp);
printf("Night Temp: %d\n", night_temp);
}
if(day_temp == -1) {
FILE *f;
f = popen ("busctl --user get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf DayTemp", "r");
fscanf(f, "i %d", day_temp);
printf("Day Temp: %d\n", day_temp);
}
if (up->new) {
if(day_temp > 0) {
INFO("We are now inhibited! Set daylight screen temperature.\n");
temp_req.temp.daytime = NIGHT;
temp_req.temp.new = day_temp;
M_PUB(&temp_req);
} else {
WARN("Day Temp unknown, not setting anything\n");
}
} else {
if(night_temp > 0) {
INFO("We're not inhibited anymore. Set auto gama.\n");
temp_req.temp.daytime = NIGHT;
temp_req.temp.new = night_temp;
M_PUB(&temp_req);
} else {
WARN("Night Temp unknown, not setting anything\n");
}
}
break;
}
default:
break;
}
}
Clight Log:
[le@w530]: ~>$ clight
SCREEN forcefully disabled as Clightd was built without screen support.
Clightd found, version: 4.0.
'/home/le/.local/share/clight/modules.d/backlight' loaded.
'/home/le/.local/share/clight/modules.d/gamma' loaded.
Set timeout of 3s 0ns on fd 11.
Failed to get property NightTemp on interface org.clight.clight.Conf: Process org.clight.clight exited with status 1
Night Temp: -1
Failed to get property DayTemp on interface org.clight.clight.Conf: Process org.clight.clight exited with status 1
Day Temp: -1
Initial AC state: connected.
New location received: 14.72, -91.16.
Set timeout of 0s 1ns on fd 37.
Emitting Location property
Normal transition to 6500 gamma temp started.
Next alarm due to: Thu Oct 10 17:18:00 2019
Set timeout of 24776s 0ns on fd 37.
AC curve: y = 0.055245 + 0.180897x + -0.008590x^2
BATT curve: y = -0.010629 + 0.153844x + -0.007284x^2
Keyboard backlight calibration supported.
Set timeout of 0s 1ns on fd 40.
Emitting Sunrise property
Average frames brightness: 0.507839.
Ambient brightness: 0.508 -> Backlight pct: 0.752.
Set timeout of 600s 0ns on fd 40.
Emitting Sunset property
Emitting DayTime property
Emitting Temp property
Emitting AmbientBr property
Emitting BlPct property
Emitting KbdPct property
New ScreenSaver inhibition held by vlc: Playing some media.. Cookie: 1804289383
ScreenSaver inhibition enabled.
Being paused.
Failed to get property NightTemp on interface org.clight.clight.Conf: Connection timed out
Night Temp: -1
@FedeDP i am still confused about the chrome/chromium inhibit problem. Can you confirm that it still works on your side. Also, can you give me some information about your setup? Distribution, package version, etc?
Thank you :)
But do I get both, day and night values? Or only the current one? I need both settings to implement my logic.
I see, sorry! I forgot about that...
My assumption is, that it is because clight is not yet fully up and running in init() state
You are probably right; note that with libmodule you will receive PubSub messages wheneveer a module gets started, thus you could wait for "INTERFACE" module started and then get those values: https://libmodule.readthedocs.io/en/latest/src/pubsub.html#system-messages . But there is too much libmodule involved in here :) Here is an example, if you wish to play with it: https://github.com/FedeDP/libmodule/blob/master/Samples/Easy/doggo.c#L82 .
About popen, note that you need to pass variable address to store its value:
FILE *f = popen ("busctl --user get-property org.clight.clight /org/clight/clight/Conf org.clight.clight.Conf NightTemp", "r");
fscanf(f, "i %d", &night_temp); // note "&night_temp" here
printf("Night Temp: %d\n", night_temp);
Note that i am not sure it will work though... I need to play with it when i got time!
Can you confirm that it still works on your side. Also, can you give me some information about your setup? Distribution, package version, etc?
Yes it works; i am on archlinux, latest chromium available, on KDE. On which DE are you on? This is the code that chromium uses to select which interface it should use to inhibit: https://github.com/chromium/chromium/blob/3d20e20f2efed870510c5482b60ad71b80f2cc61/services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc#L441. It seems chromium does support inhibition only when on Gnome, Pantheon, Cinnamon, Unity, Kde, or xfce.
As you can see, on gnome a different bus name is used: https://github.com/chromium/chromium/blob/3d20e20f2efed870510c5482b60ad71b80f2cc61/services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc#L40.
I now see why popen is not working! Stupid me! It cannot work as same process that should answer (clight, that is interface owner), is also waiting for the answer... that is why you get "Connection timed out".
This is the code that chromium uses to select which interface it should use to inhibit
We should probably ask chromium developers to default to freedesktop api on unsupported DEs/WMs by opening a new issue on https://bugs.chromium.org/p/chromium/issues/list :)
This is the code that chromium uses to select which interface it should use to inhibit
We should probably ask chromium developers to default to freedesktop api on unsupported DEs/WMs by opening a new issue on https://bugs.chromium.org/p/chromium/issues/list :)
Thanks for pointing me there. I already have this on my bucket list. But before creating a ticket, I will patch chromium and test whether it works for me or not. Also, if we have a patch, it might get into upstream sooner. :)
Created bug-report here: https://bugs.chromium.org/p/chromium/issues/detail?id=1013812
Workaround to activate inhibition:
XDG_CURRENT_DESKTOP=XFCE chromium
I tried to wait for the INTERFACE
module being loaded. Still timing out when using busctl
. I will propbably stiuck with my hardcoded solution for the following reasons.
1) Interface does not support accessing the config. Everything else are dirty workarounds. 2) My use case seems not applicable for the mainstream, so no point in putting effort into this.
Thanks for the support and maybe at some point there will be a more flexible interface.
Workaround to activate inhibition:
I will add this to wiki page, thanks! It was so easy yet i did not think about that...
Thanks for the support and maybe at some point there will be a more flexible interface.
The best thing here would be to allow custom modules to access clight's conf and state readonly. I am not sure about it though as i don't like custom modules possibly messing with them.
I think, being a custom module, it is fine to hardcode those values. Moreover, you can even parse clight conf file manually to lookup those values, or use an ugly popen("grep XXX /etc/defaults/clight.conf");
and then parse its output.
I added a wiki comment about chrome inhibition: https://github.com/FedeDP/Clight/wiki/ScreenSaver-Specification#chromiumchrome-notes.
Thanks @mindrunner for all your testing and work! I think we can consider this issue closed ;)
I hope to release new clight soonish; i have some more work on Clightd though.
The best thing here would be to allow custom modules to access clight's conf and state readonly. I am not sure about it though as i don't like custom modules possibly messing with them.
I think, this is a good idea.
I think, being a custom module, it is fine to hardcode those values. Moreover, you can even parse clight conf file manually to lookup those values, or use an ugly popen("grep XXX /etc/defaults/clight.conf"); and then parse its output.
Yeah, was thinking about that. But then got lazy... :)
I hope to release new clight soonish; i have some more work on Clightd though.
Cool :) Is there a roadmap? I finally can use clight in production because of the new module and dynamic inhbit stuff. Thats great. No more disfunctional redshift messing with my screen :)
I added a wiki comment about chrome inhibition: https://github.com/FedeDP/Clight/wiki/ScreenSaver-Specification#chromiumchrome-notes.
I would like to add something. I figured out, that using this workaround has more catches. E.g. you get logged out of all sessions if you change this setting.... Not sure, if there is more stuff..
Thanks @mindrunner for all your testing and work! I think we can consider this issue closed ;)
You are welcome! I think so too. Thanks for all the support.
Cool :) Is there a roadmap? I finally can use clight in production because of the new module and dynamic inhbit stuff. Thats great. No more disfunctional redshift messing with my screen :)
I am so proud to hear that!
I think next version is gonna be really huge and i had lot of fun developing all this stuff! And i think it is already well-tested.
I still have to merge a PR in Clightd since 2 months or so; it will greatly improve ambient brightness computation through camera captures; the downside is that it will "break" lots of clight users backlight curves.
This is not a big deal as there is a major version bump too in both Clight and Clightd, thus users should expect some changes.
After that, i should only fix some wiki pages and write a changelog for both :) I hope to release it in a month...but lately i am quite busy and missing all deadlines :D
I would like to add something. I figured out, that using this workaround has more catches. E.g. you get logged out of all sessions if you change this setting.... Not sure, if there is more stuff..
Ouch :( let's hope chromium developers listen to your enhancement request! I will follow it, may be i'll add a comment with a small patch.
You are welcome! I think so too. Thanks for all the support.
Thanks again!
I still have to merge a PR in Clightd since 2 months or so; it will greatly improve ambient brightness computation through camera captures; the downside is that it will "break" lots of clight users backlight curves.
Happy to hear that. For me personally, the default setting is too dark. Especially in combination with gamma shift, it feels exhausting for the eyes and some text is hard to read. i adapted my curve, but I am excited to see more improvements shipped by default :)
Ouch :( let's hope chromium developers listen to your enhancement request! I will follow it, may be i'll add a comment with a small patch.
I was planning to submit a patch, but since I have trouble compiling the source, I could not test it. (Created another ticket for that, but did not get any response.)
Created bug-report here: https://bugs.chromium.org/p/chromium/issues/detail?id=1013812
@mindrunner I saw your chromium bug report got fixed! THIS IS HUGE! :)
Yeah man. Its awesome! I was super stoked as well. Also they did it proper and not in a one line quick fis as i would have been doing hehe. Hope this is in stable builds soon :)
I am getting the following message when I'm starting clight
I understand that there is a fallback in case Inhibit is not found, however I would like to know how to actually solve the issue.
I could not find a solution to that anywhere on the internet or your documentation. It would be good if you can add a section on how to solve the issue.