Skip to content

linuxPackages.hid-nintendo: init at 3.1 #108348

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
May 20, 2021
Merged

linuxPackages.hid-nintendo: init at 3.1 #108348

merged 1 commit into from
May 20, 2021
+40 −0

Conversation

rencire
Copy link
Contributor

@rencire rencire commented Jan 3, 2021 •
edited
Loading

Motivation for this change

Add dkms-hid-nintendo "out-of-tree" kernel module.

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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
    • No binary files available; tested with a Nintendo Switch Pro Controller.
    • Added the module to configuration.nix.
      • i.e. boot.extraModulePackages = with config.boot.kernelPackages; [ hid-nintendo ];
    • Output from modinfo: 
      > modinfo hid_nintendo
filename:       /run/current-system/kernel-modules/lib/modules/5.9.16/extra/hid-nintendo.ko.xz
description:    Driver for Nintendo Switch Controllers
author:         Daniel J. Ogorchock <djogorchock@gmail.com>
license:        GPL
alias:          hid:b0005g*v0000057Ep00002007
alias:          hid:b0005g*v0000057Ep00002006
alias:          hid:b0003g*v0000057Ep0000200E
alias:          hid:b0005g*v0000057Ep00002009
alias:          hid:b0003g*v0000057Ep00002009
depends:        hid,ff-memless,led-class
retpoline:      Y
name:           hid_nintendo
vermagic:       5.9.16 SMP mod_unload
    • Tested with Nintendo Switch Pro Controller
    • Controller is detected via either USB or Bluetooth.
    • Tested with games that support SDL by remapping the controller buttons, works fine.
    • Also tested buttons w/ Steam Input; mostly works fine when it is connected. However, bluetooth sometimes disconnects, but outside scope of this PR. (might be related to Add detection of upcoming hid-nintendo joy-con/pro controller kernel driver ValveSoftware/steam-for-linux#6651)
  • 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.

Sorry, something went wrong.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Jan 3, 2021
@SuperSandro2000
Copy link
Member

3 packages failed to build:
linuxPackages-libre.hid-nintendo linuxPackages_4_4.hid-nintendo linuxPackages_4_9.hid-nintendo

Can you please blacklist those kernels to produce less build failures?

@SuperSandro2000
Copy link
Member

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

2 packages marked as broken and skipped:
  • linuxPackages_hardkernel_4_14.hid-nintendo
  • linuxPackages_hardkernel_latest.hid-nintendo
3 packages failed to build:
  • linuxPackages-libre.hid-nintendo
  • linuxPackages_4_4.hid-nintendo
  • linuxPackages_4_9.hid-nintendo
15 packages built:
  • linuxPackages.hid-nintendo (linuxPackages_5_4.hid-nintendo)
  • linuxPackages_4_14.hid-nintendo
  • linuxPackages_4_19.hid-nintendo
  • linuxPackages_5_10.hid-nintendo (linuxPackages_latest.hid-nintendo)
  • linuxPackages_5_9.hid-nintendo
  • linuxPackages_hardened.hid-nintendo
  • linuxPackages_latest-libre.hid-nintendo
  • linuxPackages_latest_hardened.hid-nintendo
  • linuxPackages_latest_xen_dom0.hid-nintendo
  • linuxPackages_latest_xen_dom0_hardened.hid-nintendo
  • linuxPackages_lqx.hid-nintendo
  • linuxPackages_testing_bcachefs.hid-nintendo
  • linuxPackages_xen_dom0.hid-nintendo
  • linuxPackages_xen_dom0_hardened.hid-nintendo
  • linuxPackages_zen.hid-nintendo

@rencire
Copy link
Contributor Author

rencire commented Jan 7, 2021

3 packages failed to build:
linuxPackages-libre.hid-nintendo linuxPackages_4_4.hid-nintendo linuxPackages_4_9.hid-nintendo

Can you please blacklist those kernels to produce less build failures?

thanks for feedback. I'm not sure how to do this in the code. In some of the examples, I see we have an if then else statement guarding against certain versions. Is this the approach to take?

Trying this, and it didn't seem to work?

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/how-to-ignore-specific-kernel-versions-when-building-kernel-modules/10919/1

@KamilaBorowska KamilaBorowska mentioned this pull request Feb 23, 2021
Closed
KamilaBorowska
KamilaBorowska suggested changes Feb 23, 2021
View reviewed changes
Copy link
Member

@KamilaBorowska KamilaBorowska left a comment

Choose a reason for hiding this comment

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

Seems to work for me with JoyCons.

SN30 pro+ in Switch mode did not work, but that's not an official controller, and it seems to be a known issue, so that's fine.

pkgs/os-specific/linux/hid-nintendo/default.nix Outdated
license = licenses.gpl2;
maintainers = [ maintainers.rencire ];
platforms = platforms.linux;
broken = builtins.elem kernel.version ["libre" "4.4" "4.9"];
Copy link
Member

@KamilaBorowska KamilaBorowska Feb 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
broken = builtins.elem kernel.version ["libre" "4.4" "4.9"];
broken = stdenv.lib.versionOlder kernel.version "4.14";

Instead of restricting specific versions, you may want to use versionOlder. linux-libre likely failed due to linux-libre being broken rather than due to an issue with the package, so it shouldn't be explicitly blocked.

pkgs/os-specific/linux/hid-nintendo/default.nix Outdated
@@ -0,0 +1,42 @@
# References:
Copy link
Member

Choose a reason for hiding this comment

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

May as well remove the references, they are generic Linux kernel module packing information rather than anything hid-nintendo specific.

@rencire rencire requested a review from KamilaBorowska March 12, 2021 07:55
KamilaBorowska
KamilaBorowska reviewed Mar 12, 2021
View reviewed changes
pkgs/os-specific/linux/hid-nintendo/default.nix Outdated
license = licenses.gpl2;
maintainers = [ maintainers.rencire ];
platforms = platforms.linux;
broken = stdenv.lib.versionOlder kernel.version "4.14";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
broken = stdenv.lib.versionOlder kernel.version "4.14";
broken = lib.versionOlder kernel.version "4.14";

See #108938.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would work too:

Suggested change
broken = stdenv.lib.versionOlder kernel.version "4.14";
broken = versionOlder kernel.version "4.14";

jakobrs
jakobrs reviewed Apr 5, 2021
View reviewed changes
pkgs/os-specific/linux/hid-nintendo/default.nix Outdated
installFlags = [ "INSTALL_MOD_PATH=${placeholder "out"}" ];
installTargets = [ "modules_install" ];

meta = with stdenv.lib; {
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
meta = with stdenv.lib; {
meta = with lib; {

pkgs/os-specific/linux/hid-nintendo/default.nix Outdated
license = licenses.gpl2;
maintainers = [ maintainers.rencire ];
platforms = platforms.linux;
broken = stdenv.lib.versionOlder kernel.version "4.14";
Copy link
Contributor

Choose a reason for hiding this comment

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

This would work too:

Suggested change
broken = stdenv.lib.versionOlder kernel.version "4.14";
broken = versionOlder kernel.version "4.14";

@rencire rencire requested a review from SuperSandro2000 May 8, 2021 22:48
@jakobrs
Copy link
Contributor

jakobrs commented May 9, 2021

This lgtm, but you should update to 3.1 and squash the commits.

@rencire rencire force-pushed the add-hid-nintendo branch from d31eba3 to b340a79 Compare May 10, 2021 03:55
@ofborg ofborg bot added the ofborg-internal-error Ofborg encountered an error label May 10, 2021
@cole-h cole-h removed the ofborg-internal-error Ofborg encountered an error label May 10, 2021
@cole-h
Copy link
Member

cole-h commented May 10, 2021 •
edited
Loading

@ofborg eval

(GitHub fix your API)

@ofborg ofborg bot added the ofborg-internal-error Ofborg encountered an error label May 10, 2021
@cole-h cole-h removed the ofborg-internal-error Ofborg encountered an error label May 10, 2021
@jakobrs
Copy link
Contributor

jakobrs commented May 10, 2021 •
edited
Loading

I can confirm that this evaluates and builds (on both linuxPackages and linuxPackages_latest), and that the module seems to work (I was able to insmod it). I haven't tried pairing a controller but I don't see why it shouldn't work.

@rencire rencire changed the title linuxPackages.hid-nintendo: init at 3.0 linuxPackages.hid-nintendo: init at 3.1 May 10, 2021
@rencire
linuxPackages.hid-nintendo: init at 3.1

Verified

This commit was signed with the committer’s verified signature. The key has expired.
rnhmjoj Michele Guerini Rocco
GPG key ID: BFBAF4C975F76450
Expired
Verified
Learn about vigilant mode
1c1e51c
@rencire rencire force-pushed the add-hid-nintendo branch from b340a79 to 1c1e51c Compare May 18, 2021 04:24
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/380

@Lassulus
Copy link
Member

Result of nixpkgs-review pr 108348 1

6 packages marked as broken and skipped:
  • linuxPackages-libre.hid-nintendo
  • linuxPackages_4_4.hid-nintendo
  • linuxPackages_4_9.hid-nintendo
  • linuxPackages_hardkernel_4_14.hid-nintendo
  • linuxPackages_hardkernel_latest.hid-nintendo
  • linuxPackages_latest-libre.hid-nintendo
13 packages built:
  • linuxPackages.hid-nintendo (linuxPackages_5_10.hid-nintendo)
  • linuxPackages_4_14.hid-nintendo
  • linuxPackages_4_19.hid-nintendo
  • linuxPackages_5_11.hid-nintendo (linuxPackages_latest.hid-nintendo)
  • linuxPackages_5_12.hid-nintendo
  • linuxPackages_5_4.hid-nintendo
  • linuxPackages_hardened.hid-nintendo
  • linuxPackages_latest_hardened.hid-nintendo
  • linuxPackages_lqx.hid-nintendo
  • linuxPackages_testing_bcachefs.hid-nintendo
  • linuxPackages_xanmod.hid-nintendo
  • linuxPackages_zen.hid-nintendo
  • tests.trivial

@Lassulus Lassulus merged commit 213cde9 into NixOS:master May 20, 2021
@rencire rencire deleted the add-hid-nintendo branch January 3, 2023 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KamilaBorowska KamilaBorowska

@jakobrs jakobrs

@SuperSandro2000 SuperSandro2000

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@rencire @SuperSandro2000 @nixos-discourse @jakobrs @cole-h @Lassulus @KamilaBorowska