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

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

Open
xorgy opened this issue Nov 16, 2017 · 15 comments
Open

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

xorgy opened this issue Nov 16, 2017 · 15 comments

Comments

@xorgy
Copy link
Contributor

xorgy commented Nov 16, 2017

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

@xorgy xorgy changed the title isl29018 on Acer C720[P] New ALS: isl29018 on Acer C720[P] Nov 16, 2017
@kootenpv
Copy link
Owner

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
Copy link
Contributor Author

xorgy commented Nov 16, 2017

@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
Copy link
Contributor Author

xorgy commented Nov 16, 2017

@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
Copy link
Contributor Author

xorgy commented Nov 16, 2017

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
Copy link
Owner

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?

@kootenpv kootenpv reopened this Nov 16, 2017
@xorgy
Copy link
Contributor Author

xorgy commented Nov 16, 2017

@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
Copy link

intika commented Oct 22, 2019

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
Copy link
Owner

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
Copy link

intika commented Oct 23, 2019

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
Copy link
Owner

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
Copy link
Owner

kootenpv commented Oct 23, 2019

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
Copy link

intika commented Oct 23, 2019

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:

  • Time in ms for gradual changes
  • Edit saved datas (this is already doable in brightml/data.json)
  • Switch mode (considering that a different mode will be implemented)
  • Custom brightness device location
  • Disable/enable light sensor...
  • etc.

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 brightness changes is not smooth...
  • For me the app is not practical in its current state... (i will explain...)

The main goals of this app from my understanding are:

  • No settings/configuration needed
  • Change the brightness when the current application is too bright/dark
  • Change the brightness depending on the location and time of the day...

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):

  • The app should be as simple as possible (i am thinking of its core)
  • Date/time should be removed
  • Display_pixel_mean should be removed
  • Whereami should be removed
  • Scroll support should be removed as well
  • Display_window_name should be removed as well window class is enough
  • Ambient light is great and should be leveled up to use the camera every 5 min or so.
  • Add smooth transition (i don't know why it does not work in my case because changes applied from KDE are smooth)
  • The application would have just 4 default different profile:
    -- Bright physical location - dark application
    -- Bright physical location - bright application
    -- Dark physical location - dark application
    -- Dark physical location - bright application
  • Basically this mean just 2 different profile, dark and bright one... no further calculation are needed
  • Config: the 4 profiles would be settable
  • Config: we could as well edited what application is listed in what category...
  • Automatic capture of brightness changes can be used to switch an application from one category to an other instead of recording the changed brightness...
  • etc. etc.

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 :) 👍

@xorgy
Copy link
Contributor Author

xorgy commented Oct 27, 2019

@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
Copy link
Owner

@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
Copy link
Contributor Author

xorgy commented Oct 30, 2019

@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.

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

3 participants