uhd: fix python api feature #307435
Merged
uhd: fix python api feature #307435
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofborg
bot
added
11.by: package-maintainer
13 tasks
|
Commit message typo: cmakeBook -> cmakeBool |
doronbehar
force-pushed
the
pkg/uhd-python
branch
from
fdcefc5 to
0c60ffa
Compare
|
Thanks! |
|
@systemofapwne I noticed your ❤️ reaction, but I'm not sure if that means "I approve". Here's the layout of the build, in case you don't have time to build it on your machine. |
Sorry, I didn't find the time to check this on my machine yet, but I will give this a try by today. |
|
So I just tested your PR. It builds fine, yet it still fails on installing the python module. I tested everything using this flake here: {
description = "A very basic flake";
inputs = {
#nixpkgs.url = "github:systemofapwne/nixpkgs?ref=12f5fb3"; # my hacky PR
nixpkgs.url = "github:doronbehar/nixpkgs?ref=0c60ffa"; # More elegant PR
};
outputs = { self, nixpkgs }:
let
pkgs = import nixpkgs { system = "x86_64-linux"; };
ps = pkgs.python3.pkgs;
in {
uhd = (ps.toPythonModule (pkgs.uhd.override {
python3 = pkgs.python3;
enableUtils = true;
enableCApi = false;
enablePythonApi = true;
enableOctoClock = false;
enableMpmd = false;
enableB100 = false;
enableB200 = false;
enableUsrp1 = false;
enableUsrp2 = false;
enableX300 = true;
enableN300 = false;
enableN320 = false;
enableE300 = false;
enableE320 = false;
}));
defaultPackage.x86_64-linux = pkgs.buildEnv {
name = "sdr-env-2";
paths = [
(pkgs.python3.withPackages(ps: [
ps.numpy
self.uhd
]))
];
};
};
}I think the reason is, that upstreams Python VENV check still returns true and thus ignores UHD_PYTHON_DIR. My compile log has entries like this: Here is my complete compile log: compile.log |
Haha I haven't noticed these errors! Thanks for pointing them out. Certainly this is not ideal, but I'm not sure I understand if you are lacking any files in the I'm trying to think of a way to fix those errors in the meantime. |
doronbehar
force-pushed
the
pkg/uhd-python
branch
from
0c60ffa to
bf34c01
Compare
This comment was marked as outdated.
This comment was marked as outdated.
doronbehar
force-pushed
the
pkg/uhd-python
branch
from
bf34c01 to
4e3180d
Compare
|
I'm now building locally what I pushed - will update if it works (it'll take a few hours because my resources are limited at the moment). |
doronbehar
force-pushed
the
pkg/uhd-python
branch
from
4e3180d to
28621c4
Compare
doronbehar
force-pushed
the
pkg/uhd-python
branch
2 times, most recently
from
ddd94a0 to
ac0ee3c
Compare
|
OK I got it :) It wasn't trivial at all... I built it locally, and I also made sure the python utilities the build produces all work by using a correct python interpreter, besides Now I'm testing locally these packages: Your review @systemofapwne is welcome now again. |
|
OK Now all of these packages build, and produce the desired output (fixed a small mistake since last time). Will wait a day or two for comments to arrive, if not, will self merge. |
|
I was able to build the package with the flake I provided earlier. Importing the UHD python module now works properly. However I did not test for functionality yet, since I lack the hardware at home. I will plan to test this tomorrow, when I am back in the lab. Thank you for the great job! |
|
I just confirmed that your PR works pretty well with our SDRs using the python API. I fully approve your PR. |
|
Great! Thanks! |
|
Does it also make sense to back-port it to the respective channels, that support the current UHD 4.6? |
|
Hmm technically it is a new feature added to the package, so it's not a classic PR to backport to the stable channels. I think though it won't hurt anyone, especially if we are a few weeks before the branch off of the next release. What I'm not sure I understood, was what do you mean by:
|
I was referring to the nixos release channels, that are already on 4.6. release-23.11 seems to already be on 4.6. |
|
We could also backport the updates from UHD 4.4 -> 4.6 but that feels a bit risky, and especially if you are using a flake setup, and you already compile your uhd for your project, I think it won't matter too much for you :). Thanks for you patience. 🥂 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes