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

Tuxedo Control Center #135841

Closed
wants to merge 2 commits into from
Closed

Tuxedo Control Center #135841

wants to merge 2 commits into from

Conversation

blitz
Copy link
Contributor

@blitz blitz commented Aug 26, 2021

Motivation for this change

Tuxedo is a German laptop manufacturer that provides Linux-friendly laptops. Their system control is done via an app called "Tuxedo Control Center" (TCC). This open source app provides fan control settings among other things. Without this app, the Tuxedo laptops default to very noisy fan control settings. It lives on Github.

This PR:

  • adds the Tuxedo Control Center application and daemon, and
  • a tuxedo-control-center module to enable the daemon at boot,
  • bumps tuxedo-keyboard to its latest version.

Testing done:

I've been running TCC on a Tuxedo XP14 laptop without issues. Fan control notably improves compared to running it without TCC.

This PR is based to a large degree on work done by @Vodurden.

Fixes #132206.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 26, 2021
Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please integrate the node package into our nodePackages in pkgs/development/node-packages set. 12000 lines of code for a single package is too much.

@blitz blitz marked this pull request as draft August 27, 2021 18:25
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Aug 27, 2021
@ofborg ofborg bot requested a review from blanky0230 August 27, 2021 18:42
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 11-100 labels Aug 27, 2021
@ofborg ofborg bot requested a review from midchildan August 28, 2021 06:09
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild labels Aug 28, 2021
@blitz
Copy link
Contributor Author

blitz commented Aug 28, 2021

Please integrate the node package into our nodePackages in pkgs/development/node-packages set. 12000 lines of code for a single package is too much.

@Mic92 So in general I agree. I've updated the PR to do this (not yet fully cleaned up). The problem is that TCC needs its devDependencies to build which are not included in nodePackages. This results in build failure:

Error: Cannot find module 'node-addon-api'
Require stack:
- /nix/store/xhnnim063mxkv7y2810yr75zn7j429r5-tuxedo-control-center-1.0.14/lib/node_modules/tuxedo-control-center/[eval]
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:889:15)
    at Function.Module._load (internal/modules/cjs/loader.js:745:27)
    at Module.require (internal/modules/cjs/loader.js:961:19)
    at require (internal/modules/cjs/helpers.js:92:18)
    at [eval]:1:1
    at Script.runInThisContext (vm.js:134:12)
    at Object.runInThisContext (vm.js:310:38)
    at internal/process/execution.js:81:19
    at [eval]-wrapper:6:22
    at evalScript (internal/process/execution.js:80:60) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/nix/store/xhnnim063mxkv7y2810yr75zn7j429r5-tuxedo-control-center-1.0.14/lib/node_modules/tuxedo-control-center/[eval]'
  ]

Other people seem to have this problem when packaging as well (see svanderburg/node2nix#149), without any clear solution. Do you have a pointer?

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 28, 2021
@blitz blitz force-pushed the tuxedo-control-center branch 2 times, most recently from 7620096 to 84bf29c Compare August 29, 2021 14:19
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild and removed 2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 11-100 labels Aug 29, 2021
@blitz
Copy link
Contributor Author

blitz commented Aug 29, 2021

@Mic92 I've tried to integrate TCC into nodePackages, but there are two roadblocks:

  • TCC needs dev dependencies to be avaiable to do its build, this could probably worked around by patching the package.json, but it feels very hacky.
  • it seems that the way nodePackages is composed, it doesn't respect TCC's package-lock.json, so you end up with incompatible libraries / untested library combinations.

So unless I'm missing something obvious packaging, TCC apart from nodePackages seems to be the way to go. It seems I'm also not in uncharted territory here, because there are quite a few Node/Electron packages that go the same route:

% find . -name "node-packages.nix" | wc -l
25

Don't understand me wrong, I would be happy if integrating it into nodePackages works, but I really don't see how. :(

@blitz
Copy link
Contributor Author

blitz commented Dec 19, 2021

I've rebased this and updated Tuxedo Control Center to 1.1.2.

blitz added a commit to blitz/tuxedo-nixos that referenced this pull request Dec 19, 2021
With this commit, the Nix code in this repo is idential to
NixOS/nixpkgs#135841. I'm going to try to keep it in sync from here on
until it's merged in nixpkgs.
@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 16, 2022
@MichaelAkvo
Copy link

MichaelAkvo commented Jan 19, 2022

Is there a way I can use this while waiting for it to get merged?

P.S thanks a lot for the work!

@pgronkievitz
Copy link

Is there a way I can use this while waiting for it to get merged?

P.S thanks a lot for the work!

@blitz has made a module (unfortunately it's not a flake, but I've managed to work around this)

https://github.com/blitz/tuxedo-nixos

@blitz
Copy link
Contributor Author

blitz commented Jan 19, 2022

Is there a way I can use this while waiting for it to get merged?

P.S thanks a lot for the work!

@blitz has made a module (unfortunately it's not a flake, but I've managed to work around this)

https://github.com/blitz/tuxedo-nixos

I'm slowly flakifying my NixOS configs so this repo will also become a flake soon.

@MichaelAkvo
Copy link

@blitz you the real MVP.

Thanks for pointing me to the repo @pgronkievitz

@dasj19
Copy link
Contributor

dasj19 commented Jan 19, 2022

@MichaelAkvo if you are using the unstable branch of nixpkgs you can use the PR author's branch as your base for nixpkgs.
You can clone his fork and switch to the tuxedo branch then run this:
nixos-rebuild -I nixpkgs=/path/to/your/version/of/nixpkgs switch

As for myself I have some migration work to do on my tuxedo computer then I'll start using this and come with feedback

Copy link
Contributor

@dasj19 dasj19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a merge conflict and the following message
error: Package ‘electron-11.5.0’ in /home/daniel/workspace/projects/nixpkgs/pkgs/development/tools/electron/generic.nix:25 is marked as insecure, refusing to evaluate.

I added the electron package to the exceptions for now and started building.

Edit: build finished and got this result:


/bin/sh: line 1: prime-select: command not found
/bin/sh: line 1: prime-supported: command not found
(node:74258) electron: The default of contextIsolation is deprecated and will be changing from false to true in a future release of Electron.  See https://github.com/electron/electron/issues/23506 for more information
(node:74258) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'on' of undefined
    at TccDBusController.onModeReapplyPendingChanged (/nix/store/nd10h9900zy5fvzgsc282n6c5hripn1b-tuxedo-control-center-1.1.2/e-app/common/classes/TccDBusController.js:206:24)
    at /nix/store/nd10h9900zy5fvzgsc282n6c5hripn1b-tuxedo-control-center-1.1.2/e-app/e-app/main.js:150:17
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:74258) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:74258) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@blitz
Copy link
Contributor Author

blitz commented Jan 24, 2022

@dasj19 Thanks for testing!

There is a merge conflict

I've rebased the PR, so this should be gone. If you actually want to use it while it's not merged, I would suggest you head over to this repo.

and the following message error: Package ‘electron-11.5.0’ in /home/daniel/workspace/projects/nixpkgs/pkgs/development/tools/electron/generic.nix:25 is marked as insecure, refusing to evaluate.

Checkout blitz/tuxedo-nixos#troubleshooting and tuxedocomputers/tuxedo-control-center#148.

I added the electron package to the exceptions for now and started building.

Edit: build finished and got this result:


/bin/sh: line 1: prime-select: command not found
/bin/sh: line 1: prime-supported: command not found

This is harmless, unless you have a nVidia PRIME system. In this case, please say so. It would be interesting to see whether we get this to work.

(node:74258) electron: The default of contextIsolation is deprecated and will be changing from false to true in a future release of Electron. See electron/electron#23506 for more information
(node:74258) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'on' of undefined
at TccDBusController.onModeReapplyPendingChanged (/nix/store/nd10h9900zy5fvzgsc282n6c5hripn1b-tuxedo-control-center-1.1.2/e-app/common/classes/TccDBusController.js:206:24)
at /nix/store/nd10h9900zy5fvzgsc282n6c5hripn1b-tuxedo-control-center-1.1.2/e-app/e-app/main.js:150:17
at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:74258) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:74258) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

These need to be reported upstream. If there is specific broken functionality, we can try to patch it. I would welcome help for this.

@blitz
Copy link
Contributor Author

blitz commented Jan 24, 2022

@pgronkievitz If you want to give the flake a shot, there is a flake branch here that exports a module: https://github.com/blitz/tuxedo-nixos/tree/flake

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 24, 2022
@dasj19
Copy link
Contributor

dasj19 commented Jan 25, 2022

@blitz Thanks for your response and fix.

I tried the new changes and it behaves as follows:

  1. When I put tuxedo-control-center in environment.systemPackages in my configuration.nix it ends with the same error you said that should be sent upstream
  2. when I build with hardware.tuxedo-control-center.enable = true; that error does not show up but instead I get only:
/bin/sh: line 1: prime-select: command not found
/bin/sh: line 1: prime-supported: command not found
(node:37258) electron: The default of contextIsolation is deprecated and will be changing from false to true in a future release of Electron.  See https://github.com/electron/electron/issues/23506 for more information

And a blank Tuxedo Control Center shows up.

The same happens when I use https://github.com/blitz/tuxedo-nixos/#usage

My hardware us this https://www.tuxedocomputers.com/en/Linux-Hardware/Linux-Notebooks/15-16-inch/TUXEDO-Book-XA15-Gen10.tuxedo with max specs.

Edit: now it launches successfully using https://github.com/blitz/tuxedo-nixos/#usage I guess I had to kill the previous process

Now a review of the app:

  • In the dashboard, Main Process Monitor is blurred and does not show the cpu fan, frequency or fan speed, fan profile is blurred as well
  • In Profiles tab, fan speed is blurred in all profiles
  • For all the blurred stuff there is a tooltip on mouseover, it says: "This feature is not supported on your model."

@blitz
Copy link
Contributor Author

blitz commented Jan 25, 2022

@blitz Thanks for your response and fix.

I tried the new changes and it behaves as follows:

1. When I put `tuxedo-control-center` in `environment.systemPackages` in my configuration.nix it ends with the same error you said that should be sent upstream

Ah, I didn't realize that you don't use the module. Without the module, you don't get the TCC daemon and there is some dbus magic missing. So this is not expected to work.

Edit: now it launches successfully using https://github.com/blitz/tuxedo-nixos/#usage I guess I had to kill the previous process

Yes, when you start TCC without the daemon and dbus magic it gets stuck in a weird state. I don't think there is anything we an do about it.

Now a review of the app:

It would be interesting, if the limitations you see also happen on Ubuntu or TuxedoOS. If so, this is an upstream limitation.

@dasj19
Copy link
Contributor

dasj19 commented Jan 25, 2022

It would be interesting, if the limitations you see also happen on Ubuntu or TuxedoOS. If so, this is an upstream limitation.

Hi, I figure that the prebuilt binaries for ubuntu 20.04 predates the machine I have and there is no point in trying them. So I tried compiling on ubuntu 22.04 live, but I can't figure out how to get tccd.service as mentioned in step 3. here https://github.com/tuxedocomputers/tuxedo-control-center#development-setup

Whithout the service the app crashes right after launch. So if you know of a tccd.service and tccd-sleep.service it will be handy. Otherwise I am unable to test on Ubuntu

@blitz
Copy link
Contributor Author

blitz commented Jan 26, 2022

So if you know of a tccd.service and tccd-sleep.service it will be handy.

Thanks for trying!

See here for the service files: https://github.com/tuxedocomputers/tuxedo-control-center/tree/master/src/dist-data

@dasj19
Copy link
Contributor

dasj19 commented Jan 26, 2022

@blitz thanks for guiding me so far. I did not succeeded in having a working build from source. The resulted tccd binary gives segmentation fault.
But I installed the deb packages provided by tuxedo for Ubuntu 20.04 on my Ubuntu 22.04 live usb.

The conclusion is that there is fan control support, and none of the features are unsupported. As I mentioned previously in NixOS there were certain parts of the application that printed "This feature is unsupported". In Ubuntu everything was supported.

@blitz
Copy link
Contributor Author

blitz commented Jan 26, 2022

@blitz thanks for guiding me so far. I did not succeeded in having a working build from source. The resulted tccd binary gives segmentation fault.
But I installed the deb packages provided by tuxedo for Ubuntu 20.04 on my Ubuntu 22.04 live usb.

The conclusion is that there is fan control support, and none of the features are unsupported. As I mentioned previously in NixOS there were certain parts of the application that printed "This feature is unsupported". In Ubuntu everything was supported.

Did you reboot after activating the module on NixOS? The module also loads a kernel module that is required for fan control etc.

@dasj19
Copy link
Contributor

dasj19 commented Jan 27, 2022

Did you reboot after activating the module on NixOS? The module also loads a kernel module that is required for fan control etc.

I don't know what happened but after reboot the TCC displays a blank window. When started from terminal I get TUXEDO Control Center is already running. Killing the processes and trying again does not help.

journalctl says:
Jan 27 07:55:31 tuxedo tccd[1445]: CpuWorker: Error validating/reapplying profile => Error: Could not read value from path: /sys/devices/system/cpu/cpu24/cpufreq/cpuinfo_min_freq => Error: ENOENT: no such file or directory, open '/sys/devices/system/cpu/cpu24/cpufreq/cpuinfo_min_freq => Error: ENOENT: no such file or directory, open '/sys/devices/system/cpu/cpu24/cpufreq/cpuinfo_min_freq'

Edit: Tried with both linux 5.15 and 5.16
Edit2: I tried to nail down the problem and I found out that cpufreq is only available for cores 0-23 and not for 24-31 ... as I have 32 cores in total the tccd tries to read information for all of them... I must be hitting some limit that is set to 24 cores

Yes, confirmed with cpufreq-info

analyzing CPU 23:
  driver: acpi-cpufreq
  CPUs which run at the same hardware frequency: 23
  CPUs which need to have their frequency coordinated by software: 23
  maximum transition latency: 4294.55 ms.
  hardware limits: 2.20 GHz - 4.76 GHz
  available frequency steps: 3.50 GHz, 2.80 GHz, 2.20 GHz
  available cpufreq governors: performance, schedutil
  current policy: frequency should be within 2.20 GHz and 3.50 GHz.
                  The governor "schedutil" may decide which speed to use
                  within this range.
  current CPU frequency is 2.20 GHz.
analyzing CPU 24:
  no or unknown cpufreq driver is active on this CPU
  maximum transition latency: 4294.55 ms.
analyzing CPU 25:
  no or unknown cpufreq driver is active on this CPU
  maximum transition latency: 4294.55 ms.

@blitz
Copy link
Contributor Author

blitz commented Jan 30, 2022

@dasj19 Having cpufreq sysfs files only for some CPUs is extremely weird. Do you have an unusual kernel configuration or any other special config?

That being said, given that this discussion is not necessarily regarding packaging, I would propose to continue the rootcausing as an issue in https://github.com/blitz/tuxedo-nixos/.

@NANASHI0X74
Copy link
Contributor

what's the current status of this PR? anyone currently working on it? Is it only still blocked by the upstream electron issue?

@Artturin Artturin added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Apr 13, 2022
@blitz
Copy link
Contributor Author

blitz commented Apr 14, 2022

what's the current status of this PR? anyone currently working on it? Is it only still blocked by the upstream electron issue?

I'm still interested in getting this into Nixpkgs. But upstream using ancient Electron is still an issue. Otherwise there is the issue of checking in gigantic autogenerated Nix files.

@blitz
Copy link
Contributor Author

blitz commented Aug 25, 2022

I'm closing this for now. If someone wants to revive it, feel free to checkout https://github.com/blitz/tuxedo-nixos/

@blitz blitz closed this Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: hardware 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: nodejs 8.has: changelog 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Packaging Request: Tuxedo Control Center
10 participants