zocker-160 / thinkfan-ui

A small gui app for Linux to control the fan speed and monitor temps on a ThinkPad
GNU General Public License v3.0
88 stars 8 forks source link

Add AppIndicator/SysTray icon #4

Closed blipk closed 1 year ago

blipk commented 1 year ago

Hey thanks for this wrapper around the thinkfan fan control, it works great and I use it a lot.

I was finding that it was a bit much having the window open all the time though, so I set up a QSystemTrayIcon on my panel like so:

image

Clicking any of the temp or fan readings will open up your main UI, or fan speeds can be set from the indicators menu without opening the main UI.

Could possibly add a couple of command line arguments to run in either mode if you would prefer.

Let me know if there's anything you want changed to get this merged.

EDIT: I also added in so it makes a call to pkexec to prompt for sudo password if it's needed for setting the fan speed. This is useful as I wasn't able to run it as root without a terminal due to missing QT env arrangements.

@zocker-160 I'm not sure how to assign anyone or if you have a PR process here

zocker-160 commented 1 year ago

Thank you very much for this, I will take a look.

I'm not sure how to assign anyone or if you have a PR process here

All good, since this is a very small project, I don't really have any PR process set up, I actually never expected a PR xD.

I also added in so it makes a call to pkexec to prompt for sudo password if it's needed for setting the fan speed. This is useful as I wasn't able to run it as root without a terminal due to missing QT env arrangements.

So does the start script not work properly?

This part is kinda tricky and I am using the same script that is also used for gparted and timeshift and in my testing it did work, although the theming did break during my testing.

blipk commented 1 year ago

Thanks @zocker-160 have pushed change to fix those review items.

As for the runscript, I get the following QT error:

qt.qpa.xcb: could not connect to display 
qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Available platform plugins are: eglfs, linuxfb, minimal, minimalegl, offscreen, vnc, wayland-egl, wayland, wayland-xcomposite-egl, wayland-xcomposite-glx, xcb.

/usr/bin/thinkfan-ui: line 30: 89977 Aborted                 (core dumped) ${app_command}

If I run it with sudo rather than pkexec it works fine, but then I have to have that sudo terminal section active somewhere. I don't mind too much, it's likely a distro configuration issue.

zocker-160 commented 1 year ago

As for the runscript, I get the following QT error:

Ok weird, it did work just fine for me, but that being said, there seems to be another issue that I just noticed.

When running the entire application as root (using sudo), the tray icon does not show up. Does it work for you? I feel like we are hitting the limit of the current implementation unless there is a way to change the permission requirements for /proc/acpi/ibm/fan.

EDIT: I can run a chmod 777 on that endpoint and it works, would be a potential solution, although I am not sure if it is a desirable one.

zocker-160 commented 1 year ago

Thank you very much!

blipk commented 1 year ago

No worries @zocker-160

The indicator does work for me on GNOME with sudo, but it has different styling. Perhaps a QT env issue on your end? I had to mess around with kvantum theme manager at one point

EDIT: Changing the permissions on that endpoint work for me too, weird as it didn't last time I tried. Maybe just changing the permissions on that file once with pkexec when excepting the PermissionError?

zocker-160 commented 1 year ago

Perhaps a QT env issue on your end? I had to mess around with kvantum theme manager at one point

Yes probably, but I will avoid this environment variable dance by just running the application as user.

EDIT: Changing the permissions on that endpoint work for me too, weird as it didn't last time I tried. Maybe just changing the permissions on that file once with pkexec when excepting the PermissionError?

Yep that is exactly what I am going to do and then I can also get rid of the sudo and pkexec fiddling in the start script.

blipk commented 1 year ago

no worries @zocker-160

I just added changed the command when excepting the PermissionError to:

cmd = [f"""pkexec python -c \"import os; os.chmod('{PROC_FAN}', 0o777)
with open('{PROC_FAN}', 'w+') as soc: soc.write('level {speed}')\""""]

See - https://github.com/zocker-160/thinkfan-ui/compare/master...blipk:thinkfan-ui:master

This works well as I found that the file permissions where getting reset by something any time I rebooted.

zocker-160 commented 1 year ago

This works well as I found that the file permissions where getting reset by something any time I rebooted.

this case should be covered by https://github.com/zocker-160/thinkfan-ui/blob/914e160c3a5043e86419a8b7669b28b3a0ee030f/src/fan.py#L52

which gets executed on start, so even in a use case of doing autostart, this should work