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

bazecor: rule for Dygma Defy serial dev #349215

Closed
wants to merge 1 commit into from

Conversation

blackxored
Copy link

Bazecor fails to launch with a Dygma Defy connected due to permission errors on a serial device, e.g /dev/ttyACM0, this is owned by the dialout group but rather than have my user be part of that group which feels like a workaround, there's a rule added for the tty subsystem which wasn't covered by the previous rules that are adapted from upstream.

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:
  • 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.11 Release Notes (or backporting 23.11 and 24.05 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.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Oct 17, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Oct 17, 2024
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

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

Thanks! I only own a Raise, so I can't test this; but I think @gcleroux has one.

Also, is this something that could be upstreamed to bazecor?

@amesgen
Copy link
Member

amesgen commented Oct 17, 2024

This also seems related: #269513 (comment) cc @gabyx

@blackxored
Copy link
Author

Thanks, it could be upstreamed, I'm not sure it applies to other distros though, I need it working :D
The linked issue there seems definitely related. This is the only device /dev/ttyACM0 that seems accessed by the app and is owned by root:dialout with no user access without this rule.
Note that I have tested opening, configuration and saving settings, I have not tested flashing since there hasn't been any firmware update.

@gcleroux
Copy link
Contributor

@blackxored I do own a Defy, but I'm not sure if this udev rule is actually needed.
My user is not part of the dialout group and I can access the Defy.

I didn't dig too deep into this, but to me it seems like a configuration issue.
I know systemd-logind handles a lot of device permissions dynamically.

It also looks like logind is managing ttyACM0

❯ loginctl seat-status 
seat0
Sessions: *4
 Devices: n/a
         ...
         ├─/sys/devices/pci0000:00/0000:00:07.1/0000:2b:00.0/0000:2c:00.0/0000:2d:00.0/usb5
         │ usb:usb5
         ...
         │ └─/sys/devices/pci0000:00/0000:00:07.1/0000:2b:00.0/0000:2c:00.0/0000:2d:00.0/usb5/5-3
         │   usb:5-3
         │   ├─/sys/devices/pci0000:00/0000:00:07.1/0000:2b:00.0/0000:2c:00.0/0000:2d:00.0/usb5/5-3/5-3:1.0
         │   │ usb:5-3:1.0
         │   │ └─/sys/devices/pci0000:00/0000:00:07.1/0000:2b:00.0/0000:2c:00.0/0000:2d:00.0/usb5/5-3/5-3:1.0/tty/ttyACM0
         │   │   tty:ttyACM0

Are you perhaps not using systemd and that's why you need to add this rule?

@blackxored
Copy link
Author

I am using systemd.

Without the rule I get this from Bazecor:

20:19:27.573 › receivedDevice:  /dev/ttyACM0
20:19:27.574 › error when opening port:  Error: Error: Permission denied, cannot open /dev/ttyACM0
20:19:27.593 › SerialPort devices find: [
  {
    path: '/dev/ttyACM0',
    manufacturer: 'DYGMA',
    serialNumber: '22D51FB86486F1D2',
    pnpId: 'usb-DYGMA_DEFY_22D51FB86486F1D2-if00',
    vendorId: '35ef',
    productId: '0012'
  },
  { path: '/dev/ttyS0' },
  { path: '/dev/ttyS1' },
  { path: '/dev/ttyS2' },
  { path: '/dev/ttyS3' }
]
...

I do not see ttyACM0 or any tty's when running loginctl seat-status.

There's also no ACL on the file at all:

❯ ll -g /dev/ttyACM0
crw-rw---- 166,0 root dialout 2024-10-17 20:18  /dev/ttyACM0

@gabyx
Copy link
Contributor

gabyx commented Nov 17, 2024

I can try to test this without my user in the dialout group (what i use now), maybe later this week.

I use systemd. What does the udev rule do? Sorry not that great expr. in linux details.

@blackxored
Copy link
Author

It gives the current user rw permissions on /dev/ttyACM0. I'm no expert but without it, the file had no ACL and I couldn't open Bazecor or have it detect the device.
You could run getfacl /dev/ttyACM0 and see what it looks like without this rule, with the rule mine looks like this:

# file: dev/ttyACM0
# owner: root
# group: dialout
user::rw-
user:xored:rw-
group::rw-
mask::rw-
other::---

and here's the file:
crw-rw----@ 166,0 root dialout 15 Nov 09:31  /dev/ttyACM

@gabyx
Copy link
Contributor

gabyx commented Nov 18, 2024

  • I can confirm that removing myself from dialout group without this patch will not allow me to access the /dev/ttyACM0.

  • I also use systemd and do not see any tty* devices managed with loginctl seat-status.

  • Applying this patch (just adding your Nixos as a flake input and using your bazcore) did not work (getfacl /dev/ttyACM0 returns the same stuff?) . I guess I am doing something wrong...
    Anybody knows when the $out/lib/udev/... files are applied? Are these files assembled by NixOS globally somehow, or who interacts with the udev rules in the /nix/store paths?

@blackxored
Copy link
Author

Ok, Bazecor i just weird, I removed the overlay I have with this patch and it seems to be working, I have no idea why.
@gabyx Is bazecor under services.udev.packages? If you add it and it works without this patch we can probably close, if it doesn't (like it didn't for me back then), then can you try again making sure the right 60-dygma-rules is in your store?

@gabyx
Copy link
Contributor

gabyx commented Nov 21, 2024

Ah I just seen that I did not even had bazecor in services.udev.packages hm...
althoug with dialout it worked.
let me check again.

@gabyx
Copy link
Contributor

gabyx commented Nov 21, 2024

@blackxored I tried that and it works, which is cool, so no need to add myself to dialout. Without it it does not work. So I guess these changes are good to go!

getfacl: Removing leading '/' from absolute path names
# file: dev/ttyACM0
# owner: root
# group: dialout
user::rw-
user:nixos:rw-
group::rw-
mask::rw-
other::---

It would maybe be good to add something to the description for the user of this package, what needs to be done?
For example adding it to services.udev.packages etc.

@blackxored
Copy link
Author

For point #2 package should probably provide an option programs.bazecor.enable or similar that adds the package and the udev rules.
I'm just confused as to why is working without my overlay now.

@gabyx
Copy link
Contributor

gabyx commented Nov 25, 2024

@blackxored : You are right, it works without your patch, I get the same getfacl output. lol
So I guess this patch is not even needed.

@blackxored
Copy link
Author

Interesting, well if that works let's feel free to close. Would also like my suggestion of having a programs.bazecor.enable option to make all this easier be implemented at some point.

@amesgen amesgen mentioned this pull request Nov 25, 2024
13 tasks
@amesgen
Copy link
Member

amesgen commented Nov 25, 2024

Would also like my suggestion of having a programs.bazecor.enable option to make all this easier be implemented at some point.

I agree, I opened #359143 for that 👍

@blackxored
Copy link
Author

Perfect! Closing this one then. Thank you all for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants