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

openrazer: init at 2.3.1 (driver, daemon, lib and NixOS module) #47009

Closed
wants to merge 1 commit into from

Conversation

@roelvandijk
Copy link
Contributor

roelvandijk commented Sep 20, 2018

Motivation for this change

I want to disable the annoying 'breath' effect of my Razer Blackwidow 2016 keyboard.

Things done
  • Kernel drivers for talking to Razer products (keyboards, mice, etc.)
  • Daemon for talking to the kernel drivers.
  • Python library for talking to the daemon.
  • NixOS module hardware.openrazer which adds the kernel drivers and daemon to a system.

Enabled the NixOS module on my system. Tested with various Razer devices. Was able to successfully control the devices via the packaged python library.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

cc @FRidh because of Python packages

pkgs/os-specific/linux/openrazer/driver.nix Outdated
description = "An entirely open source driver and user-space daemon that allows you to manage your Razer peripherals on GNU/Linux";
homepage = https://openrazer.github.io/;
license = stdenv.lib.licenses.gpl2;
maintainers = [ stdenv.maintainers.roelvandijk ];

This comment has been minimized.

Copy link
@basvandijk

basvandijk Sep 20, 2018

Member

This should be stdenv.lib.maintainers.roelvandijk (note the evaluation error reported by ofborg). Better yet place a with stdenv.lib; after the meta = so that you don't need to repeat yourself.

pkgs/os-specific/linux/openrazer/driver.nix Outdated
let
openrazerSrc = import ./src.nix;

razermountPATH = stdenv.lib.makeBinPath [ coreutils ];

This comment has been minimized.

Copy link
@basvandijk

basvandijk Sep 20, 2018

Member

This doesn't seem to be used.

pkgs/os-specific/linux/openrazer/daemon.nix Outdated
sourceRoot = "source/daemon";
patches = [
# https://github.com/openrazer/openrazer/pull/681
./openrazer_python_daemon_script.patch

This comment has been minimized.

Copy link
@basvandijk

basvandijk Sep 20, 2018

Member

Instead of storing the patches in nixpkgs you might be able to refer to them using:

(fetchpatch {
  url = https://github.com/openrazer/openrazer/commit/feefb86497af6ff7db827e682ae560c12d0aee2f.patch;
  sha256 = "...";
})

This comment has been minimized.

Copy link
@roelvandijk

roelvandijk Sep 21, 2018

Author Contributor

The patches from the pull requests are relative to the entire openrazer repository. The patches stored in nixpkgs are relative to the sourceRoot "source/daemon". We could of course take the patches from github and rewrite them to be relative to "daemon".

This comment has been minimized.

Copy link
@basvandijk

basvandijk Sep 21, 2018

Member

You can use the stripLen option of fetchpatch to strip of the "daemon/" prefix like:

(fetchpatch {
  url = https://github.com/openrazer/openrazer/commit/feefb86497af6ff7db827e682ae560c12d0aee2f.patch;
  sha256 = "...";
  stripLen = 1;
})

This comment has been minimized.

Copy link
@roelvandijk

roelvandijk Sep 21, 2018

Author Contributor

Done

@basvandijk basvandijk self-requested a review Sep 20, 2018
Copy link
Member

basvandijk left a comment

Looks pretty good. I wrote some minor issues and recommendations inline.

Kernel drivers for talking to Razer products (keyboards, mice, etc.)
Daemon for talking to the kernel drivers.
Python library for talking to the daemon.

Adds NixOS module hardware.openrazer which adds the kernel drivers and
daemon to a system.
@roelvandijk roelvandijk force-pushed the LumiGuide:feat-openrazer branch to fa90486 Sep 21, 2018
@basvandijk

This comment has been minimized.

Copy link
Member

basvandijk commented Sep 21, 2018

@GrahamcOfBorg build linuxPackages.openrazer python3Packages.openrazer python3Packages.openrazer-daemon

@GrahamcOfBorg

This comment has been minimized.

Copy link

GrahamcOfBorg commented Sep 21, 2018

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: linuxPackages.openrazer, python3Packages.openrazer, python3Packages.openrazer-daemon

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg

This comment has been minimized.

Copy link

GrahamcOfBorg commented Sep 21, 2018

Success on aarch64-linux (full log)

Attempted: linuxPackages.openrazer, python3Packages.openrazer, python3Packages.openrazer-daemon

Partial log (click to expand)

writing manifest file 'openrazer.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/0574z2wb4rm17xmz0mhjg8zwx31zjsp7-openrazer-2.3.1-4.14.71
/nix/store/8vg5l6r9flb1j0dykaglx8qzv651jidi-python3.6-openrazer-2.3.1
/nix/store/60qb6k9wjav06zzsc3lj487smr1bb467-openrazer_daemon-2.3.1

@GrahamcOfBorg

This comment has been minimized.

Copy link

GrahamcOfBorg commented Sep 21, 2018

Success on x86_64-linux (full log)

Attempted: linuxPackages.openrazer, python3Packages.openrazer, python3Packages.openrazer-daemon

Partial log (click to expand)

writing manifest file 'openrazer.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/6cplla4387falgc3in7jc8diahhfpp2g-openrazer-2.3.1-4.14.71
/nix/store/mzdpxlyrahfskcq1bmnlj3r7nniqw70h-python3.6-openrazer-2.3.1
/nix/store/lj3jjd69mi26wsnjjvvjqj8p8k5nclws-openrazer_daemon-2.3.1

@roelvandijk

This comment has been minimized.

Copy link
Contributor Author

roelvandijk commented Sep 21, 2018

Note: there is a similar effort at lezed1@9b9cdf6 . It could be useful to compare both approaches.

@lezed1

This comment has been minimized.

Copy link

lezed1 commented Sep 21, 2018

@roelvandijk the biggest differences between ours I found on my phone was manually setting up service files vs using the ones that come from the repo. I'll look at it closer tonight.

@basvandijk

This comment has been minimized.

Copy link
Member

basvandijk commented Sep 28, 2018

@roelvandijk what about using @lezed1's trick of having a common.nix file instead of src.nix? This saves a bit of common code. I implemented this at:
LumiGuide/nixpkgs@feat-openrazer...LumiGuide:feat-openrazer-common

@z3ntu z3ntu mentioned this pull request Oct 28, 2018
@@ -3545,6 +3545,9 @@ in {

odfpy = callPackage ../development/python-modules/odfpy { };

openrazer = callPackage ../os-specific/linux/openrazer/pylib.nix { };

This comment has been minimized.

Copy link
@FRidh

FRidh Nov 4, 2018

Member

The expressions need to move under pkgs/development/python-modules/*.
Also, fix indentation.

@DanielFabian

This comment has been minimized.

Copy link
Contributor

DanielFabian commented Apr 15, 2019

What's the status on this? Is there anything I could do to help getting this merged?

@evanjs

This comment has been minimized.

Copy link
Member

evanjs commented May 19, 2019

Also happy to help keep this one going.
As far as I can tell, the only hard blockers look like a merge conflict and moving the expressions in top-level/python-packages?

@evanjs

This comment has been minimized.

Copy link
Member

evanjs commented Jun 14, 2019

Fixed (per @FRidh) and rebased but I can't push 😕
ERROR: Permission to LumiGuide/nixpkgs.git denied to evanjs.
Unless I'm doing something wrong

@cawilliamson

This comment has been minimized.

Copy link
Contributor

cawilliamson commented Jun 24, 2019

@evanjs You wouldn't be able to push to LumiGuide's repo - my advice if @roelvandijk has gone radio silent on this (which considering the amount of time this PR has been active wouldn't surprise me) would be to create another PR with your changes and mention this PR number (#47009) in the new PR.

If you don't want to do that though I'm happy to take over this one since I just got a new Razer laptop myself and really need this merged ASAP.

@evanjs evanjs mentioned this pull request Jul 10, 2019
5 of 10 tasks complete
evanjs added a commit to evanjs/nixpkgs that referenced this pull request Jul 19, 2019
- Refactor to de-duplicate common code per NixOS#47009 (comment)
- Remove rejected patch
- Use wrapGAppsHook in place of manual GI wrapping steps
evanjs added a commit to evanjs/nixpkgs that referenced this pull request Aug 28, 2019
- Refactor to de-duplicate common code per NixOS#47009 (comment)
- Remove rejected patch
- Use wrapGAppsHook in place of manual GI wrapping steps
evanjs added a commit to evanjs/nixpkgs that referenced this pull request Aug 29, 2019
- Refactor to de-duplicate common code per NixOS#47009 (comment)
- Remove rejected patch
- Use wrapGAppsHook in place of manual GI wrapping steps
@evanjs

This comment has been minimized.

Copy link
Member

evanjs commented Sep 3, 2019

Resolved with #64552

@jtojnar jtojnar closed this Sep 3, 2019
@jtojnar

This comment has been minimized.

Copy link
Contributor

jtojnar commented Sep 3, 2019

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

10 participants
You can’t perform that action at this time.