Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fix for GPIO under recent Linux kernel versions #1562

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open

Bug fix for GPIO under recent Linux kernel versions #1562

wants to merge 3 commits into from

Conversation

romaricr
Copy link

Slight update of MySensors using libgpiod instead of all standard sysfs to map interrupts.

@romaricr
Copy link
Author

Tested on my Raspberry PI / NRF24 gateway and working fine (with around 10 devices). Not tested on any other platform.

@mfalkvidd
Copy link
Member

Nice work, thanks!

Attaching the restyling that the butler requests:
restyling.patch

Will add further comments shortly.

mfalkvidd
mfalkvidd reviewed Mar 16, 2024
View reviewed changes
Copy link
Member

@mfalkvidd mfalkvidd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding review comments.

Do you know how long this interface has been available? The reason I'm asking is that we would prefer to not break older gateways (running on older Linux versions) if people upgrade.

MyConfig.h Outdated
@@ -442,7 +442,7 @@
* - RF24_PA_MAX = 0dBm
*/
#ifndef MY_RF24_PA_LEVEL
#define MY_RF24_PA_LEVEL (RF24_PA_HIGH)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we didn't change the default PA level in this PR, since it is not related to the change.

hal/architecture/Linux/drivers/core/interrupt.cpp Outdated
Comment on lines 224 to 230
// FILE *fp = fopen("/sys/class/gpio/unexport", "w");
// if (fp == NULL) {
// logError("Unable to unexport pin %d for interrupt\n", gpioPin);
// exit(1);
// }
// fprintf(fp, "%d", gpioPin);
// fclose(fp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind deleting those lines instead of commenting them? We have git for history.

hal/architecture/Linux/drivers/core/interrupt.h
@@ -24,6 +24,9 @@

#include <stdint.h>

// Ajout RRO
#include <gpiod.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if this is installed by default on Raspberry Pi OS? If it is not, we should probably document how to install the dependency.

@romaricr
Copy link
Author

romaricr commented Mar 16, 2024 •
edited
Loading

Styles and minor updates done.
For libgpiod, it has been there for a while. The release notes says 6 years.
About the potential dependency, you are right. It might require a sudo apt-get install libgpiod-dev to build and sudo apt-get install libgpiod to run. I have not tested what it looks like on a genuine installation.

For the documentation, sorry for my question but, how do I do that ?

@mfalkvidd
Copy link
Member

Excellent, thanks!

6 years should be good enough, imo.

The documentation is not under version control unfortunately but I can add it to https://www.mysensors.org/build/raspberry

@romaricr
Copy link
Author

Thank you to the reviewers.
I have seen some of the errors but I'm really not sure how to fix them ... Anyone could help ?

@mfalkvidd mfalkvidd mentioned this pull request Apr 19, 2024
Open
@ricgyver
Copy link

Hi,
I was trying the fixed version after upgrading my RPI to the latest Raspbian OS. However, even after using the MySensors-gpio_fix version of the library I get "Could not open /sys/class/gpio/gpio16/direction"
Can you please help me?
Thanks in advance!

@functionpointer
Copy link
Contributor

Just tested this on RPi Zero W with Raspbian Bookworm 32-bit kernel. Works great.

@raks-dev
Copy link

Hi, I was trying the fixed version after upgrading my RPI to the latest Raspbian OS. However, even after using the MySensors-gpio_fix version of the library I get "Could not open /sys/class/gpio/gpio16/direction" Can you please help me? Thanks in advance!

What is the SOC and CPU detected as part of the configure command execution?

E.g.

[SECTION] Detecting target machine.
  [OK] machine detected: SoC=BCM2712, Type=rpi5, CPU=aarch64.
[SECTION] Detecting SPI driver.
  [OK] SPI driver detected:BCM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfalkvidd mfalkvidd mfalkvidd left review comments

@d00616 d00616 Awaiting requested review from d00616

@tbowmo tbowmo Awaiting requested review from tbowmo

@henrikekblad henrikekblad Awaiting requested review from henrikekblad

@Yveaux Yveaux Awaiting requested review from Yveaux

@tekka007 tekka007 Awaiting requested review from tekka007

@scalz scalz Awaiting requested review from scalz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@romaricr @mfalkvidd @ricgyver @functionpointer @raks-dev