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.29k stars 49 forks source link

Let legiond replace fancurve-set completely #195

Closed st0nie closed 2 months ago

st0nie commented 2 months ago

CC @MrDuartePT

fancurve-set is too slow, I replaced its functionality with the legiond module.

Note: User need to migrate their /etc/legiond/.env to /etc/legiond/legiond.ini legiond-cli rename to legiond-ctl to prevent confusion with legion_cli

MrDuartePT commented 2 months ago

Pls Rebase fix typo commit

MrDuartePT commented 2 months ago

@johnfanv2 i think would be a good ideia to add @st0nie as a mantainer

st0nie commented 2 months ago

Pls Rebase fix typo commit

done

MrDuartePT commented 2 months ago

Well if all check pass I will test it and then accept it.

MrDuartePT commented 2 months ago

The fail is not your code problem is my dockerfile is broken

st0nie commented 2 months ago

The fail is not your code problem is my dockerfile is broken

Oh I forget fclose, let me fix it

MrDuartePT commented 2 months ago

The fail is not your code problem is my dockerfile is broken

Oh I forget fclose, let me fix it

This would require changes to the openrc files. I gonna add them to the service folder

st0nie commented 2 months ago

This would require changes to the openrc files. I gonna add them to the service folder

remember to add inih to build dependencies I use this lib to parse ini On gentoo it is dev-libs/inih

https://github.com/benhoyt/inih/blob/master/ini.h

MrDuartePT commented 2 months ago

Thanks for the warning. I probably can accept this tomorrow need to fix pylint and test. Let see if 9999 ebuild accept patches never tried😂 (gonna apply this patch to test)

MrDuartePT commented 2 months ago

I think now is everthing ok

MrDuartePT commented 2 months ago

Fix the OpenRC cpuset service and now I gonna test the PR

MrDuartePT commented 2 months ago

Funny your code not likes lto and clang (parseconf is failing)

 *   Installing legion_linux-9999-py3-none-any.whl to /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/work/lenovolegionlinux-9999-python3_12/install
python3.12 -m gpep517 install-wheel --destdir=/var/tmp/portage/sys-firmware/lenovolegionlinux-9999/work/lenovolegionlinux-9999-python3_12/install --interpreter=/usr/bin/python3.12 --prefix=/usr --optimize=all /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/work/lenovolegionlinux-9999-python3_12/wheel/legion_linux-9999-py3-none-any.whl
2024-04-05 13:13:07,349 gpep517 INFO Installing /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/work/lenovolegionlinux-9999-python3_12/wheel/legion_linux-9999-py3-none-any.whl into /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/work/lenovolegionlinux-9999-python3_12/install
2024-04-05 13:13:07,423 gpep517 INFO Installation complete
make -j14 -l14
clang -march=native -O3 -pipe -fdiagnostics-color=always -flto=thin -o legiond legiond.c modules/parseconf.c modules/powerstate.c modules/setapply.c
clang -march=native -O3 -pipe -fdiagnostics-color=always -flto=thin -o legiond-ctl legiond-ctl.c
ld.lld: error: undefined symbol: ini_parse
>>> referenced by parseconf.c
>>>               /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/temp/legiond.lto.parseconf-835772.o:(parseconf)
>>> referenced by legiond.c
>>>               /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/temp/legiond.lto.legiond-00ccc0.o:(main)
>>> referenced by legiond.c
>>>               /var/tmp/portage/sys-firmware/lenovolegionlinux-9999/temp/legiond.lto.legiond-00ccc0.o:(main)
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:11: legiond] Error 1
 * ERROR: sys-firmware/lenovolegionlinux-9999::guru failed (compile phase):
 *   emake failed

ok the problem is that I not use: -Wall -linih on my CFLAGS

st0nie commented 2 months ago

wait I need do some change

MrDuartePT commented 2 months ago

@st0nie you basically remove all my changes I will go have to force push, you forget to rebase before making changes

st0nie commented 2 months ago

@st0nie you basically remove all my changes I will go have to force push, you forget to rebase before making changes

Sorry, I forget to pull first😢 plz rebase that

MrDuartePT commented 2 months ago

@st0nie you basically remove all my changes I will go have to force push, you forget to rebase before making changes

Sorry, I forget to pull first😢

No problem I will solve this

MrDuartePT commented 2 months ago

@st0nie I removed the 3 commit you added but everthing should be right, you just need to add the docs

st0nie commented 2 months ago

@st0nie I removed the 3 commit you added but everthing should be right, you just need to add the docs

Is legiond.initd wrong? the legiond-ctl reload is used to reload legiond.ini, not the service itself

MrDuartePT commented 2 months ago

@st0nie I removed the 3 commit you added but everthing should be right, you just need to add the docs

Is legiond.initd wrong? the legiond-ctl reload is used to reload legiond.ini, not the service itself

Oh ok, yup it wrong you can solve it. By the way also solve this file: https://github.com/johnfanv2/LenovoLegionLinux/pull/195/commits/64d619c3a4808ef13b907986a2787a1104de70eb#diff-8c01cd0b44a718a015412f7f115b6bb946e4d356c501b80db987dd9562752c3a It should be this:

#!/bin/bash
while true; do
    /usr/bin/legiond-ctl cpuset
    sleep 30
done

edit: Just do this script since openrc dosen't support timers

MrDuartePT commented 2 months ago

Well I notice a file that forget to commit, but do your changes I will add that commit a41743 later

st0nie commented 2 months ago

Well I notice a file that forget to commit, but do your changes I will add that commit a41743 later

Debian && fedora package has legiond_ctl It should be legiond-ctl

https://github.com/johnfanv2/LenovoLegionLinux/pull/195/files#diff-448bbb2cc165f3a6359497be37388889b25c0f9d7d12c7cf4655704e923f598dL72

https://github.com/johnfanv2/LenovoLegionLinux/pull/195/files#diff-88022c22ed7edf0988dc57573eed9c956c01a59e185e09191f513271020afeb9L53

echo "legiond-ctl /usr/bin/legiond_ctl" | sudo tee -a debian/install

install -D -m 0755 %{_builddir}/%{srcname}-%{version}-prerelease/python/legion_linux/legion_linux/extra/service/legiond/legiond-ctl %{_bindir}/legiond-ctl
MrDuartePT commented 2 months ago

Ok I gonna correct it, you need to do more changes @st0nie?

MrDuartePT commented 2 months ago

LGTM See if you want to add anything else.

st0nie commented 2 months ago

LGTM See if you want to add anything else.

^^^^

Find some small problems See the review

st0nie commented 2 months ago

After u fix that I wanna add some TODO to the README of legiond Then we can merge it

st0nie commented 2 months ago

Emmm better use checkbox

st0nie commented 2 months ago

ready to merge 🚀

MrDuartePT commented 2 months ago

Emmm better use checkbox

Org checkboxes seems to not work well on github, but it ok

MrDuartePT commented 2 months ago

@st0nie you have any ideia why ubuntu is failing to compile: https://github.com/johnfanv2/LenovoLegionLinux/actions/runs/8572175765/job/23494315069

It can be the old gcc version

st0nie commented 2 months ago

It can be the old gcc version

maybe need gcc -L argument? Or try setting LIBRARY_PATH to /usr/lib/x86_64-linux-gnu

st0nie commented 2 months ago

@st0nie you have any ideia why ubuntu is failing to compile: https://github.com/johnfanv2/LenovoLegionLinux/actions/runs/8572175765/job/23494315069

It can be the old gcc version

Yeah, it might be image

clang on Ubuntu works fine