IRQ support using Linux kernel Character Device + Posix threads #961

2bndy5 · 2024-03-18T00:36:18Z

resolves #943

I think its ready. Given the fact that nRF24/RF24Gateway#32 relies on these changes, I'm ready to move forward and patch things up as (or if) they occur.

Interrupt changes

💯 Use Linux kernel's character device API to implement attachInterrupt() and detachInterrupt() for both SPIDEV and RPi drivers. Combined with Posix threads, this gives a very quick and reliable behavior that one would expect from microcontroller platforms.
wraps wiringPi's wiringPiISR() into attachInterrupt() for better compatibility with interruptConfigure Linux example. No detachInterrupt() is provided as the wiringPi does not implement that functionality (to my knowledge).
changed IRQ_PIN to 24 in examples_linux/interruptConfigure.cpp along with a TX timeout and improved stdout display
modified build system (CMake and old static makefiles) to utilize the selected driver's interrupt implementation.
- MRAA, wiringPi, and pigpio all have their own implementation. Only MRAA's implementation is not wrapped into an Arduino API, thus it is still not compatible with examples_linux/interruptConfigure.cpp.
- pyRF24/setup.py no longer adds any special compiler args to disable compilation of interrupt code. Also, attachInterrupt() and detachInterrrupt() are not exposed in the python bindings, so python user are still left to find other opportunities to use the radio's IRQ pin in python (RPi.GPIO, pigpio, gpiod, etc).
resolves compiling gettingstarted for wiringPi fails #669 by adding wiringPi-specific linker args to the old examples_linux/Makefile.examples script. ~~This means we can also re-enable wiringPi driver in Linux CI using the old static makefiles.~~ Cross-compiling wiringPi requires cross-compiled dependencies (crypt and rt in this case); I re-disabled wiringPi in CI builds that are cross-compiled.

Changes that follow-up #959

GPIO chip verification and info fetching is now performed in GPIO::open (for SPIDEV driver). attachInterrupt() also does GPIO chip verification and info fetching for both RPi and SPIDEV drivers. (see Bump RF24 from 9eca153 to 05efe34 pyRF24#41 (comment))
RF24_SPIDEV_GPIO_CHIP macro definition is renamed to RF24_LINUX_GPIO_CHIP because it affects the GPIO/IRQ support for SPIDEV driver and the IRQ support for RPi driver.

Additional changes

In reviewing the Linux drivers for interrupt support, I also went through and

removed any unnecessary #include statements, presumably from unchecked copy-and-paste of utility/Template files.
improved the SPIException messages in SPIDEV driver to be a bit more helpful/descriptive.
allow wiringPi SPI wrapper to use /dev/spidev0.1 (previously hard-coded to /dev/spidev0.0)
add wiringPi's pin number for the CE_PIN to Linux examples

…it()

- Add timeout to interrupt example

remove unused `#include` and reorder so that std/external libs are pre-processed first

- switch RPi driver to char-dev IRQ - wrap wiringPiIsr() into attachInterrupt() - adjust build system generators (lib and examples) - use IRQException instead of GPIOException - make defined RF24_LINUX_GPIO_CHIP more agnostic of selected driver (applied to RPi and SPIDEV drivers) - fix compiling wiringPi with examples using old makefile - update URL in examples_linux/README - remove explicit wiringPi from list of linked libs in pyRF24/setup.py

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

outdated suggestion

I am not going to try cross-compiling dependencies of dependencies. WiringPi needs libs: crypt and rt compiled for the target system.

utility/pigpio/compatibility.cpp

utility/wiringPi/RF24_arch_config.h

TMRh20

Very nice work! Still needs some testing on my part, but we can hold off on doing a release for a little bit.

2bndy5 · 2024-03-18T12:48:01Z

The 2 drivers mainly effected by this are SPIDEV and RPi. We've tested SPIDEV rather well. I tested RPi with expected results. I don't expect RPi to have different IRQ behavior because the code is pretty much copied from SPIDEV's gpio and interrupt sources.

Most other changes should only affect the build/compilation process. But, wiringPi SPI wrapper should be re-tested.

utility/pigpio/interrupt.h

this should fix the issue with loading pyRF24 modules( downstream) when building docs. There should be negligible performance hit as most of this init stuff (`pinMode()` and `attachInterrupt()`) only takes place at user space startup. Although, devs that dynamically allocate pins (with `RF24::begin(CE, CSN)`) will see a performance cost.

2bndy5 · 2024-03-18T23:16:19Z

I just went through another round of hardware tests.

Caution

The interruptConfigure example doesn't work with wiringPi because wiringPi doesn't like using GPIO24 (wiringPi # 5) for the radio's IRQ pin.

gpio: Unable to open GPIO direction interface for pin 24: No such file or directory
wiringPiISR: unable to open /sys/class/gpio/gpio24/value: No such file or directory

On RPi3

All drivers still work on RPi3.

On RPi4

RPi, SPIDEV, wiringPi, and pigpio still work on RP4. MRAA doesn't work on RPi4, but I think that was known already.

2bndy5 · 2024-03-19T03:49:38Z

I think I'm finally ready to merge this. I reviewed the code 3 times since opening this PR. Hardware tested all drivers twice on both my RPi setups. This one was so ambitious, I'm pretty sure there's something I'm missing...

2bndy5 · 2024-03-19T03:53:04Z

My PC's clock was out-of-sync. 😆 This happens every time I reboot from ubuntu into windows. Now github thinks I pushed a commit in the future.

TMRh20 · 2024-03-19T09:52:13Z

Just FYI been testing on RPi5, 4, 3 and 2 for speed, stability & functionality and have found 0 problems.

2bndy5 · 2024-03-19T12:04:48Z

I want to add a bool to the GPIOChipCache struct that will prevent altering the used path after a working path has been detected. Otherwise, I think GPIO::open() could set it to /dev/gpiochip0 while /dev/gpiochip4 is being accessed by another process.

2bndy5 and others added 16 commits March 13, 2024 05:13

move chip info into GPIOChipCache struct

e434507

make gpioCache static in gpio.h

af70aa8

use pthread & poll with Linux kernel's char-dev API

32f7803

fix segfault; avoid using extern funcs

abe8d25

use pthread_cancel instead; fix ISR callback trigger

ed289b4

don't release pin upon exiting attachInterrupt()

678a144

remove unneccessary config flags for FALLING edge detetction

bbd5680

clear the edge event from kernel buffer

ec92ac5

[example] change IRQ_PIN number and flush cout stream before pin_n_wa…

995bca5

…it()

IRQ example timeout

004291e

- Add timeout to interrupt example

don't poll(); just read() and track event sequence number

870d3a6

[example] use endl instead of flush

03661b1

code cleanup (in all Linux driver files)

cf6d5f2

remove unused `#include` and reorder so that std/external libs are pre-processed first

increase event buffer size; only zero-out irqEventInfo once

bd48742

re-enable wiringPi driver in Linux CI using old makefile build

b950a42

2bndy5 added the enhancement label Mar 18, 2024

This comment was marked as outdated.

Sign in to view

Apply suggestions from clang-format code review

b74f17e

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

This comment was marked as resolved.

Sign in to view

rearrange pre-processor statements in a header file

bf2282c

2bndy5 added 4 commits March 17, 2024 18:12

disable wiringPi in linux CI

1322530

I am not going to try cross-compiling dependencies of dependencies. WiringPi needs libs: crypt and rt compiled for the target system.

reviewed Linux CI (again)

7ff5565

[CI] fix typo in path

9aa37b3

some initial code review

f8decf1

2bndy5 commented Mar 18, 2024

View reviewed changes

utility/pigpio/compatibility.cpp Outdated Show resolved Hide resolved

2bndy5 commented Mar 18, 2024

View reviewed changes

utility/wiringPi/RF24_arch_config.h Outdated Show resolved Hide resolved

TMRh20 approved these changes Mar 18, 2024

View reviewed changes

2bndy5 mentioned this pull request Mar 18, 2024

Bump RF24 from 9eca153 to 05efe34 nRF24/pyRF24#41

Closed

2bndy5 commented Mar 18, 2024

View reviewed changes

utility/pigpio/interrupt.h Outdated Show resolved Hide resolved

2bndy5 added 7 commits March 18, 2024 09:49

prefer using time.h

e56ecf4

remove INT_EDGE_SETUP define (wiringPi-specific meaning)

4f79413

review includes for RF24_arch_config.h files

0b80565

add C linkage instead of declaring extern functions

5ab12b0

more #include statements review

c3845f7

reviewed wiringPi/spi wrapper

d81439f

2bndy5 force-pushed the char-dev-irq branch from c79c60e to 7973ba6 Compare March 18, 2024 18:37

add wiringPi pin number for CE_PIN to linux examples

0df788c

2bndy5 force-pushed the char-dev-irq branch from 8481d6d to 0df788c Compare March 18, 2024 22:27

2bndy5 force-pushed the char-dev-irq branch from 062d011 to 3b3187a Compare March 19, 2024 03:30

2bndy5 merged commit c6c0191 into master Mar 19, 2024
35 checks passed

2bndy5 deleted the char-dev-irq branch March 19, 2024 04:01

wait for thread to terminate before closing pin fd

3b3187a

2bndy5 mentioned this pull request Mar 19, 2024

cache gpio chip detection when properly initialized #962

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IRQ support using Linux kernel Character Device + Posix threads #961

IRQ support using Linux kernel Character Device + Posix threads #961

2bndy5 commented Mar 18, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as resolved.

TMRh20 left a comment

2bndy5 commented Mar 18, 2024 •

edited

Loading

2bndy5 commented Mar 18, 2024

2bndy5 commented Mar 19, 2024

2bndy5 commented Mar 19, 2024 •

edited

Loading

TMRh20 commented Mar 19, 2024

2bndy5 commented Mar 19, 2024 •

edited

Loading

IRQ support using Linux kernel Character Device + Posix threads #961

IRQ support using Linux kernel Character Device + Posix threads #961

Conversation

2bndy5 commented Mar 18, 2024 • edited Loading

Interrupt changes

Changes that follow-up #959

Additional changes

This comment was marked as outdated.

This comment was marked as resolved.

TMRh20 left a comment

Choose a reason for hiding this comment

2bndy5 commented Mar 18, 2024 • edited Loading

2bndy5 commented Mar 18, 2024

On RPi3

On RPi4

2bndy5 commented Mar 19, 2024

2bndy5 commented Mar 19, 2024 • edited Loading

TMRh20 commented Mar 19, 2024

2bndy5 commented Mar 19, 2024 • edited Loading

2bndy5 commented Mar 18, 2024 •

edited

Loading

2bndy5 commented Mar 18, 2024 •

edited

Loading

2bndy5 commented Mar 19, 2024 •

edited

Loading

2bndy5 commented Mar 19, 2024 •

edited

Loading