Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for new monitor overrides feature? #12

Closed
taprobane99 opened this issue May 2, 2021 · 12 comments
Closed

Support for new monitor overrides feature? #12

taprobane99 opened this issue May 2, 2021 · 12 comments

Comments

@taprobane99
Copy link

i notice that if I add a monitor override manually to the .conf and then changes some settings in GUI and press store, the monitor override is deleted!

could this be fixed?

and of course, also, a new Tab in the GUI to insert custom monitor overrides, when you have time.

thanks

@FedeDP
Copy link

FedeDP commented May 2, 2021

Store just calls clight dbus Store method.
If it is not working, it is a bug in clight!
Anyway, it is working fine for me.
Are you able to reproduce it on latest clight master?

@nullobsi
Copy link
Owner

nullobsi commented May 2, 2021

ah, that is strange.... it could be a clight issue if it doesn't work on the latest master, but I will look into adding the option in the gui in a bit

@taprobane99
Copy link
Author

It did not work on a build from yesterday, and unfortunately can't test again for a few days. Will update then.

@nullobsi
Copy link
Owner

Hi,

i have finally gotten around to looking into adding the feature. Unfortunately Clight does not expose these properties under the DBus interface... @FedeDP , is this something that is on the roadmap?

however i would be happy to add it to the Sensor tab, perhaps under a dropdown for each monitor ID

@FedeDP
Copy link

FedeDP commented May 30, 2021

Hi!
I am not sure I'd expose the monitor_override property through dbus, it seems a pretty "niche" use case and I'd avoid adding the complexity for it!
If anyone has different opinions, please share!

@jclsn
Copy link

jclsn commented Aug 2, 2021

+1

@FedeDP
Copy link

FedeDP commented Aug 15, 2021

Hi!
Latest clight commit: cd174174d0d67b0816fcc1429d5e8d2be1374fc6 implements dbus api around monitor override; it is quite simple, just exposing 2 methods: List() and Set().
Here the relevant doc: https://github.com/FedeDP/Clight/wiki/Api#orgclightclightconfmonitoroverride
I am here for any question @nullobsi !

@nullobsi
Copy link
Owner

looks good!

ill be brainstorming a good implementation and ill get something working in a bit :)

@FedeDP
Copy link

FedeDP commented Aug 15, 2021

In my opinion, it would be best to present the user with a list of specific curves used right now by Clight (ie: calling List()), then a "+" button to add a new one (and a "-" button to remove already present ones);
when clicking the "+" button, a list of monitors currently attached (obtained using Clightd Backlight.GetAll method), will be presented and the user can then choose the one she/he needs and set the curve points.

Note: to remove a specific curve ("-" clicking) you should just pass its serial and 2 empty arrays, eg:
$ busctl --user call org.clight.clight /org/clight/clight/Conf/MonitorOverride org.clight.clight.Conf.MonitorOverride Set "sadad" "amdgpu_bl0" 0 0
I will add a note on clight wiki.

@nullobsi
Copy link
Owner

that sounds like a good idea
I think I will add a drop-down to the top of the Sensor tab's pane, with the plus/minus button, and there will always be a Default option

@jclsn
Copy link

jclsn commented Aug 15, 2021

Ah you are working on it. Alright :)

@nullobsi
Copy link
Owner

nullobsi commented Nov 1, 2021

i finally merged the PR, let me know if there are any issues....

@nullobsi nullobsi closed this as completed Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants