search

LVMakerHub / LINX

LabVIEW Community Edition LINX
Other
109 stars 114 forks source link

Fix Rpi GPIO issues in PI OS 6.6.31+rpt-rpi-v8. #132

Closed albsbc closed 1 month ago

albsbc commented 1 month ago

What does this Pull Request accomplish?

Fix Rpi 5 GPIO issues in PI OS 6.6.31+rpt-rpi-v8.

Why should this Pull Request be merged?

The changes are required to allow the LINX toolkit to be used on a RPI 5 running PI OS 6.6.31+rpt-rpi-v8.

What testing has been done?

Tested from LabVIEW. The GPIO pin states were monitored using a Maker HAT Base. The GPIO state was also tested using the PI OS pinctrl command e.g. pinctrl 4 4: no pu | -- // GPIO4 = none

sharpk commented 1 month ago

This is looking good in general, but I've found that every RPi5 image that the RPi folks have released lately has changed the GPIO base value, so I think the gpio base needs to be dynamically determined.

Related to that, if the GPIO base is set dynamically, can the RaspberryPi2B.cpp and RaspberryPi5.cpp be merged back together? If so, this would have several advantages, including allowing me to again create the images with LV pre-installed (I haven't created the 24Q1 images yet because of this issue).

sharpk commented 1 month ago

I should mention there are some deeper concerns with the entire GPIO implementation, because the sysfs interface we've been using is deprecated and the way forward is not entirely clear. So I'm afraid whatever we do here may be temporary. That being said, I would accept the pull request if it gets GPIO working again.

This post provides some detail about the current state of affairs on RPi: https://raspberrypi.stackexchange.com/questions/147465/current-proper-way-to-interface-gpio-from-c-code

albsbc commented 1 month ago

Hi @sharpk Thanks for all your comments. I agree that the GPIO base should be determined at runtime. I will write a routine that calculates the base. I think it should be possible to go back to having just the RaspberryPi2B.cpp file.

Would it be possible for you to build the existing pull request so that users can test it? I've built and tested it on my system, but it may be better if NI supply the shared object file.

In the long term libgpiod is the best solution. Is it already in the header/lib packages for compiling in the chroot?

sharpk commented 1 month ago

Thanks for your work on this, I think if we get the dynamic base calculation in place I think we'll be ready to go with this PR.

As far as creating a version of the library to test, we don't have a good way to create a test version of the installer and distribute it to users. Plus it takes quite a bit of effort to generate those installers and I don't have the time at the moment to do it. If you want to get this out to certain users to test I'd suggest just providing the .so on the forum with instructions on how to use it. Otherwise I'll commit to testing it out on my RPi5 and RPi2 boards prior to pulling it.

Part of the difficulty with libgpiod is that it is not present in chroot, so that's an additional barrier to re-writing liblinx to use it. I'd like to get libgpiod pulled into the chroot in the next release just to make it easier to start developing against it.

albsbc commented 1 month ago

Hi, I've modified LinxRaspberryPi5.cpp to read the GPIO base at runtime. Please can you review the changes? The library file should also work on earlier Rpi versions.

albsbc commented 1 month ago

This looks good to me, besides a couple of suggestions below. I can work on testing this on my side on an RPi5 and RPi2 with recent Raspbian images. Also, I can take on merging together RaspberryPi2B.cpp and RaspberryPi5.cpp.

Thanks for.ypur comments Ken and for merging the cpp files. I wanted to ensure that gpiobase defaulted to 0 on older versions of PI OS. I agree that the -1 could be returned if the fopen's fail.

sharpk commented 1 month ago

@albsbc I pulled your changes, built them, and testing them on a RPi5 and RPi2. They worked just fine. It's also notable that I ran the RPi5 version of the library on the RPi2 board, which proves that we can merge the RPi2 and RPi5 code together.

So if you can make the changes we discussed I can pull your branch and get started building the installer.

One technical detail: Please also add a "Signed-off-by" tag to your commit descriptions. See this link for details: https://github.com/LVMakerHub/LINX/blob/main/CONTRIBUTING.md