johnfanv2 / LenovoLegionLinux

Driver and tools for controlling Lenovo Legion laptops in Linux including fan control and power mode.
https://github.com/johnfanv2/LenovoLegionLinux
GNU General Public License v2.0
1.58k stars 58 forks source link

Add releases to ensure some basic stability #56

Closed rozwell closed 1 year ago

rozwell commented 1 year ago

It seems this module is growing every day gaining more features, which is pretty awesome :smiley: However I recently came across stability issues, errors while building a module, DKMS build script missing, etc.

IMHO it will be a good idea to do use releases to give people some anchor points on which version works for them, because main branch is a work in progress and today could be fine but tomorrow could be totally broken - which already happened more than once.

johnfanv2 commented 1 year ago

@rozwell @MasterKia Great idea! I've actually been thinking the same thing. How would you implement releases?

Obviously, I already try to make main working by adding tests that run automatically in the CI.Could you describe all the errors you encountered in detail so I can add tests such that they do not happen in the future? Note that the DKMS script is now replaced by make dkms.

rozwell commented 1 year ago

@johnfanv2 Packages for each distro will be perfect. In the meantime GitHub releases/tags will suffice and will be way easier than checking every commit until one works.

May I suggest pre-releases, which I found very helpful for example with IronOS - I can play with RC if I have time, otherwise I really prefer sticking to stable one to avoid headaches, especially in daily use :wink:

Don't worry about errors I had, these were caused by some typo in the code which got fixed next day or something but it happened when I updated the Kernel and took me little while to figure out it wasn't kernel itself... Since I didn't have much time lately, I'm still using 16889a72605036ca32a33a4a5b9533b08599c2b6 :sweat_smile:

p.s. My apologies for late response.

MrDuartePT commented 1 year ago

In the case of gentoo if the repo is updated and fails to compile I have two option: 1 - Ignore the changes and move on 2 - If I need to reinstall (exemple new kernel) just force the old commit and push the changes to the ebuild in the overlay

On arch never happen because most of the time is a couple version behind and I always test the commit on my gentoo system frist 😄.

For now I think we not need tags because the module and python gui is very stable, and tags will make my work double because now I have to make ebuild for the new “stable version”.

@johnfanv2 always test before pushing (will should use branch when will made huge change even if we delete the branch after) and after that I also test as well (besides new features for new model I only have a gen 6 legion).

I think that can be close for now @rozwell if you think the issue should be reopen pls make a statement 😁

rozwell commented 1 year ago

Looks like releases are there and I really dig .deb builds! This is way better than I ever hoped for! 😁 PPA repository would make it perfect... :wink:

johnfanv2 commented 1 year ago

@rozwell Are you using a Debian based distro with .deb packages? In that case, could you please test the .deb package. It should:

rozwell commented 1 year ago

@rozwell Are you using a Debian based distro with .deb packages?

Yes, Kubuntu 22.04 but with Kernel 6.2.16-060216-generic

In that case, could you please test the .deb package

I already have, they seem to work fine. And there are 2 packages, I used these: v0.0.4-prerelease

  • install kernel module with dkms

lenovolegionlinux-dkms_1.0.0_amd64.deb installed, works

  • install the GUI and CLI

python3-legion-linux_1.0.0-1_amd64.deb installed, works

  • commands sudo legion_gui and legion_cli should be runnable in command line

They are runnable but running sudo legion_gui doesn't show up in the tray, when running as normal user it does but with the warning to run as root

  • legion_gui should be added to the start menu (run as user and run as root) and it should be startable. The GUI should work as root and as user. If you run it as a user, it should also add an icon in the tray and you should be able to minimize it into the tray.

Both are there and both show up in tray when started

  • you should be able to enable "Autostart Legion GUI on Session Startup" to automatically start it after logging in

It's greyed out on Root version. User version seems to be running fine for a couple of days now :)

For now the main problem I have is inability to resize window vertically so bottom buttons get hidden behind taskbar. To see them I need to use "always on top" option. That could be KDE related but I can usually resizes windows at will.

I didn't test all the features but basics seem to be working fine.

MrDuartePT commented 1 year ago

I also never use the root version never work well for me but that is also I’m on Wayland. But at least the user one work. @johnfanv2 i think it possible to not prompt the password when changing option and values you think I should try to implement this is the gui and cli or better not for “security reason”?

johnfanv2 commented 1 year ago

It should only ask once (in 15 minutes?) for the password when changing something. This is done by the legion_cli.policy file.

MrDuartePT commented 1 year ago

Yha I know but what me to remove that timer and never ask for password or is better to ask?

rozwell commented 1 year ago

BTW, I've made myself a white monochromatic icon for dark theme:

obraz

Should I upload it? ;)

johnfanv2 commented 1 year ago

Yha I know but what me to remove that timer and never ask for password or is better to ask?

I guess one has to ask at least once. Otherwise, a user or any program could just set all fan speeds to 0 and overheat the laptop.

johnfanv2 commented 1 year ago

BTW, I've made myself a white monochromatic icon for dark theme:

obraz

Should I upload it? ;)

Looks goood. Please create a pull request and put it into the folder python/legion_linux/legion_linux next to the logo in color.

MrDuartePT commented 1 year ago

BTW, I've made myself a white monochromatic icon for dark theme:

obraz

Should I upload it? ;)

@rozwell ping me after open the PR to update the python and deb build. I like that design. If you can make also light mode onde (basically change the monochrome to dark). @johnfanv2 if you agree we can make it the default for the tray.

MrDuartePT commented 1 year ago

Yha I know but what me to remove that timer and never ask for password or is better to ask?

I guess one has to ask at least once. Otherwise, a user or any program could just set all fan speeds to 0 and overheat the laptop.

Well in that case let stay the way it is right now.

rozwell commented 1 year ago

@johnfanv2 I've noticed one issue with Autostart - it seems after rebooting it works as expected, but when logging in after power off, it seems both User & Root instances are started and the other shows an error Program must be run as root user! I'll create a ticket for that.

@MrDuartePT I should create icons PR today ;)