Skip to content

Conversation

@kshetline
Copy link
Contributor

@kshetline kshetline commented Feb 14, 2025

I've removed the changes I'd had made for life on a different fork, as I suspect I'd need to do, to make this PR easier to accept.

@kshetline kshetline changed the title Raspberry Pi Compatibility - The Sequel Raspberry Pi 5 Compatibility - The Sequel Feb 14, 2025
@momenso momenso self-assigned this Feb 15, 2025
@momenso
Copy link
Owner

momenso commented Feb 15, 2025

Thank you so much for this wonderful contribution! The addition of libgpiod support opens up crucial compatibility with Raspberry Pi 5, and your abstraction layer between BCM2835 and libgpiod is clean and effective. I’ve tested these changes on real hardware with no issues found, so we’re good to offer them right away.

In the future, we may refine the documentation to clarify libgpiod installation steps and streamline examples. While the new live-test.js helps demonstrate continuous reads, it duplicates some functionality already available in async-explicit.js. We might remove live-test.js at a later date to avoid confusion and dependency on hard-coded sensor/pin values. Overall, fantastic work—thank you!

@kshetline
Copy link
Contributor Author

Thank you for your kind words. I'm happy to help. As a now-retired software engineer, I need projects! 😄

One change I'd suggest that I didn't know how to do myself is a better test for whether or not to use libgpiod or not, better than looking at /proc/cpuinfo an checking for Raspberry Pi 5.

I had been hoping that bcm2835_init() would fail if the BCM2835 library wasn't going to work. Unfortunately the initialization succeeds even when the library won't work after that. If you know a better way to test BCM2835 functionality that would be a good change.

As for live-test.js, no biggie if you remove that. It was just a easy way for me to do my own testing. If you like I can, in a separate PR later, submit a more robust version that takes command line arguments for sensor type and pin.

@momenso
Copy link
Owner

momenso commented Feb 16, 2025

We really need to remove the live-test.js otherwise the tests will never finish and the CI pipeline won't complete. The live-test.js script runs indefinitely and prevents our CI pipeline from finishing successfully. The existing async-explicit.js example covers similar functionality without blocking the pipeline, so that can remain our go-to live test script for now.

To ensure our tests complete in an automated environment, we’ll remove live-test.js from the PR
@momenso momenso merged commit b336657 into momenso:master Feb 16, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants