mcontrolcenter: init at 0.4.1

[Tommimon]

MControlCenter provides a relatively full-featured control panel for MSI laptops, written in Qt. It allows users to control MSI laptop power plans, fan speed, battery charging limits, keyboard backlight settings, USB power sharing and more.

The application comes with strong desktop integration: shortcuts, taskbar item, etc. etc. -- screenshots here

The package comes with a patch that corrects MControlCenter's path to the modprobe binary, allowing it to automatically load the ec_sys kernel module (included with NixOS by default) with the right settings when launched.

This package has been previous requested by the NixOS community (#244881). Another pull request (#251816) was submitted for this same package. The pull request has been reviewed but I contacted the author personally and he is not interested in completing it any more. For this reason I'm opening a new pull request incorporating all the changes suggested in the review.

This NixOS package has been tested on a NixOS Plasma5 and GNOME 45 desktops.

fixes: #244881

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 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.

---

Add a 👍 reaction to pull requests you find important.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3847

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-december/1711/21

[nim65s]

Result of nixpkgs-review pr 306216 run on x86_64-linux 1

1 package built:

• mcontrolcenter

[nim65s]

I guess you could move this to pkgs/by-name. I think using libsForQt5.callPackage is not needed, and you can maybe directly extract packages from libsForQt5. Could you try ?

[Tommimon]

I guess you could move this to pkgs/by-name. I think using libsForQt5.callPackage is not needed, and you can maybe directly extract packages from libsForQt5. Could you try ?

I applied all the suggested changes and tested the package on x86_64-linux, everything should work as intended. Here is a list of what I did:

• move to pkgs/by-name
• avoid using libsForQt5.callPackage by extracting packages from libsForQt5
• getting hash from finalAttrs.version
• remove unused cmake flags
• patching with substituteInPlace
• add kmod to build inputs

@nim65s con you confirm the pool request looks fine to you now?

[nim65s]

This looks good to me, thanks !
I do not know if we have a better solution for this DBus .service file.

[nim65s]

Discussions on matrix raised that maybe this .service file (and maybe the desktop file btw) should be proposed upstream. Maybe you can contribute a PR there with those files ?

Also, using pkgs.writeText instead of those echo would be a bit cleaner.

[LordGrimmauld]

Built and tested, launches fine on linux-x86-64 (msi GF75 thin) once the ec-sys kernel module is actually loaded.
However, most options are pretty much locked to something with no way to edit them, with the exception of fan curves. Whether that is due to my extensive customization on tlp, acpi and similar i have not yet fully tested.

[Tommimon]

Built and tested, launches fine on linux-x86-64 (msi GF75 thin) once the ec-sys kernel module is actually loaded. However, most options are pretty much locked to something with no way to edit them, with the exception of fan curves. Whether that is due to my extensive customization on tlp, acpi and similar i have not yet fully tested.

It seams to be an upstream issue, someone else reported the same problems for the same model which apparently is not fully supported.

For me the battery options are working correctly, it might be a problem related to tlp in your case (as you suggested).

[LordGrimmauld]

Ah upstream lists my precise model (GF75 thin 10SDR) as fully unsupported currently, soo i guess thats my hardware just not being supported then

[LordGrimmauld]

Looking good, i would however actually make use of the upstream dbus service file and use substituteInPlace to fix the paths on it.

Also remember to squash your commits eventually, you want two commits:

1. maintainers: add Tommimon where you only add the entry in the maintainers file
2. mcontrolcenter: init at 0.4.1 where all the rest is squashed into it
   The maintainer addition should come first in the history.

[LordGrimmauld]

Discussions on matrix raised that maybe this .service file (and maybe the desktop file btw) should be proposed upstream. Maybe you can contribute a PR there with those files ?

Also, using pkgs.writeText instead of those echo would be a bit cleaner.

In an effort to learn more about nix, i tried doing just that; turns out getting the $out path for mcontrolcenter into writeText is either nearly or completely impossible, at which point you'd have to use substituteInPlace during installPhase anyways. Hence there is no reason why you shouldn't just take the upstream dbus service config and fix that using substituteInPlace instead. I left code to do just that in my suggested changes in my review above.

[LordGrimmauld]

Looking good now 👍

[Tommimon]

I added the suggested changes:

• squashed everything into two commits
• using upstream service file
• using substituteInPlace to patch the Exec path
• using --replace-fail in every other occurrence of substituteInPlace

[nim65s]

Good ! I still have another nit suggestion, but feel free to ignore it if you are tired of those changes 😅

[LordGrimmauld]

approving again because weganks bot will discard my approval otherwise

[Tommimon]

@Aleksanaa, can I ask you if you approve the current state of this pr? After I applied the last changes.