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.

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

ultraleap-hand-tracking: init #310138

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

ultraleap-hand-tracking: init #310138

wants to merge 6 commits into from

Conversation

Scrumplex
Copy link
Member

@Scrumplex Scrumplex commented May 8, 2024 •
edited
Loading

Description of changes

Closes #292624

This packages the UltraLeap hand tracking SDK for Linux.

See https://docs.ultraleap.com/linux/

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.05 Release Notes (or backporting 23.05 and 23.11 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.

Pandapip1
Pandapip1 reviewed May 8, 2024
View reviewed changes
Copy link
Contributor

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

The PR title should include the version of the package (3.4.1)

@Scrumplex
Copy link
Member Author

The PR title should include the version of the package (3.4.1)

As there are multiple packages here with different versions each, I don't think that'll be needed here

@Pandapip1
Copy link
Contributor

As there are multiple packages here with different versions each, I don't think that'll be needed here

Whoops, I thought the versions of all the ultraleap packages were synchronized. You're right; that isn't the case. Disregard that comment :)

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label May 9, 2024
@ofborg ofborg bot requested a review from Pandapip1 May 9, 2024 22:18
@ofborg ofborg bot added 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: 1-10 labels May 9, 2024
@Scrumplex Scrumplex force-pushed the pkgs/ultraleap branch 3 times, most recently from 7a61334 to ef367de Compare May 9, 2024 22:22
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 9, 2024
@Pandapip1
Copy link
Contributor

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

1 package blacklisted:
  • nixos-install-tools
4 packages built:
  • openxr-ultraleap-layer
  • ultraleap-hand-tracking-control-panel
  • ultraleap-hand-tracking-service
  • ultraleap-hand-tracking-service.dev

@ofborg ofborg bot requested a review from Pandapip1 May 9, 2024 23:43
Pandapip1
Pandapip1 suggested changes May 10, 2024
View reviewed changes
Copy link
Contributor

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

Please add release notes entries for the new module and for the update to the monado module.

@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels May 10, 2024
@Scrumplex Scrumplex marked this pull request as ready for review May 10, 2024 15:01
@ofborg ofborg bot requested a review from Pandapip1 June 30, 2024 19:50
Pandapip1
Pandapip1 suggested changes Jul 1, 2024
View reviewed changes
pkgs/by-name/op/openxr-ultraleap-layer/package.nix
, systemdLibs
, ultraleap-hand-tracking-service
}:
stdenv.mkDerivation (finalAttrs: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Might make sense to use stdenvNoCC here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are already pulling in stdenv.cc.cc in buildInputs, so we wouldn't really gain anything from using stdenvNoCC here.

pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-service/package.nix Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-service/package.nix
installPhase = ''
runHook preInstall

install -d $out/bin
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason you're using install -d + cp instead of either mkdir -p + cp or a single install command?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really any reason. I personally like using install where convenient, but as we are copying a lot of files around, cp -Tr was a better choice here IMO.

pkgs/by-name/ul/ultraleap-hand-tracking-service/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-service/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Outdated Show resolved Hide resolved
Pandapip1
Pandapip1 reviewed Jul 1, 2024
View reviewed changes
Copy link
Contributor

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

Nit: strictDeps = true is also probably a good habit.

pkgs/by-name/op/openxr-ultraleap-layer/package.nix Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-service/package.nix Show resolved Hide resolved
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2024
@Pandapip1 Pandapip1 mentioned this pull request Sep 4, 2024
Open
13 tasks
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 7, 2024
@Scrumplex
Copy link
Member Author

During NixCon 2024 I was able to try this module with @MatthewCroughan's Ultraleap Motion Camera and while the control panel recognized the device, it was unable to actually open the camera stream.

When keeping the service stopped, I was able to get a single frame of its camera using guvcview, before it errored out.

When the service is running it replaces the uvcvideo driver of the camera with usbfs. I am not sure if this is intended behavior.

Perhaps it is and this is just a permission issue? I sadly didn't have enough time to debug this.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 27, 2024
@FliegendeWurst FliegendeWurst added the awaiting_changes (old Marvin label, do not use) label Nov 8, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 9, 2024
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 11, 2024
@ofborg ofborg bot requested a review from Pandapip1 November 11, 2024 02:42
@Pandapip1
Copy link
Contributor

Pandapip1 commented Dec 4, 2024 •
edited
Loading

Given the nontrivial contents of the ultraleap license, it probably deserves a nixpkgs config option to indicate acceptance. See #341095 for an example of how to do that.

Documents that might need to be tracked:

getchoo
getchoo requested changes Dec 8, 2024
View reviewed changes
nixos/doc/manual/release-notes/rl-2411.section.md Outdated Show resolved Hide resolved
nixos/modules/services/hardware/monado.nix Outdated Show resolved Hide resolved
pkgs/by-name/op/openxr-ultraleap-layer/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-control-panel/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ul/ultraleap-hand-tracking-service/package.nix Outdated Show resolved Hide resolved
@getchoo getchoo added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 8, 2024
@SigmaSquadron SigmaSquadron removed the awaiting_changes (old Marvin label, do not use) label Jan 5, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 5, 2025
@Scrumplex
ultraleap-hand-tracking-control-panel: init at 3.4.1
1a720b3 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
ultraleap-hand-tracking-service: init at 5.17.1.0
450780c 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
openxr-ultraleap-layer: init
cffcb2d 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
monado: add Ultraleap support
58b365d 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/ultraleap: init
1e01152 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/monado: add enableUltraleap option
7a7d128 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@getchoo getchoo getchoo requested changes

@prusnak prusnak Awaiting requested review from prusnak

@expipiplus1 expipiplus1 Awaiting requested review from expipiplus1

@Pandapip1 Pandapip1 Awaiting requested review from Pandapip1

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
6.topic: freedesktop 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 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: 1-10 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.

Package request: ultraleap-hand-tracking
8 participants
@Scrumplex @Pandapip1 @technobaboo @getchoo @wegank @FliegendeWurst @h7x4 @SigmaSquadron