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

Jump to bottom

asus-wmi-sensors: init at 0.0.1 #75885

Merged
merged 1 commit into from Feb 4, 2020
Merged

asus-wmi-sensors: init at 0.0.1 #75885

merged 1 commit into from Feb 4, 2020

Conversation

voanhduy1512
Copy link
Contributor

Motivation for this change

Not really sure if this module should be included but i think it would be nice if nixos support asus ryzen and threadripper's motherboard monitor but default.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@Frostman
Copy link
Member

Frostman commented Jan 4, 2020

@voanhduy1512 hey, I've been looking exactly for it ;) could you please share how are you enabling it in your configuration.nix to make it work?

Thanks

@voanhduy1512
Copy link
Contributor Author

voanhduy1512 commented Jan 6, 2020 •
edited

@voanhduy1512 hey, I've been looking exactly for it ;) could you please share how are you enabling it in your configuration.nix to make it work?

Thanks

Hi, If you using this PR, I have these 2 lines in my configuration.nix to make it work

{ config, pkgs, ... }: {
    # ...
    boot.kernelModules = [ "asus_wmi_sensors" ];
    boot.extraModulePackages = [ pkgs.linuxPackages.asus-wmi-sensors ];
    # ...
}

@Frostman
Copy link
Member

It works great for my homelab setup (tested with default and latest kernel on release-19.09 and unstable with ASUS Zenith Extreme Alpha motherboard). I have it enabled and using it for about 3 weeks already without issues.

@ofborg ofborg bot requested a review from Frostman February 1, 2020 06:50
@ofborg ofborg bot requested a review from Frostman February 2, 2020 11:17
Frostman
Frostman approved these changes Feb 3, 2020
View reviewed changes
Copy link
Member

@Frostman Frostman left a comment

Choose a reason for hiding this comment

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

LGTM. nixpkgs-review passes successfully, switched to it in my lab.

4 package marked as broken and skipped:
linuxPackages_4_4.asus-wmi-sensors linuxPackages_4_9.asus-wmi-sensors linuxPackages_hardkernel_4_14.asus-wmi-sensors linuxPackages
_hardkernel_latest.asus-wmi-sensors

12 package built:
linuxPackages-libre.asus-wmi-sensors linuxPackages.asus-wmi-sensors linuxPackages_4_14.asus-wmi-sensors linuxPackages_4_19.asus-wm
i-sensors linuxPackages_5_5.asus-wmi-sensors linuxPackages_hardened.asus-wmi-sensors linuxPackages_latest-libre.asus-wmi-sensors l
inuxPackages_latest_hardened.asus-wmi-sensors linuxPackages_latest_xen_dom0.asus-wmi-sensors linuxPackages_testing_bcachefs.asus-w
mi-sensors linuxPackages_testing_hardened.asus-wmi-sensors linuxPackages_xen_dom0.asus-wmi-sensors

worldofpeace
worldofpeace reviewed Feb 3, 2020
View reviewed changes
pkgs/os-specific/linux/asus-wmi-sensors/default.nix Outdated
makeFlags = [
"TARGET=${kernel.modDirVersion}"
"KERNEL_MODULES=${kernel.dev}/lib/modules/${kernel.modDirVersion}"
"MODDESTDIR=$(out)/lib/modules/${kernel.modDirVersion}/kernel/drivers/hwmon"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"MODDESTDIR=$(out)/lib/modules/${kernel.modDirVersion}/kernel/drivers/hwmon"
"MODDESTDIR=${placeholder "out"}/lib/modules/${kernel.modDirVersion}/kernel/drivers/hwmon"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/nixos-20-03-feature-freeze/5655/20

@ofborg ofborg bot requested a review from Frostman February 3, 2020 05:36
jonringer
jonringer reviewed Feb 3, 2020
View reviewed changes
pkgs/os-specific/linux/asus-wmi-sensors/default.nix Outdated

meta = with stdenv.lib; {
description = "Linux HWMON (lmsensors) sensors driver for various ASUS Ryzen and Threadripper motherboards";
homepage = https://github.com/electrified/asus-wmi-sensors;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
homepage = https://github.com/electrified/asus-wmi-sensors;
homepage = "https://github.com/electrified/asus-wmi-sensors";

pkgs/os-specific/linux/asus-wmi-sensors/default.nix Outdated

stdenv.mkDerivation rec {
name = "asus-wmi-sensors-${version}-${kernel.version}";
version = "0.0.1";
Copy link
Contributor

Choose a reason for hiding this comment

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

since you're collecting from an unofficial revision rather than release

Suggested change
version = "0.0.1";
version = "unstable-2019-11-07";

@ofborg ofborg bot requested a review from Frostman February 4, 2020 03:47
Frostman
Frostman approved these changes Feb 4, 2020
View reviewed changes
Copy link
Member

@Frostman Frostman left a comment

Choose a reason for hiding this comment

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

Re-tested on my system & switched overlay to use it.

@worldofpeace worldofpeace merged commit 49fb456 into NixOS:master Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer left review comments

@worldofpeace worldofpeace worldofpeace left review comments

@Frostman Frostman Frostman approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@voanhduy1512 @Frostman @nixos-discourse @jonringer @worldofpeace