[Feature] als.none.manual

[salman-farooq-sh]

So I decided to add the table that maps luma ranges to brightness reduction values under als.none. It made sense to think of als.none as also having a manual mode as well that doesn't rely on learning at all but is more, you know... manual. But underneath it just switches to a different predictor when als.none.manual isn't empty.

I am configuring it like this for testing:

[als.none]

[als.none.manual]
50 = 0
100 = 60
150 = 120
255 = 150

How this table is interpreted:

• luma values from 0 to 50 reduce the brightness by 0
• luma values from 50 to 100 reduce the brightness by 60
• luma values from 100 to 150 reduce the brightness by 120
• luma values from 150 to 255 reduce the brightness by 150

There is no sanity check on the new config right now.

Changing the brightness really quickly also has a bit of weirdness. Though I am not sure how to solve that or why that happens.

Also, I am a total beginner with rust so please don't hesitate to suggest any improvements as this probably does have room for improvement.

Resolves: #126

[maximbaz]

Thanks! I'll have a look at the code in the coming days. My main request is that you actually use this in a few days, and tell share your experience here - does it work perfectly, are you sometimes struggling with it, do you wish it to behave differently but can't exactly nail it, etc? I'd really love us to achieve something that you (and hopefully others) will actually want to use, not something that you will abandon after a week because it doesn't really solve the main pain point...

[salman-farooq-sh]

Yeah. I will keep you posted.

A thing I noticed right away is that an absolute brightness reduction, say 50 (out of a total max of 255 for my machine), was too much when the brightness was already near 50 or below. So maybe the reduction should be a percentage of the current brightness value? Like when it is the same brightness of 50, it will now apply 50% reduction instead of an absolute 50 (which makes it zero, bad) resulting in 25 absolute brightness.

Do you have any idea how brightness intensity is perceived by human eyes so that the configuration can be made to be the most natural?

[maximbaz]

Looks like "Weber-Fechner Law" is one good model for this, see e.g. here on just on wiki

https://www.biaslighting.com/blogs/news/the-weber-fechner-law-and-why-you-might-think-the-and-buttons-on-your-dimmer-arent-working

[salman-farooq-sh]

Nice!

I guess reductions being in percentages do a good enough job of decreasing the brightness by more when it is high and by less when it is low, right?

Edit: What I feared was that the human perception might be uneven across the brightness spectrum. Like it wouldn't be a clean law like this; some ranges behave completely differently from the ones before and after. Thankfully that doesn't seem to be the case.

[maximbaz]

I suppose you are thinking about percentages of percentages? So e.g. on my laptop /sys/class/backlight/apple-panel-bl/max_brightness contains 500, so the absolute value of 500 is 100% brightness and 250 for 50%, so when you say you want wluma to reduce brightness by 10%, if I'm currently at 250 = 50%, you want it to be down by "10% of 50%", i.e. to reach 225 = 45% ?

Try, let's see how it feels :) This at least sounds simpler than implementing some kind of power law logarithmic function, we can go that route if percentages don't achieve great results :)

[salman-farooq-sh]

"10% of 50%"

Yes, that's correct, and more simply, just -10% of current 250 brightness in your example which gives the same 250 - 250 * 0.1 = 225. Effectively the same as "10% of 50%" without needing to calculate percentage twice.

I will test, I think this will be enough. Worst case a custom curve is also an option.

[salman-farooq-sh]

[als.none]

[als.none.manual]
5 = 0
7 = 0
10 = 0
15 = 1
20 = 3
25 = 6
30 = 10
35 = 13
40 = 15
45 = 17
50 = 20
60 = 25
65 = 30
70 = 35
75 = 40
80 = 45
85 = 50
90 = 55
100 = 60

The current config I am using for real-life testing, after the latest commit.

@maximbaz What is the upper bound for the computed luma value? I assumed it was 255 but I could only get it upto ~92 I think while displaying a solid white picture that spanned the whole screen.

[maximbaz]

It's 0-100 like a percentage, but there could be a bug, try here, do you get 100 on white with all controls hidden? https://deadpixeltest.org

Make sure you don't run some tools like dimming screen or enabling opacity of inactive windows or something like that, that could potentially affect whiteness of white.

[salman-farooq-sh]

Getting 93 on that link too. With no controls, no black bars around the white, full window opacity, etc.

[maximbaz]

If you have time and desire, let's debug this in a separate issue not to take focus away from your interesting contribution here :) I pushed debug-pixels-on-white branch, it will log entire screen content as rgba (r then g then b then alpha then next pixel r then g etc... - values from 0 to 255), feel free to play around and see why on white it's not fully 255 all around - you can even dump those pixels in a file and then do a simple python script that generates an image out of those rgba values, to actually see what wluma sees :)

---

Back to this PR, I'll go through the code once we get close to the behavior that you are satisfied with. I think it's awesome that you are continuing to experiment, and I feel like we are definitely getting somewhere.

Here are some thoughts, semi-organized 😅

1. Your config has a very clean curve, this is interesting, just curious, nothing more - did you know that ChatGPT can fit data to find polynomial curve? It's very fun 😂

   

2. I'm wondering if the curve has to grow like that because we are working with relative numbers... We are very reactive to luma changes, so imagine you start from 250 brightness in a dark editor, and the screen contents became a little brighter as you switch to browser - you go down say by 10% in brightness to 250-10%=225. Now you switch back to the dark editor - because of the relative numbers, your percentage just has to be larger for you to jump back to the original 250 brightness, right? In other words, 225+10%=247.5, not 250.

3. In fact I'm suspecting if the goal is to be "consistent" (you go 10 times back and forth between editor and browser and after that the brightness the same as it was in the beginning), it's not any kind of curve that will fit, and if we don't achieve consistency, then it feels like throughout the usage the errors will add up and after switching back and forth a number of times, you will e.g. achieve screen brightness too bright or too dark, where you will manually have to correct it.

4. As it stands now, this is a static behavior, once you define the numbers in the config, that's it, and if you need to adjust it, you need to change config and restart wluma. Combine this with the fact that it seems not very obvious how to come up with a curve that will not add up mistakes "too quickly" throughout the usage, so that you dont feel like you have to correct it every 5 minutes... I was wondering if we could flip this somehow, to have a smart self-learning way for wluma to understand that e.g. if you turned brightness down from 250 to 225 while luma was changed from 60 to 70, then this means this is a learning data point for wluma to find some pattern in - buuut, as you correctly have said, this doesn't really fit with the idea that in this manual mode you do actually want to manually change brightness without wluma using it as a new data point 🤔 🤔

What do you think about all this, am I missing something perhaps?

[salman-farooq-sh]

If you have time and desire, let's debug this in a separate issue not to take focus away from your interesting contribution here :) I pushed debug-pixels-on-white branch, it will log entire screen content as rgba (r then g then b then alpha then next pixel r then g etc... - values from 0 to 255), feel free to play around and see why on white it's not fully 255 all around - you can even dump those pixels in a file and then do a simple python script that generates an image out of those rgba values, to actually see what wluma sees :)

For now I am okay with 93 as highest luma because it is easy to work around it in the config for this PR. But if it turns out that the luma calculations are wrong in other ways too then I will definitely look at it. I created a new issue for tracking here: #129

Your config has a very clean curve, this is interesting, just curious, nothing more - did you know that ChatGPT can fit data to find polynomial curve? It's very fun 😂

I actually laughed out loud on this for a full minute. Nice find! I came up with it only semi-randomly.

I'm wondering if the curve has to grow like that because we are working with relative numbers... We are very reactive to luma changes, so imagine you start from 250 brightness in a dark editor, and the screen contents became a little brighter as you switch to browser - you go down say by 10% in brightness to 250-10%=225. Now you switch back to the dark editor - because of the relative numbers, your percentage just has to be larger for you to jump back to the original 250 brightness, right? In other words, 225+10%=247.5, not 250.

In fact I'm suspecting if the goal is to be "consistent" (you go 10 times back and forth between editor and browser and after that the brightness the same as it was in the beginning), it's not any kind of curve that will fit, and if we don't achieve consistency, then it feels like throughout the usage the errors will add up and after switching back and forth a number of times, you will e.g. achieve screen brightness too bright or too dark, where you will manually have to correct it.

Not sure if I understand correctly what you are saying but going back and forth between dark and bright windows the brightness value switches back and forth between the same exact two values. I monitored it like this: while true; set x (notify-send --replace-id=$x --print-id Current\ Brightness:\ (brightnessctl get)); sleep 0.1; end (fish shell)

... buuut, as you correctly have said, this doesn't really fit with the idea that in this manual mode you do actually want to manually change brightness without wluma using it as a new data point 🤔 🤔

Yup.

[salman-farooq-sh]

Was your only concern inconsistent values moving back and forth?

[maximbaz]

I feel like I had a different impression of how this is supposed to work, could you help me understand on an example?

1. Say I start with brightness 100 at luma 50.
2. luma drops to 20 (screen gets darker) - normally you'd want to increase brightness at this point, but does your algorithm increase or decrease it? I assumed that it increased it by 3% (using your config above) to 103. If it decreases it, is the intention that you would increase manually?
3. luma increases to 80 (screen gets really bright), we drop the brightness by 45% as per your config, so 103 - 45% = 57.
4. luma goes back to the original 50, as it gets darker we want to increase brightness again, right? So do we increase it by 20% as per your config, to 57 + 20% = 68?

And so it looks like after this simple trip we returned to the exact conditions on screen (luma 50) but wluma changed brightness from initial 100 to 68, and am I left to increase it again by hand, if I was comfortable at 100?

But now that I'm re-reading your earlier messages and looking at the code, it looks like the code reduces brightness on every that event? So from 100 we went to 100 - 3% = 97, then 97 - 45% = 53, then 53 - 20% = 42 ? That can't be right either, am I going in the wrong direction?

[salman-farooq-sh]

A variable is keeping track of "pre-reduction" brightness such that at all times, the following holds true:

(pre-reduction brightness) - (current reduction calculated based on current luma + config) = (current physical brightness)

This is done by:

• If the physical brightness changes, we adjust pre-reduction brightness variable's value.
• If luma changes, we change the physical brightness.

Using the previously posted config as example where

current physical brightness = b
reduction value to be applied = r
current pre-reduction brightness = pr

1. Say I start with brightness 100 at luma 50.

b = 100
r = 20% of 100 = 20
pr = 100 + 20 = 120

2. luma drops to 20

b = 120 - 3 = 117
r = 3% of 100 = 3
pr = 120

3. luma increases to 80

b = 120 - 52 = 68
r = 45% of 117 = 52
pr = 120

4. luma goes back to the original 50

b = 120 - 20 = 100
r = 20% of 100 = 20
pr = 120

[maximbaz]

I seeee...! Many thanks for explaining - this is clearly a totally different algorithm that I imagined you were developing 😅

I honestly like the idea, your step-by-step scenario helped me a lot to understand exactly why it makes sense 😁

How's it going so far in the last couple of days, are you so far satisfied with how it behaves throughout the day? I'm also curious how often you find yourself adjusting the brightness?

I have now many thoughts, now that I finally am on board with your idea 😅 Maybe just thoughts out loud that we don't have to act upon, but if you have some ideas I'd appreciate your feedback.

1. I suspect it's not a coincidence that you came up with that beautiful curve, it probably makes sense that it's a curve like that, that gives a good experience for the eyes. I'm wondering about UX for a typical user, defining a curve by points is prooobably not the most intuitive thing in the world to do, but if we think that it always is a curve of that shape, maybe a better way could be to have a single parameter that changes steepness of that curve? Some kind of constant in a certain range, that tells wluma to make more or less drastic changes to the current brightness given luma value.

2. There's a certain empowerment in this kind of algorithm of relative adjustments, that I feel like even users who have ALS would appreciate. What happens now is that basically you have to train wluma for every environment condition -you train it once for daylight for different screen contents, then you train it for illuminated evenings, then you train it for a night time, and depending on how sensitive you are to this, you might find yourself training it almost for every different hour of a sunset. Once you train that all, that works wonders and you don't have to touch brightness buttons anymore - but I'm wondering if a blend of the two techniques can be even better - maybe a pattern we learned during the night can be applied during the day, just with a smaller intensity - and that way you have to train wluma even less.

   In other words, maybe what you have now can be considered a special case of an even better predictor that should be used for everyone. One quick idea is that the "pre-reduction" value can be associated with ALS value, and this association can be stored. So imagine that in a day light wluma automatically goes to 90% of "pre-reduction", in evening time it automatically goes to 50% of "pre-reduction" and at night time to 20%. When you don't have ALS value to associate this with, you just don't store any association - i.e. you continue to adjust "pre-reduction" by hand, as your current code does.

[salman-farooq-sh]

Thanks for the admiration.

How's it going so far in the last couple of days, are you so far satisfied with how it behaves throughout the day? I'm also curious how often you find yourself adjusting the brightness?

Working perfectly. Haven't even needed to fine tune the config further. My laptop stays in the same room with windows closed (winter here in Pakistan, basically constant ambience) so have needed to adjust brightness manually much lesser than before. But as this is based on the premise that manual adjustments will be explicitly relied on by wluma, this isn't problematic anyway, I see the point of your question though 😊.

I suspect it's not a coincidence that you came up with that beautiful curve

Yup, semi-randomly reduced the values bit by bit.

it probably makes sense that it's a curve like that, that gives a good experience for the eyes.

Maybe, but what is the logical explanation?

I'm wondering about UX for a typical user, defining a curve by points is prooobably not the most intuitive thing in the world to do, but if we think that it always is a curve of that shape, maybe a better way could be to have a single parameter that changes steepness of that curve? Some kind of constant in a certain range, that tells wluma to make more or less drastic changes to the current brightness given luma value.

Will need a solid explanation first if all the config is going to be reduced to one or two constants, I am not sure what that is at this point.

Even then, this gives exact control to the user in an easy to understand way, just set what reduction for which luma. We can always give magic defaults though which the user can tweak manually if they require. On second thought, I suspect even a simple linear plot will be just as effective, even an inverted curve for that matter, which is what I think you mean. Still, I am not sure, maybe depends on the exact monitor, common luma values in certain workflows etc. and even then might vary person to person. My point is the learning counterpart to this manual thing allows very nuanced predictions, this is in line with that.

I mean, the user can always set a simple config like this to start and go from there, just like I did:

[als.none.manual]
20 = 5
60 = 10
80 = 20

I think we need a number of willing testers with varying setups to get the best answer unless we can come up with a good explanation. Otherwise the best course of action for us in my opinion is to let users do as they desire.

Ahhh, this is a never ending debate if we go down this rabbit hole. I am thinking KDE vs. Gnome.

There's a certain empowerment in this kind of algorithm of relative adjustments, that I feel like even users who have ALS would appreciate. What happens now is that basically you have to train wluma for every environment condition -you train it once for daylight for different screen contents, then you train it for illuminated evenings, then you train it for a night time, and depending on how sensitive you are to this, you might find yourself training it almost for every different hour of a sunset. Once you train that all, that works wonders and you don't have to touch brightness buttons anymore - but I'm wondering if a blend of the two techniques can be even better - maybe a pattern we learned during the night can be applied during the day, just with a smaller intensity - and that way you have to train wluma even less.

So, some manual input / holding the hands for helping wluma learn?

In other words, maybe what you have now can be considered a special case of an even better predictor that should be used for everyone. One quick idea is that the "pre-reduction" value can be associated with ALS value, and this association can be stored. So imagine that in a day light wluma automatically goes to 90% of "pre-reduction", in evening time it automatically goes to 50% of "pre-reduction" and at night time to 20%. When you don't have ALS value to associate this with, you just don't store any association - i.e. you continue to adjust "pre-reduction" by hand, as your current code does.

Nice idea, can be discussed, can you make a new Github issue/discussion for this?

[maximbaz]

Maybe, but what is the logical explanation?

Simply that you want whiter screen contents to reduce brightness more than darker screen contents?

Will need a solid explanation first if all the config is going to be reduced to one or two constants, I am not sure what that is at this point.

Well they can simply be the parameters for the polynomial, say up to cubic one 😁 And we can provide a way to find the parameters, e.g. something like this: https://observablehq.com/@grahampullan/interactive-curve-fitting

What's potentially interesting with this is that it becomes smooth, for every single luma value we can get a corresponding reduction point, so we no longer have ranges of values, and so it will be potentially less noticeable when wluma reduces or increases brightness.

Not saying we have to do anything now, I'm just expressing some considerations 😁

[maximbaz]

Would you like to try how a smooth adjustment would feel like? I'm thinking to just hardcoding the curve that chatgpt fit on your data, it should feel like as if you extended your config for all 100 luma values.

I suppose something like this?

let x = current_luma;
let y = -1.41 + 0.1302*x + 0.0072*x*x - 0.000022*x*x*x;
let brightness_reduction = current_brightness.unwrap() * y / 100;

[maximbaz]

In terms of code layout, we should go away from using "als" term here. I think what we should get is rename predictor/controller.rs into something like predictor/controller/learn.rs and place your new controller in predictor/controller/manual.rs (dont worry about naming too much, we can find good names in the end).

In terms of config, every output and keyboard should have an ability to choose their own predictor, so e.g. just like today every output can choose a capturer (e.g. capturer = "wayland"), so they should be able to choose predictor = "learn/manual" and optionally pass additional data (like your mapping of luma to target value).

Would you like to try to adjust layout? Just to say this explicitly, don't worry if you don't - the important part here is to try different ideas and find a solution we are happy with, I can shuffle the final code around afterwards, no problem 😁

[salman-farooq-sh]

Simply that you want whiter screen contents to reduce brightness more than darker screen contents?

I realize now that with polynomials you can describe anything that is possible with the current config. So my previous argument isn't useful. Curves are a superset of current config.

Well they can simply be the parameters for the polynomial, say up to cubic one

I think current config is easier to understand and write than any curve specified by equations which are pretty abstract to grasp right away just looking at the parameters?

https://observablehq.com/@grahampullan/interactive-curve-fitting

No dependency on anything like this too. It would be another thing if this type of GUI was built into wluma but that is pretty out of scope.

What's potentially interesting with this is that it becomes smooth, for every single luma value we can get a corresponding reduction point, so we no longer have ranges of values, and so it will be potentially less noticeable when wluma reduces or increases brightness.

It feels pretty smooth right now in usage. Room for more smoothness too currently with current config.

Curves might be a bit more expensive to compute multiple times a second? I guess not by much?

[salman-farooq-sh]

Regarding code layout and config keywords etc., I don't have any strong opinion on that stuff so we can do anything that feels good to you there. No issues.

In fact the more I think about it, the more I feel that configuring with curves is alright too other than the points I already mentioned so we can do whatever you as the maintainer feel is the best for the project.

I just want to thank you for the engaging discussions. It was really fun. I find your great attention to detail and UX very inspiring.

[salman-farooq-sh]

And forgot to add, per display config for manual mode is a must, yes. But maybe predictor / choosing manual or auto can be global? What do you say?

[maximbaz]

I think you had good points regarding curve, I think it's nice that it can be concise and smooth, but you are definitely right that it's much harder to read and understand and debug and make slight adjustments.

Regarding the smoothness, I believe the existing predictor in main branch actually smoothens the prediction, it's been a while since I looked at that part, but I imagine it's some kind of linear interpolation - e.g. if we know from learned data that for luma 20 the desired value is 40, and for luma 40 the desired value is 50, then for actual luma 30 the predicted value should be in the middle, i.e. 45 - we could just do the same in your predictor too. It's not exactly the same as a cubic curve (it's more like connecting dots with straight lines), but still should feel smooth despite you just defining a few points!

And forgot to add, per display config for manual mode is a must, yes. But maybe predictor / choosing manual or auto can be global? What do you say?

I think it will be simply easier to have predictor's-additional-data (such as points in this case) together with defining what predictor to use, rather than having two different places (global predictor + per-output link to global predictor with its additional data). Plus potentially you do want to use different predictors, e.g. the one in "main" for keyboard, if you e.g. want your keyboard to be tied to als.time 😁

[salman-farooq-sh]

...and make slight adjustments.

Yeah, this too.

Regarding the smoothness, I believe the existing predictor in main branch actually smoothens the prediction, it's been a while since I looked at that part, but I imagine it's some kind of linear interpolation - e.g. if we know from learned data that for luma 20 the desired value is 40, and for luma 40 the desired value is 50, then for actual luma 30 the predicted value should be in the middle, i.e. 45 - we could just do the same in your predictor too. It's not exactly the same as a cubic curve (it's more like connecting dots with straight lines), but still should feel smooth despite you just defining a few points!

That's a good idea.

Plus potentially you do want to use different predictors, e.g. the one in "main" for keyboard, if you e.g. want your keyboard to be tied to als.time

Yeah you are right.

---

I guess we are at a point now where we can decide what's left to merge this PR. Can you write up what are the final adjustments needed and we can divide the work between us or either one of us can do it all by themselves? What do you say?

[maximbaz]

• Rename predictor/controller.rs into something like predictor/controller/learn.rs and place your new controller in predictor/controller/manual.rs

• Update the code such that this is the config to activate the new feature:

   [als.none]
  
   [[output.backlight]]
   name = "eDP-1"
   path = "/sys/class/backlight/intel_backlight"
   capturer = "wayland"
   predictor = "learn"
  
   [[output.backlight]]
   name = "eDP-2"
   path = "/sys/class/backlight/intel_backlight"
   capturer = "wayland"
   [output.backlight.predictor.manual]
   thresholds = { 5 = 0, 30 = 30, 60 = 60 }

  (if for some reason this is hard to parse I'm of course fine to make adjustments, I just go for a pretty config with minimal duplication really, and less code to support all that). For example, if supporting predictor = "learn" requires too much extra code, let's switch that line for [output.backlight.predictor.learn].

• I've been considering if we should make your predictor also take ALS as input. Yes I know that the whole premise was that there is no real ALS, but hear me out 😅 With ALS such as time, you potentially can define multiple curves to suit your daily activities. Imagine for example that at work you basically dont want wluma to change much or at all, while in the evening you want a steep curve, that luma values make drastic changes to brightness. So the same luma on the same "pre-reduction" brightness will give you more or less reduction based on time or webcam, optionally. That is easy and natural to express with the config below. Do you think this could be a useful addition?

   [als.time]
   thresholds = { 0 = "dark", 8 = "work", 18 = "dark" }
  
  
   [[output.backlight]]
   name = "eDP-1"
   path = "/sys/class/backlight/intel_backlight"
   capturer = "wayland"
   predictor = "learn"
  
   [[output.backlight]]
   name = "eDP-2"
   path = "/sys/class/backlight/intel_backlight"
   capturer = "wayland"
   [output.backlight.predictor.manual]
   thresholds.dark = { 5 = 0, 30 = 30, 60 = 60 }
   thresholds.work = { 5 = 0, 30 = 3, 60 = 6 }

• Smoothen your algorithm by finding reduction % between two points, i.e. given { 5 = 0, 30 = 30, 60 = 60 } and luma 45, reduction should be 45 too. UPDATE: in fact I would be curious to see how the weighted interpolation implemented in predict() would work in this case, i.e. what if we just plug that exact function here on your data points, so that we don't have different interpolation algorithms. I assume it would actually give more or less the same result... It already kinda accounts for having data with multiple curves per ALS too :) If yes, that predict() function should maybe just be extracted into the parent Adjustable interface, and both predictors just call it.

• Things we can take up to the end is to compare your controller with the one in main, for example: we know that initial brightness (upon wluma startup) is being sent to controller, so we don't need to handle special cases about it; consider if cooldown algorithm is necessary to prevent wluma conflicting with your manual changes while you are pressing buttons (i.e. pause wluma for some time to let you finish adjusting brightness), potentially some tests to "document" the expected behavior of this controller (not exhaustive, just to prevent me or other contributors from breaking the desired behavior in future contributions).

[maximbaz]

I can't reproduce the weirdness, whether I hold the button or press-release quickly multiple times. Could it be that you didn't give wluma access to raw driver? (See here). Could you maybe explain in more details what you see and how this looks like?

From what I could experience during testing, this works amazingly!

[salman-farooq-sh]

Odd, I can't reproduce it right now. Never experienced it on main at any point in time though (didn't test on main when it happened at the time of that comment).

My key-repeat rate is/was always 60 times a second, maybe that caused it only at that time?

If you as well can't experience anything weird now with 60+ times a second repeat rate and the code looks fine to you, then I guess it is okay to not worry about it if it doesn't happen again in the coming days (I didn't use my laptop since my previous comment).

[salman-farooq-sh]

So besides this, only writing a few tests remains? Anything else?

What about some extra timeout? Ref. #127 (comment)

[maximbaz]

So first of all I want to say that you've already done the most important part of the contribution - I dare say, we landed on an implementation that is better than perhaps either of us initially envisioned, it is very pleasant to use, and a nice overall addition to wluma! Thank you for sticking for this :)

In terms of what we do next, my first and foremost goal is to ensure that the implementation we have is useful to you, so while working on the final things, if you find ways to reproduce weird behavior or find other issues, identifying and fixing those is the most important.

Other than that, one or few tests would be nice to add, just to seal the idea of this algorithm, i.e. to protect it from future me 😂 Not super necessary by any means. And I'd like to have ALS as an input, just because it changes format of config and potentially adds interesting use-cases with no compromises on your use-case. I'll do some code cleanup afterwards, so dont worry e.g. about that extra timeout.

I'm very flexible on how to proceed - we can continue working inside this PR (e.g. to reduce friction, since both of us can just push), we can open new PRs that merge into this PR, we can merge this one into main and open new PRs on top of main. What do you feel most comfortable with? I should also ask, do you have capacity and/or desire to add those changes, or would you rather be done with this? I'm happy to merge this whenever you want :)

[salman-farooq-sh]

...that is better than perhaps either of us initially envisioned...

Yeah.

...it is very pleasant to use, and a nice overall addition to wluma! Thank you for sticking for this :)

Thanks, and I thank you for all your efforts too, wouldn't have been possible without.

In terms of what we do next, my first and foremost goal is to ensure that the implementation we have is useful to you, so while working on the final things, if you find ways to reproduce weird behavior or find other issues, identifying and fixing those is the most important.

Actually, I realize I had inadvertently made a small change (not anything useful) only my local copy of the code before testing just now. Sorry for that. The bug is still there once I reverted this change. So, can you try reproducing once again while something like this command is running so that you can see the exact values for more info:

# for fish shell:
while true; set x (notify-send --replace-id=$x --print-id Current\ Brightness:\ (brightnessctl get)); sleep 0.01; end

Bug description: after the cooldown period, the first brightness change (increase or decrease) makes the brightness change to what value you attempted to set it to, then instantly reverts back to where you started, then finally settles to what you attempted to set it to initially after a time period that seems to be directly proportional to:

const COOLDOWN_STEPS: u8 = 15; // this value, multiply this by roughly 100ms

I think that's all, everything else works great.

And I'd like to have ALS as an input, just because it changes format of config and potentially adds interesting use-cases...

We can make it a non-breaking change, for example like this: (which I think we should do anyway)

[[output.backlight]]
name = "eDP-2"
path = "/sys/class/backlight/intel_backlight"
capturer = "wayland"
[output.backlight.predictor.manual]

# if this key is present, use it, and ignore any thresholds.* or refuse to start, can be discussed which
thresholds = { 5 = 0, 30 = 30, 60 = 60 }

# if thersholds.* is present, use them as you described earlier
thresholds.dark = { 5 = 0, 30 = 30, 60 = 60 }
thresholds.work = { 5 = 0, 30 = 3, 60 = 6 }

I'm very flexible on how to proceed - we can continue working inside this PR (e.g. to reduce friction, since both of us can just push), we can open new PRs that merge into this PR, we can merge this one into main and open new PRs on top of main. What do you feel most comfortable with? I should also ask, do you have capacity and/or desire to add those changes, or would you rather be done with this? I'm happy to merge this whenever you want :)

Let's merge this one into main after tests and timeout bug, and open new PRs on top of main. Regarding desire/capacity for further PRs, I am afraid I cannot commit or guarantee because I don't have/use an ALS, and this along with the keyboard backlight thing pretty much makes wluma complete for me, at least e.g. until I change laptops and the new one has an ALS.

[maximbaz]

OK! I'll try to reproduce the bug tomorrow, unless you beat me to it with a fix 😁

We can make it a non-breaking change, for example like this: (which I think we should do anyway)

Other parts of code already use the fact that ALS threshold is called "none" when using [als.none], I dont think we need to introduce two different conflicting ways to define thresholds, either leave it as is or feel free to just hardcode thresholds.none instead of thresholds and I'll make it dynamic and match ALS separately, while your config will remain working.

Besides the bug to fix, I'll merge when you tell me you are happy with this PR being merged 👍

[salman-farooq-sh]

I think I didn't understand what you just said.

How are the two thresholds keys conflicting? One is for ALS and one is for manual predictor, they are separate things serving separate purposes. If you are referring to

wluma/src/als/none.rs

Line 8 in db77d27

Ok("none".to_string())

then what does that have to do with the thresholds key under [output.backlight.predictor.manual] possibly being another table (with keys night, dark, dim, and so on)? We can just error out calling it a bad config when [als.none] is used alongside output.backlight.predictor.manual.thresholds.{night,dark,dim,normal,bright,outdoors}.

I am sure I am missing something.

I mean, either it will be like this: (current)

[als.iio]
path = "/sys/bus/iio/devices"
thresholds = { 0 = "night", 20 = "dark", 80 = "dim", 250 = "normal", 500 = "bright", 800 = "outdoors" }

[[output.backlight]]
name = "eDP-2"
path = "/sys/class/backlight/intel_backlight"
capturer = "wayland"
[output.backlight.predictor.manual]
thresholds = { 5 = 0, 30 = 30, 60 = 60 }

or like this: (non-breaking future change)

[als.iio]
path = "/sys/bus/iio/devices"
thresholds = { 0 = "night", 20 = "dark", 80 = "dim", 250 = "normal", 500 = "bright", 800 = "outdoors" }

[[output.backlight]]
name = "eDP-2"
path = "/sys/class/backlight/intel_backlight"
capturer = "wayland"
[output.backlight.predictor.manual]
thresholds.dark = { 5 = 0, 30 = 30, 60 = 60 }
thresholds.work = { 5 = 0, 30 = 3, 60 = 6 }

And we can refuse to run on this: (or prefer one of thresholds.* or thresholds under [output.backlight.predictor.manual] and ignore the other)

[als.iio]
path = "/sys/bus/iio/devices"
thresholds = { 0 = "night", 20 = "dark", 80 = "dim", 250 = "normal", 500 = "bright", 800 = "outdoors" } # this is never an issue

[[output.backlight]]
name = "eDP-2"
path = "/sys/class/backlight/intel_backlight"
capturer = "wayland"
[output.backlight.predictor.manual]
thresholds = { 5 = 0, 30 = 30, 60 = 60 }
thresholds.dark = { 5 = 0, 30 = 30, 60 = 60 }
thresholds.work = { 5 = 0, 30 = 3, 60 = 6 }

I am genuinely curious to know what you mean. Thanks.

[maximbaz]

There's no smart purpose, this is purely due to laziness to maintain advanced config validation logic ( 😅 ), I want the syntax to simply be thresholds.<als_threshold> = { ... }.

In wluma there is no concept of "not having an ALS provider", it always is there, only that in IIO and webcam a user is free to choose their preferred names for als_thresholds (such as dark or dim), while in [als.none] threshold name is hardcoded to be none.

And so this is then a totally valid config that we don't need to error out on - it just picks the correct threshold based on which ALS provider is enabled and what threshold that particular provider reports.

# [als.none]

[als.iio]
path = "/sys/bus/iio/devices"
thresholds = { 0 = "night", 20 = "dark", 80 = "dim", 250 = "normal", 500 = "bright", 800 = "outdoors" }

[[output.backlight]]
name = "eDP-2"
path = "/sys/class/backlight/intel_backlight"
capturer = "wayland"
[output.backlight.predictor.manual]
thresholds.none = { 5 = 0, 30 = 30, 60 = 60 }
thresholds.dark = { 5 = 0, 30 = 30, 60 = 60 }
thresholds.work = { 5 = 0, 30 = 3, 60 = 6 }

[maximbaz]

I saw that on startup this controller instantly goes into cooldown mode, I pushed a small change to prevent that.

Bug description: after the cooldown period, the first brightness change (increase or decrease) makes the brightness change to what value you attempted to set it to, then instantly reverts back to where you started, then finally settles to what you attempted to set it to initially after a time period that seems to be directly proportional to:

Nice bug you caught! It is in fact not specific to your branch, and is even reproducible on main 😁

It is simply a race condition - we periodically predict a good brightness value and sent the prediction further to another thread, but it's not atomic operation, the predicted value isn't instantly being applied. So when you manually change brightness, we stop sending new predictions during the cooldown period, but the last prediction that we sent might have not been applied yet, so it is being applied a millisecond later after you did your manual adjustment.

I think nobody has noticed that before simply because people usually press button several times, so even if wluma undid the first keypress, starting from the second keypress wluma no longer conflicts with you.

You probably also discovered this, as you wrote that the first brightness change gives the weird behavior, whereas the consequent brightness changes within cooldown period no longer bug out.

I can think of some ways to reduce the likelihood of this happening, but I don't think we can ever totally eliminate this, simply because of the nature of this being a shared resource, where we cannot take an exclusive lock to do atomic change. In any case let's not try to fix it in this PR :)

[salman-farooq-sh]

There's no smart purpose, this is purely due to laziness to maintain advanced config validation logic...

I see. But no worries, this is a totally genuine reason too if you feel it will be complex to do.

Thanks for explaining.

And even though I might not be the one to implement this, I say that let's wait at least a few weeks before anyone works on it. It is fine if it will be a breaking change because we can keep this whole PR undocumented behavior for now until we have a final decision on thresholds.<als_threshold>.

[salman-farooq-sh]

I saw that on startup this controller instantly goes into cooldown mode, I pushed a small change to prevent that.

Thanks for the fix.

Nice bug you caught! It is in fact not specific to your branch, and is even reproducible on main...

Ah. Then it is out of scope for this PR indeed.

I guess I will push some tests now and then we merge 🥳.

[maximbaz]

Cool! Why would you like to wait though with adding als support? There is a number of users that run latest code from main branch, and generally I'd be happy to cut a release before new year :) Happy to take that implementation myself after we merge this PR

[salman-farooq-sh]

My thinking is that we might change our minds on:

• whether it is a useful addition or just feature creep
• whether thresholds and thresholds.<als_threshold> can co-exist

And merging this PR doesn't break anyone's existing config as all new config introduced here is optional with defaults being the old stuff.

[maximbaz]

ah I see - at least my mind is already made up, in my eyes this achieves a generic solution, easily composable and opens up more possibilities, I'll go with this unless we discover new arguments before we merge this one.

[salman-farooq-sh]

The premise of als support for the new predictor is that it is useful to essentially specify different "curves" based on different als readings.

But I think that one single curve is enough for every use case.

Let's say my curve is a straight line (which actually works pretty well), I will be happy with the same straight line whether the als says it is a dark room or a bright sunny day. The same reduction percentages based on current luma will work fine in either case.

I think what we want is maybe something like this: like manually changing the brightness adjusts pre_reduction_brightness now, likewise als readings (if available) should also adjust pre_reduction_brightness?

[salman-farooq-sh]

Basically this manual predictor excludes luma from the learning process (making it manually specified).

[maximbaz]

I'm not quite sure how we can transform als value into a pre-reduction brightness adjustment, especially without user input expressing that desire...?

Example with multiple curves is very simple though - at work I want bright browser screen to reduce brightness at most by 10%, while in the evening I want 80% reduction.

[salman-farooq-sh]

...at work I want bright browser screen to reduce brightness at most by 10%, while in the evening I want 80% reduction.

Well when you put it in those magic words it is somehow making sense to me suddenly 😆.

You said the same exact thing with config before but I couldn't understand it then. Sorry for that.

Okay then, I am renaming it to thresholds.none and pushing.

[maximbaz]

Awesome 😁😁

[salman-farooq-sh]

[als.iio]
path = "/sys/bus/iio/devices"
thresholds = { 0 = "night", 20 = "dark", 80 = "dim", 250 = "normal", 500 = "bright", 800 = "outdoors" }

What is "night", "dark", "dim", "normal", "bright", and "outdoors" used for in main?

Edit: Nevermind, I read it in the readme. So it is only used for giving learning data point fields some name?

[maximbaz]

Yeah, to give ranges of ALS values some human-friendly name, and then additionally to associate training data with the named range (also in part because real ALS is logarithmic, so jump from 0 to 10 feels a lot more than from 10 to 100, so it's hard to use it purely as a coefficient).

So when we need to predict a value, we only use training data available for this one particular range only, the current one. In other words, it's again basically different curves for different ALS, the only difference is that with "manual" predictor we define curves in advance, and in "smart" predictor you build the curves as you continue to use wluma and adjust values from time to time, every time you adjust values you are basically giving an additional point to one specific curve, making it that more precise.

[salman-farooq-sh]

Currently hard coded it to always use thresholds.none. Feel free to change later.

[salman-farooq-sh]

Ready for final review.