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
config.hardware.onlykey configuration option #33794
Conversation
nixos/modules/hardware/onlykey.nix
Outdated
config = mkIf config.hardware.onlykey.enable { | ||
services.udev.extraRules = | ||
'' | ||
# UDEV Rules for Teensy boards, http://www.pjrc.com/teensy/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole block appears to be copied from a file. Either:
- download the file to the containing directory. Use builtins.readFile.
- remove the superfluous comments about how to install the file. Not relevant.
nixos/modules/hardware/onlykey.nix
Outdated
# | ||
ATTRS{idVendor}=="16c0", ATTRS{idProduct}=="04[789B]?", ENV{ID_MM_DEVICE_IGNORE}="1" | ||
ATTRS{idVendor}=="16c0", ATTRS{idProduct}=="04[789A]?", ENV{MTP_NO_PROBE}="1" | ||
SUBSYSTEMS=="usb", ATTRS{idVendor}=="16c0", ATTRS{idProduct}=="04[789ABCD]?", MODE:="0666" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mode "0666" is questionable. That adds RW access to all users.
Typically udev rules add RW access to all users in the "plugdev" group. Which is users who are authorized to "plug in devices".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new style is to use TAG+="uaccess"
instead of using GROUP= and MODE=.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjornfor I just tested with TAG+="uaccess"
on my local system and the device didn't work, switch back to GROUP
made it work. Not yet sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at other packages it looks like that'd require creating a new udev file with an earlier precedence. Is this what you want me to do?
Who'd be the best person to talk to about this change? Thanks! |
@coreyoconnor thank you for the feedback, will address shortly. |
By default, OnlyKey device (https://crp.to/p/) won't work on Linux (and, therefore, NixOS). This is unintuitive and requires one to search for a solution in the documentation. This change allows one to enable OnlyKey device support directly from their NixOS configuration.
@coreyoconnor I've updated this PR to address your feeedback. Hopefully I've got it right now :) |
Anything else I should address in this patch? |
It really should be in the docs that users should be part of the "plugdev" group if we want them to be able to access various USB devices. I have to create a rules for ledger nano s. We should also standardise across the different USB devices to use the "plugdev" group as the main group. Currently it's just a magic string that you have to know about by going through the nixpkgs source. |
@yrashk Hi, Onlykey updated it's vendor and product id's, I'm not sure if it's set in hw or firmware, it would be nice to include new values and maybe preserve old ones. Does current NixOS config work for you at the moment? |
Indeed the onlykey now has a different vendor and product id, so the current udev rule doesn't work. https://docs.crp.to/linux.html By looking at the commit history of the official onlykey udev rule, I see that the version of this PR was kept from May 14 2018 to Nov 12 2018, so only for 6 months. Users can upgrade their onlykey by using the official app. I think they even get a notification, so the users who bought the onlykey from May 14 to Nov 12 2018 probably have already upgraded by now. |
By default, OnlyKey device (https://crp.to/p/) won't work on Linux (and,
therefore, NixOS). This is unintuitive and requires one to search for a
solution in the documentation.
This change allows one to enable OnlyKey device support directly from
their NixOS configuration.
Motivation for this change
Adding straightforward support for OnlyKey to NixOS.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)