search

kootenpv / brightml

Convenient Machine-Learned Auto Brightness (Linux)
MIT License
120 stars 4 forks source link

New ALS: isl29018 on Acer C720[P] #7

Open xorgy opened 6 years ago

xorgy commented 6 years ago

On the Acer C720 (and C720P), there is an ambient light sensor with a light value at /sys/bus/iio/devices/iio:device0/in_illuminance0_input and a name "isl29018\n" in /sys/bus/iio/devices/iio:device0/name

kootenpv commented 6 years ago

Fantastic :)! This is what open source is about. This file is actually also present on my machine, so it's a nice generalization. If you don't mind, I'll just actually update the code to use "your" path (and parsing) instead. Though: is there a reason we need to know the name? I could just imagine:

Replacing the original:

def get_ambient_light():
    try:
        # please, if your ambient light sensor doesn't work, post the path in the issues on github
        with open("/sys/devices/platform/applesmc.768/light") as f:
            return int(f.read()[1:-1].split(",")[0])
    except:
        return np.nan

with 

def get_ambient_light():
    try:
        # please, if your ambient light sensor doesn't work, post the path in the issues on github
        with open("/sys/bus/iio/devices/iio:device0/in_illuminance0_input") as f:
            return int(f.read().strip())
    except:
        return np.nan
xorgy commented 6 years ago

@kootenpv I guess that's true. I suppose the zeroth iio device is most likely to be an ALS if there is one at all. I'll just update my patch with your changes. :- )

xorgy commented 6 years ago

@kootenpv I've updated my PR to use your approach, except I removed the .strip() (I don't think it's necessary, the only whitespace would be a line feed, and int(str) only cares about the first run of digits in the string anyway, so it'd parse despite whitespace on both sides, making the strip() pointless).

xorgy commented 6 years ago

I think it would be wise to enumerate all of the iio devices to look for a device with an "illuminance" channel though. I think iio is also used for orientation and enclosure air temperature sensors and things of that sort, so device0 could just as easily be a sensor-fused IMU as it could be an ALS, especially on a tablet or convertible.

Update: see #9 for more details on this.

kootenpv commented 6 years ago

That's a good point to make. I guess not as likely that it will be different on a machine on which you want to manage a backlight, but still possible. I'll keep the issue open; but I think we need more examples for people in which it wouldn't work?

xorgy commented 6 years ago

@kootenpv I think this issue is addressed, #9 should be a better home for any further work. I think I might have access to a device with more iio devices than the ALS, so I'll take a look.

intika commented 4 years ago

Some info before going nut here with a C720P, (the device does not have a sensor) the module isl29018 does load but /sys/bus/iio/devices is empty... so not all c720 have a sensor... here is some pics. (just posting for pple with the same issue)

1

2

3

4

kootenpv commented 4 years ago

Amazing... love the screenshots haha.

For the record: note that I disabled using my ambient light sensor as input, and it still works well (even though it is the best feature, without it, it can still do quite a good job).

intika commented 4 years ago

By the way still on the same device... the brightness change is not smooth it just jump to an other level... it would be nice if you add gradual change... (i did not create an new issue because i am using open issue listing to track my other works...)

Also, it would be very cool if the app had some tune-able settings over a config file or something

kootenpv commented 4 years ago

I was considering doing something for a gradual change, but indeed in my experience it felt smooth already. So it makes sense what you said! Maybe you could clone and install in editable mode (pip install -e .) and experiment in brightness.py with making it scale (linearly?) between an old (need to introduce state) and new value in X seconds (possibly lower than 1). Let me know if you're interested :)

kootenpv commented 4 years ago

Also, what kind of config are you thinking? I haven't yet felt a need to use config. Note that it goes against the ideals but if they are good features we should add it of course.

intika commented 4 years ago

Also, what kind of config are you thinking? I haven't yet felt a need to use config. Note that it goes against the ideals but if they are good features we should add it of course.

I am thinking of:

But as you say this will make the project deviate from its main objective... any way here are the issues that i am facing:

The main goals of this app from my understanding are:

In practical usage i found that brightml act too often and most of the time the changes are not needed (like between dark apps)... i am not criticizing your work it's amazing, i just want to share how i see it :) ... i'll may be get my hand dirty if i have the time for it :D and add those changes in a different mode of the application... any way, here is how i see the app (this is just my point of view of course):

I am just laying down how i see the application and i think it will be more easy to actually develop a new mode/fork other than speak about it lol :D... just to play a little bit with the code :D and see what happen :p

Any way this or that idea could come handy or interest you ;)

One again thanks for this great app :) :+1:

xorgy commented 4 years ago

@intika

The brightness changes are not smooth...

It can also be too slow.

Disable/enable light sensor...

I've also noticed that on the Acer C720P, the screen itself can light up the ALS (which is located in front of the screen behind the keyboard).

(I'm just laying down how I see the application, and I think it will be easier to just develop a new mode/fork than to speak about it, lol.

It usually is.

kootenpv commented 4 years ago

@xorgy @intika It's better to make new issues for specific issues, though obviously any feedback is appreciated.

@intika Replying to:

In practical usage i found that brightml act too often and most of the time the changes are not needed (like between dark apps)... i am not criticizing your work it's amazing, i just want to share how i see it :)

You haven't taken enough samples yet :) Just keep correcting it a few more times and I literally now see no % change when switching between similarly black apps :)

@intika Replying to removing functionality and changing things to how you like them

I understand where you are coming from, I add a lot of features that most likely won't be important to everyone. However, the beauty is that it is up to the ML and will be able to learn complex functions if the situation exists. Maybe you don't care about difference between being on your "couch" vs "being at work", but some others might. No one will need to change config though to be able to use this. You are not obliged to use whereami either: if it is installed, it will use it, otherwise, it is just a missing value. I guess the scrolling should be optional though. At some point I will probably make this a CLI feature.

Lastly: I completely agree that one breakthrough in what brightml does is use the mean pixel value. I would definitely recommend anyone that makes an automatic brightness app for laptops to consider the "inner" brightness (the mean pixel value). So feel free to use any part of brightml's code and make auto brightness ran on simple config code. It just goes against the philosophy of what we're creating here. But I certainly see value if it would be possible to create an easy to use working solution for everyone without much config.

xorgy commented 4 years ago

@kootenpv

I would definitely recommend anyone that makes an automatic brightness app for laptops to consider the "inner" brightness (the mean pixel value).

A great place to do this would be at the level of the compositor, especially if the mean can be done quickly in the GPU, and can know when a window is done drawing its unfocused state.