IntelRealSense / librealsense

Intel® RealSense™ SDK
https://www.intelrealsense.com/
Apache License 2.0
7.53k stars 4.81k forks source link

__init__.py not getting installed by 'make install' #10199

Open jpswensen opened 2 years ago

jpswensen commented 2 years ago

Required Info
Camera Model D400 (doesn't really matter, but the one we are using)
Firmware Version whatever is matched with SDK 2.49.0
Operating System & Version RPiOS Buster and Bullseye
Kernel Version (Linux Only) 5.10.92
Platform Raspberry Pi 4
SDK Version 2.49.0
Language python
Segment sensing

Issue Description

I have seen a lot of tutorial about installing librealsense+python wrappers on the rpi and they all have a step after the 'make install' where you have to copy the python wrapper init.py from the librealsense/wrappers/python/pyrealsense2/ folder into the /usr/lib/python3/dist-packages/pyrealsense2/ to get the import to work properly. I have done this multiple times with great success and it works fine.

However, I am in the process of using CPACK to create a .DEB file for our deployment purposes, so that we can simplify the process of getting a new machine up and running. This missing file is preventing the generated .DEB file from working properly. Everything else works wonderfully.

I am a CMake amateur, and despite trying to dig through the dozens of CMake related files, I honestly can't figure out why it isn't installing or the "right" way to make it happen. I think I could cobble together an "install(FILES ...)" statement in one of the CMakeLists.txt files for the python wrapper to hard code the installation of this file to the python installation folder, but this seems like me just flailing/hacking something that works for me, but might not be useful to other users/.platforms.

Can anyone point me in the right direction to figure out why this isn't getting copied on the rpi platform (I don't know if this is a problem on other platforms also).

MartyG-RealSense commented 2 years ago

Hi @jpswensen The subject of creating .deb files was discussed in the following cases:

https://github.com/IntelRealSense/librealsense/issues/4431 https://github.com/IntelRealSense/librealsense/issues/4846 https://github.com/IntelRealSense/librealsense/issues/5618

Intel created an official librealsense feature request to look at the possibility of providing .deb packages for Arm and my understanding is that it is now part of currently ongoing work on the SDK's LRS networking code. I do not have a time schedule available for when that might become available though.

In the meantime you could perhaps consider creating a complete SD card image file of a working librealsense Pi installation that has the Python wrapper included and then install that image on other Pi boards.

https://www.tomshardware.com/how-to/back-up-raspberry-pi-as-disk-image

jpswensen commented 2 years ago

That is the way we have been doing it, and it kindof works for a development environment, but not great for deploying a product.

The problem I am describing goes beyind creating a DEB file and relates more to a build system problem on RPi. I was simply using the explanation of the DEB file we are trying to make as a motivation for why that file really needs to be copied as part of CMake’s install step.

One would normally expect that a package is completely installed when a ‘make install’ is executed. Instead, there seems to be a bug in the install process where the init.py file isn’t getting installed/copied to the right location (despite all the .so files being installed correctly).

I am more than happy to do the legwork to submit a change request, I just requesting some guidance on CMake to help me hunt it down. My first step has been trying to figure out what CMake file handles building the list of .so files that gets installed to site-packages and does that copying during install.

MartyG-RealSense commented 2 years ago

A RealSense user suggested an alternative method at https://github.com/IntelRealSense/librealsense/issues/8083#issuecomment-907239779 for installing librealsense + pyrealsense2 together on Pi with CMake that worked for them.

jpswensen commented 2 years ago

Again, I already have librealsense2 installed and working fine, following directions very similar to those. The problem with that method is that if you have 1000+ deployed devices in the field in and IOT scenario and need to upgrade all of them. Installing a new a sdcard is not a viable option. A remote login step to re-run a build script is not viable (from networking/firewall perspective and from computational load perspective and from a duplicated effort perspective ).

Even if we did cobble together the second approach, this still doesn’t solve the problem that the CMake system is broken for rpi and doesn’t copy all to correct files to their proper locations during the “make install” step.

I am really just trying to get some help to work towards fixing the build system bug on rpi. Like I said, I am willing to do the legwork, but would appreciate some insight from a CMake expert to point me in the right direction.

Edit: in response to the linked issue, I think they forgot a step of getting the init.py in the right place. I don’t think any of those steps solve that problem.

MartyG-RealSense commented 2 years ago

The available options may be limited in this case, as building the pyrealsense2 wrapper on Raspberry Pi can be a more awkward process than other platforms.

Have you tried building pyrealsense2 as a self-contained static library by adding -DBUILD_SHARED_LIBS=false to the CMake instruction?

jpswensen commented 2 years ago

I spent some time today working through this. I think this patch (https://github.com/jpswensen/librealsense/commit/98a57730cd43df2f39fe374958e055bddb48fac5) should both fix the problem of the missing __init__.py file, as well as be useful for generating a .DEB file on pretty much any linux platform. I did my testing on a blank install of Ubuntu 20.04 and will try it out on a blank install of RpiOS tomorrow.

This is a list of the changes made:

  1. Modified the CMakeLists.txt in the python wrapper folder to copy the __init__.py file as part of the installation
  2. Added a packing.cmake file in the toplevel CMake folder to give variables to aid in the CPack .deb generation
  3. Modified the toplevel CMakeLists.txt to include the packing.cmake file
  4. Modified the install_config.cmake file to automatically do the udev/rules.d modifications if it is a Linux system and that folder exists
  5. Created a script called postinst (AFAICT it has to be named exactly that) so that the .deb file will reload the udev rules at the end of the .deb installation

The basic steps to build a function DEB file are as follows:

  1. Apply this patch (I am working against the 2.50 HEAD, but I think it should apply to most recent tagged versions)
  2. Created a subfolder called build. Change into that directory.
  3. cmake .. -DBUILD_PYTHON_BINDINGS=bool:true -DPYTHON_EXECUTABLE=$(which python3)
  4. export MAKEFLAGS=-j6 (feel free to change this to the number of your physical CPUs for faster compilation)
  5. cmake --build .
  6. sudo cpack -G DEB

The .deb file will be located in a folder librealsense/_packages with the name librealsense2_2.50.0_amd64.deb, of course with the major.minor.patch and architecture based on whatever version you used and whatever platform it was built on.

The one further outstanding issue I would like to get working is to modify the packing.cmake file to get dependencies right so that a user could simply install the .deb and it would check all the necessary dependencies correctly. Right now, it kindof assumes that the user had done that and I haven't gone through the process of figuring out exactly which ones were necessary.

MartyG-RealSense commented 2 years ago

Thanks so much for your commit!

In regard to checking dependencies of a .deb with CMake, multiple sources suggest using dpkg-shlibdeps

MartyG-RealSense commented 2 years ago

Hi @jpswensen Do you require further assistance with this case, please? Thanks!

jpswensen commented 2 years ago

I think it is resolved, but wasn't sure what the procedure is to get this pulled into the main repository.

Who makes those decisions? Do they want it in some other format that just a commit on my forked branch? It is worth making a pull request? Do you close issues before a pull request is either accepted/rejected?

I am just looking for guidance as to whether this is something that the development team is willing to incorporate so that I don't have to apply my patch to the repository (and maybe rebase) every time we decide to upgrade firmware and/or library version.

MartyG-RealSense commented 2 years ago

A pull request for including a project in the main librealsense SDK can be submitted at the link below.

https://github.com/IntelRealSense/librealsense/pulls

Once the pull request is submitted and it is passing automated build tests, a reviewer from the RealSense developer team can be assigned to the pull by Intel to review it and consider whether to merge it into the main SDK. It can sometimes take a long period of time before a submission is reviewed though.

The librealsense pull contribution guidelines are here:

https://github.com/IntelRealSense/librealsense/blob/c94410a420b74e5fb6a414bd12215c05ddd82b69/CONTRIBUTING.md

When I personally are handling an issue on this GitHub support forum that is associated with a pull, I put an Enhancement label on the issue to remind me not to close the issue until the pull associated with it is closed or merged.

The RealSense team welcomes pull submissions for consideration for inclusion in the SDK. As mentioned above though, it could take a prolonged period of time for that pull to be reviewed and merged.

MartyG-RealSense commented 2 years ago

Hi @jpswensen Do you require further assistance with this case, please? Thanks!

jpswensen commented 2 years ago

I just haven’t had the time to get the pull request polished and submitted yet.

On Feb 12, 2022, at 12:40 AM, MartyG-RealSense @.***> wrote:

Hi @jpswensen https://github.com/jpswensen Do you require further assistance with this case, please? Thanks!

— Reply to this email directly, view it on GitHub https://github.com/IntelRealSense/librealsense/issues/10199#issuecomment-1037067821, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFQKJB4ESDTEWDYVVMV34LU2YMGHANCNFSM5ND6LZCA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.

MartyG-RealSense commented 2 years ago

Okay, thanks very much @jpswensen for the update. I will keep this case open for a further time period. Good luck!

MartyG-RealSense commented 2 years ago

Hi @jpswensen If you are busy, do you want to close this case until you are ready to create a PR and then re-open it at that time, please? Thanks!

MartyG-RealSense commented 2 years ago

Hi @jpswensen I will close this issue for the time being. If you wish to re-open it at a future date when you are able to create a pull request then you are welcome to do so. Thanks!

jpswensen commented 2 years ago

@MartyG-RealSense , I ended up getting that original patch split into two other patches and submitted as pull requests. I also made a third pull request that gets the kernel patches working for x86_64 and raspberry pi 4.

Is it common practice to make an issue for each of the pull request, and was just wondering what the timeframe is for someone to take a look at those and evaluate for inclusion?

MartyG-RealSense commented 2 years ago

Hi @jpswensen Thanks so much for creating your pulls! I am not involved in pull request processing so cannot provide a time estimate for when it may be reviewed. It is likely to be later rather than sooner though. In the meantime, pull requests are a great reference for showing other RealSense users which edits to make to achieve the purpose of the pull, so they are definitely worth creating. Thanks again. :)

I have added an Enhancement label to this case to signify that it should be kept open whilst your pull requests are active. Could you post your pull request links in a comment on this case, please?

jpswensen commented 2 years ago

Just for documentation purposes, the patch I linked above from my own repository has been split into two pull requests for the two disparate pieces.

Pull request #10487 encompasses the problem of not installing the package init.py file correctly Pull request #10488 encompasses the addition to allow for the easy creation of .deb files

There is a third pull request (maybe I should make a different issue) that I added in #10489 which encompasses distilling, adding copious comments, and simplifying the kernel patches for Ubuntu 22.04 LTS for the default kernels on both x86_64 and raspberrypi4.