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

vdu_controls('s tray icon) becomes unresponsive after a while (especially when display turns off and on again) #98

Open
major-mayer opened this issue Nov 1, 2024 · 10 comments

Comments

@major-mayer
Copy link

major-mayer commented Nov 1, 2024

I noticed that when I leave my PC on idle for a while and my display turns off, as soon as I move the mouse cursor again the display becomes quite dark (more than I would expect from automatic light adjustment).
So I tried to check why this is the case and wanted to open vdu_controls by left-clicking the tray icon, but nothing happened.
Trying to right-click the tray icon also didn't show any response, but eventually, after a very long delay, the context menu finally opened, but even then nothing opened when i clicked the different menu entries.

I wasn't able to find anything from vdu_controls in the journal, only ddcutil-service created logs regularly: https://pastebin.mozilla.org/ok5htWGs

I tried to open vdu_control normally using the start menu, but this seems to open a second instance, that conflicts with the first one.
At least that's how I would interpret the read errors from my Arduino lux meter: https://pastebin.mozilla.org/t5bNQgmz

The only way to get vdu_controls responsive again, was to kill the process externally and restart it.
Any idea how to debug this problem?

I am using version 2.1.1 with ddcutil 2.1.4

@digitaltrails
Copy link
Owner

Yes, I think I was experiencing this too. You might try 2.1.2 the latest on the main branch which has not yet been formally released. If you look at the version-history notes in the README.md you will see it mentions fixing a potential locking issue.

Just download the latest vdu_controls.py. Move/backup the existing vdu_controls script and replace it.

What I think is happening is that the DPMS-wake event that occurs when the system "wakes-up" is being handled in code protected by a concurrency lock. If just at that moment the application already has the lock because it's doing an update for lux/brightness, then a deadlock occurs and things fall apart.

The DPMS-wake event handling does not need to lock, I think I just copied and pasted code from a more normal operation (which do need locks).

I have been running 2.1.2 for some weeks now, and I haven't experienced any further issues. It's hard to say we're out of the woods for sure though. I've tried to informally enforce an order on locking by reasoning it out. But locking should have really been formally designed in from the start (with a proper enforced locking ordering/protocol).

At the moment 2.1.2 is pretty stable and includes only minor changes. But in the next few dates I will replace or modify the Qt QTimer based eventing because it doesn't account for PC-hibernation (#96). So perhaps grab the commit of vdu_controls as at this date.

@major-mayer
Copy link
Author

Yeah, this locking thing would make sense as far as I understand it ^^
I've downloaded and installed the new Python file in the version that you linked (pretty surprised that the whole program is just one script) and will have a look if I experience this bug again.

Thanks for your swift response as always 🙂 👍

@digitaltrails
Copy link
Owner

pretty surprised that the whole program is just one script

It started out as a small script. I've thought about breaking it down into modules, but having shared the small script, it seemed easier for users to continue having to download only one thing that would just run (no install required). Plus modern IDE's kind of obsolete the concept of files, so breaking it up would be more an exercise in conformity and correctness that something of practical value. I guess it would then be easier to organise unit tests. But at this point, it does 100% of what I need it to do, I'm not keen change it greatly from now on.

@major-mayer
Copy link
Author

Interesting to hear that opinion.
Can you explain what you mean with "modern IDEs obsolete the concept of files"?
You mean because you can find the desired function /class anyway using the advanced search methods they provide?

@digitaltrails
Copy link
Owner

digitaltrails commented Nov 1, 2024

Interesting to hear that opinion.
Can you explain what you mean with "modern IDEs obsolete the concept of files"?
You mean because you can find the desired function /class anyway using the advanced search methods they provide?

Yes, exactly so. I can see the need to break down units somehow into pieces for reasons such as coupling and cohesion in large disjoint libraries and projects. But in a tightly coupled, single focused codebase, such as vdu_controls, the multi-windowed IDE does makes the structure quite navigable. But, for python, I guess that works for me because I still maintain some discipline about minimising the use of globals, trying to avoid excessive coupling, etc - so the language does not fully support my assertion, I have to play ball too.

We still do heavily depend on the file-abstraction for compiler based languages. Even when things are broken down and placed in files, the IDE kind of brings that all back together for us, life has become much easier - the file is less of a barrier.

Yeah, so I think I talking from a coders point of view, the ease of finding and manipulating stuff. So it's a bit of a sweeping statement without that qualification.

@major-mayer
Copy link
Author

Fair enough, for this project I can definitely see your point, even though it surprised me at first.

@major-mayer
Copy link
Author

major-mayer commented Nov 2, 2024

So I just resumed my PC from standby and again the brightness of my secondary display was turned down much more than expected.
With the difference being that this time I was able to get into the light metering settings, so maybe with the newest patch the deadlock was resolved indeed.

Here is what the settings show:
grafik
As you can see, the brightness of the green (primary) display looks what I would expect, but the red (secondary) display is much darker.
From the ambient/display brightness curve, I would expect it to be at ~ 80 % while it's actually at ~ 10%.
I doubt that it's because of incorrect data from the brightness sensor., because then the primary's brightness should be also much lower.

Do you have any explanation for this?
In the journal, I can only find this line:

02.11.24 14:57	vdu_controls	14:57:40 INFO: Wrote config to /home/laurenz/.config/vdu_controls/AutoLux.conf

Edit: I've just found the syslog and debug logs settings, which I now enabled.
Whenever this occurs again, I will check my logs again and post them here.

@digitaltrails
Copy link
Owner

Are you using a recent KDE6? KDE6 assumes KDE is in control of the brightness and makes alterations when locking/unlocking and possibly at other times. If yes, see these two forum posts for suggestions.

https://discuss.kde.org/t/powerdevil-is-re-setting-my-display-brightness-after-the-display-wakes-up/23079/16?u=digitaltrails

https://discuss.kde.org/t/powerdevil-is-re-setting-my-display-brightness-after-the-display-wakes-up/23079/13?u=digitaltrails

@major-mayer
Copy link
Author

Ahh yes I am indeed using KDE Plasma 6.
I've already disabled the automatic screen dimming as recommended by your README, but I didn't know about this Powerdevil environment variable.
I added this to the systemd service, let's see if it resolves the issue.

@digitaltrails
Copy link
Owner

digitaltrails commented Nov 3, 2024

In the journal, I can only find this line:

02.11.24 14:57	vdu_controls	14:57:40 INFO: Wrote config to /home/laurenz/.config/vdu_controls/AutoLux.conf

When ever auto/manual gets set, the config file gets a rewrite. This happens even if the code sets auto to on and it's already on. It doesn't change brightness, the current brightness isn't saved in the config.

If you have debugging on, any VCP change sent to the monitor will be logged, for example:

16:18:10 DEBUG: set_vcp: vdu_number='2' vcp_code='10' new_value=84

So, if vdu_controls is the culprit, the smoking gun will be in the debug log.

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

2 participants