-
-
Notifications
You must be signed in to change notification settings - Fork 83
chore: bump libs & fix libcamera, fix upstream nixpkgs #23
Conversation
Thanks for the fast update (and providing binary-cache to test) - the changes itself look good to me but I encountered issues compared to building against master. When booting the pi networking is somehow broken - I haven't debugged further - just took that photo when plugging a monitor for now: Normally the pi is configured to join by wifi after boot which works when building against master with 24.05 ✌🏻 |
I noticed some wifi issues also. could be that the latest wifi firmware is unstable. I switched to 5ghz and the issue still persists but I see no warnings. something's not right with the firmware. |
I see, I would suggest (not a maintainer of the project) to maybe split up the PR with the "safe" / "stable" and the wireless update in separate then maybe? |
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.
Thanks for this PR! I put this stuff on github in the first place because I want to collaborate on this work with others, so I welcome this PR.
That being said, I think there are a number of uncontroversial changes in here, but they are mixed in with other unrelated changes that require more discussion.
It would be easier to consider PRs that have a more narrow focus. So, I would recommend closing this PR and opening a number of PRs, each with a specific focus (e.g. dependency bumps, adding defconfig to kernel, etc).
overlays/default.nix
Outdated
kernelPatches = []; | ||
defconfig = "${board}_defconfig"; | ||
postFixup = ""; |
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.
Adding these options sounds good to me. However, most of the other changes to this file conflict with its current design I think.
For example, master supports having multiple kernel versions under pkgs.rpi-kernels
, as each key in this attrset is a kernel version, and the helper pkgs.rpi-kernels.latest
exists to point to the latest. The rpi-kernel
binding in this let is to help build any kernel version. The changes in this branch specialize it to the provided version, replace <VERSION>
with latest_<BOARD>
, and drop the latest
setting. None of that seems like an improvement to me.
It would probably make more sense to replace pkgs.rpi-kernels.<VERSION>.kernel
with an attrset that provides the kernels as configured per board. For example: pkgs.rpi-kernels.<VERSION>.kernels.<BOARD>
.
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.
Also maybe untangle the boot loader and wireless firmware from it. Because I don't think those are board or kernel dependent. I'll see what I can do.
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.
updated this to pkgs.rpi-kernels.<VERSION>.<BOARD>
, and untangled it from the firmware. What do you think of the new code @tstat ?
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'm on mobile right now, but it looks great! I'll give it a closer look when I'm at a computer later.
, ninja | ||
}: | ||
|
||
{ libcamera-apps-src, lib, pkgs, stdenv }: |
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.
Why diverge from the well-established callPackage
pattern and instead pass in pkgs
?
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.
well, those are coming from pkgs, Its mentally easier to wrap your head around the fact that callPackage just provides pkgs rather than it also does a spread, which confuses me, because we're not passing all these extra params.
overlays/libcamera.nix
Outdated
buildInputs = old.buildInputs ++ (with final; [ | ||
libpisp openssl libtiff | ||
(python3.withPackages (ps: with ps; [ | ||
python3-gnutls pybind11 pyyaml ply | ||
])) | ||
libglibutil gst_all_1.gst-plugins-base | ||
|
||
]); | ||
patches = [ ]; | ||
mesonFlags = [ | ||
"--buildtype=release" | ||
"-Dpipelines=rpi/vc4,rpi/pisp" | ||
"-Dipas=rpi/vc4,rpi/pisp" | ||
"-Dv4l2=true" | ||
"-Dgstreamer=enabled" | ||
"-Dtest=false" | ||
"-Dlc-compliance=disabled" | ||
"-Dcam=disabled" | ||
"-Dqcam=disabled" | ||
"-Ddocumentation=enabled" | ||
"-Dpycamera=enabled" | ||
]; | ||
|
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.
Can you describe/motivate these changes?
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.
libcamera version 2 now requires these flags to build. it was in their readme.
rpi/default.nix
Outdated
# Some patches cannot be applied because they are already upstream. | ||
ignoreConfigErrors = true; |
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.
Can you motivate this?
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.
Latest kernel has patches that could not be overwritten, so rather than not using nixpkgs rpi-kernels nix configuration, I just said, ignore failed patch applies. If you have another suggestion, please let me know.
It wasn't fully applied and it isn't necessary for these overrides anyway
I went to build an image with this branch yesterday, but there were some syntax errors in the kernel derivation. I pushed some fixes and will test an image later today. |
So a silly mistake happened. My lock file locked a version of the local copy I had, I thought I was making testable changes but it was just running an older version of the flake :D Thanks for your changes, much appreciated! |
I have tested it with 6.6.34 and it works fine, with the exception of the wifi, there seems to be an issue with it being terribly slow/choppy (but no patches required). Compiling the 6.6.31 also went smoothly! No more ignore errors! |
Thanks for the contribution! |
things tested and working:
things to try out:
[ ] uboot booting, see if its fixed.Uboot still not workingThis should fix #19
Untested but maybe also #16
Serial error