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.

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

Use BCM2835 lib's Edge Detection Status for IRQ support #969

Merged
merged 4 commits into from
Apr 1, 2024
Merged

Conversation

2bndy5
Copy link
Member

@2bndy5 2bndy5 commented Mar 26, 2024

Just does what the title says, instead of using the Linux Kernel.

github-actions[bot]

This comment was marked as resolved.

@github-actions github-actions bot dismissed their stale review March 27, 2024 03:18

outdated suggestion

@2bndy5 2bndy5 force-pushed the bcm2xxx-isr branch 2 times, most recently from 9c18209 to d2525d5 Compare March 27, 2024 03:24
@2bndy5

This comment was marked as resolved.

@2bndy5 2bndy5 marked this pull request as draft March 27, 2024 05:57
@2bndy5 2bndy5 marked this pull request as ready for review March 29, 2024 04:55
@2bndy5
Copy link
Member Author

2bndy5 commented Mar 29, 2024

This work on my RPi3 and my RPi4. 🎉

@TMRh20
Copy link
Member

TMRh20 commented Mar 30, 2024

On my RPi4 I can't get it to work even with the basic gettingstarted example.
On my RPi Model B it works with the gettingstarted and interruptConfigure examples, but not with the Gateway examples...
Not sure if user error or what, but I will see about finding time to mess around with this more over the weekend.

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 30, 2024

I usually have to reboot my RPi when changing between drivers now.

Copy link
Member

@TMRh20 TMRh20 left a comment

Choose a reason for hiding this comment

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

I'm really not sure where the problem is but the interrupts just don't trigger when I use the gateway example. Everything seems to be functional using the regular ncurses example, but interrupts just get missed with the interrupt based example. This might take some time to figure out, but in the meantime, it works at the RF24 layer, so I'll assume something is wrong at the gateway layer.

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

Has this happened after a long runtime (like days of uptime)?

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

oops. I didn't see which thread this was. I thought it was the gateway PR (& I assumed SPIDEV).

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

the bcm2xxx lib has asynch variants of the rising/falling edge detect status, but I didn't think it would be helpful as the docstring describes them as super sensitive:

RF24/utility/RPi/bcm2835.h

Lines 1538 to 1544 in b4edfbd

/*! Enable Asynchronous Falling Edge Detect Enable for the specified pin.
When a falling edge is detected, sets the appropriate pin in Event Detect Status.
Asynchronous means the incoming signal is not sampled by the system clock. As such
falling edges of very short duration can be detected.
\param[in] pin GPIO number, or one of RPI_GPIO_P1_* from \ref RPiGPIOPin.
*/
extern void bcm2835_gpio_afen(uint8_t pin);

I was worried about debouncing again. We could switch to the async variants and try that instead...

@TMRh20
Copy link
Member

TMRh20 commented Mar 31, 2024

Same thing, works at RF24 layer, but not triggering the interrupt function at Gateway. Its slow going because I'm testing on an old RPi model B

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I found no problems running the example using the async variants on RPi3 & RPi4.


I think my SD card in my RPi4 is deteriorating. 😭 Its been bricking lately; prompting me to pull the power, wait a few minutes, then power back up. 😱

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I see the gateway curses_int example only calls attachInterrupt(). I'm not sure, but I think the BCM2xxx lib might also require setting the pin to input:

    pinmode(interruptPin, INPUT);
    attachInterrupt(interruptPin, INT_EDGE_FALLING, intHandler);

that's the only difference I noticed between the 2 layers' interrupt examples.

@TMRh20
Copy link
Member

TMRh20 commented Mar 31, 2024

Same thing with pinMode(), no triggering of the interrupt function.

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I'm falling asleep (finally)... The way I see it, the only real problem point might be in gw.poll() (relatively timed: after/before invoking gw.update(true)). I'll investigate further tomorrow.

@TMRh20
Copy link
Member

TMRh20 commented Mar 31, 2024

I re-tested on my RPi4 as well and I'm getting this repeating very quickly on receive, nothing on transmit with the gettingstarted example:

Received 4 bytes on pipe 7: 8.3773e-21
Received 4 bytes on pipe 7: 0.185784
Received 4 bytes on pipe 7: 0.154412
Received 4 bytes on pipe 7: 1.00714e-20
Received 4 bytes on pipe 7: 8.37068e-21

My RPi3 gives same results as the model B, RF24 layer works, no ISR at Gateway...

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I have to lower my SPI speed to about 6MHz on my RPi4 with the RPi driver (still using that adapter board you sent me).

@TMRh20
Copy link
Member

TMRh20 commented Mar 31, 2024

Hmm, lowering the SPI speed works for the gettingstarted example, but with the interruptConfigure example it gives Timeout was reached. Going back to setRole() on receive, and no IRQ on TX.
I think it must be the BCM driver itself not functioning correctly on my RPi4. I'm betting because I'm running 64-bit bookworm. Are you running 32 or 64 bit on your RPi4?

Btw, I also made some more RF24/RPi shields that sit inside the case, with a bit better design because it was my second go-around.

I'm using one or the other on all my RPis!

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

Are you running 32 or 64 bit on your RPi4?

I'm also using 64 bit RPi OS bookworm. I don't have problems with example defaults on my RPi3. I've got in the habit of sudo reboot between installing with different RF24 drivers.


Nice redesign! I was thinking of getting some stacking header pins (to use for that shield), so I could get access to my I2C pins while the shield was still attached. I'm lazy though. I just use a different RPi3 (Model A+) for testing my CirquePinnacle lib (which uses both SPI & I2C).

@TMRh20
Copy link
Member

TMRh20 commented Mar 31, 2024

Yeah, I've rebooted as well, but you can also sudo modprobe -r spi-bcm2835 then sudo modprobe spi-bcm2835 to reload the module.
I don't know what to say, technically it works on the other RPis with the same configuration and code, so it must be something specific to my RPi4 then?

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I was more concerned with how different drivers leave my GPIO access (sysfs state for most of them except SPIDEV and RPi) after Ctrl+C. That and there was some memory leakage when developing the newer GPIO stuff for SPIDEV.

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I have no idea what could be the problem. It is sounding hardware-specific though.

@2bndy5
Copy link
Member Author

2bndy5 commented Mar 31, 2024

I think we're pinned to bcm2835 lib v1.62. I see there have been some updates since, currently up to v1.75. I could try updating our copy, but I doubt it would effect the edge detection feature.

About secondary SPI (a tangential note)

There is a patch released in v1.75 about bcm2835_aux_spi_transfernb(), but we're not using that function. In fact, I don't think our SPI wrappers for the bcm2835 lib even allow for using a secondary SPI bus. I also just noticed that there is no error handling in the utility/RPi/spi.cpp.

bcm2835_spi_begin();

RF24/utility/RPi/bcm2835.h

Lines 1657 to 1665 in c6d05b8

/*! Start SPI operations.
Forces RPi SPI0 pins P1-19 (MOSI), P1-21 (MISO), P1-23 (CLK), P1-24 (CE0) and P1-26 (CE1)
to alternate function ALT0, which enables those pins for SPI interface.
You should call bcm2835_spi_end() when all SPI funcitons are complete to return the pins to
their default functions.
\sa bcm2835_spi_end()
\return 1 if successful, 0 otherwise (perhaps because you are not running as root)
*/
extern int bcm2835_spi_begin(void);

I have revised the bcm2835 wrappers in my CirquePinnacle lib in an attempt to add support for secondary SPI bus, but I also neglected proper error handling.

@2bndy5
Copy link
Member Author

2bndy5 commented Apr 1, 2024

According to BCM2835 lib docs:

Tested on debian6-19-04-2012, 2012-07-15-wheezy-raspbian, 2013-07-26-wheezy-raspbian and Occidentalisv01, 2016-02-09 Raspbian Jessie. CAUTION: it has been observed that when detect enables such as bcm2835_gpio_len() are used and the pin is pulled LOW it can cause temporary hangs on 2012-07-15-wheezy-raspbian, 2013-07-26-wheezy-raspbian and Occidentalisv01. Reason for this is not yet determined, but we suspect that an interrupt handler is hitting a hard loop on those OSs. If you must use bcm2835_gpio_len() and friends, make sure you disable the pins with bcm2835_gpio_clr_len() and friends after use.

Maybe its not a good idea to make IRQ support specific to the BCM2835 lib. Still its wierd that these changes work in RF24 layer but not in Gateway layer.

@TMRh20
Copy link
Member

TMRh20 commented Apr 1, 2024

It might be time to begin looking at phasing out BCM lib support altogether, its an old driver and not being updated for the newer RPi5. I don't see the need to fully support this driver since we have so many other options, SPIDEV, PiGPIO, WiringPi, MRAA which work well.

@2bndy5
Copy link
Member Author

2bndy5 commented Apr 1, 2024

I'm of the opinion that all but SPIDEV should be phased out.

  • MRAA isn't well maintained since Intel passed it on to the eclipse github org; contributions don't seem to be well verified/tested. And it stopped working on the RPi4.
  • BCM2835 can't really support RPi5 using interactions with CPU registers due to the hardware abstraction on RPi5.
  • pigpio is practically in the same position as BCM2835 lib concerning RPi5 support. They'd have to overhaul the lib's internals to behave more like the newer lgpio lib of the same author, but that would introduce breaking changes to the API.
  • wiringPi has been limping on via sparse community support since Gordon ended official development. The more I dive into that lib's source, the less appealing it is. Long story made short: its old and not well written.
  • Has littlewire ever worked? The source I found is not compatible with the driver API we have. The header that our littlewire driver looks for no longer exists in the littlewire sources, and I can't even figure out when that changed using git blame.

@TMRh20
Copy link
Member

TMRh20 commented Apr 1, 2024

I think you're right, while keeping them is nice its a lot of extra work, but lets go a bit slowly with actually removing support tho. We can mark them deprecated and suggest users move to the SPIDEV driver.

@2bndy5
Copy link
Member Author

2bndy5 commented Apr 1, 2024

Removing them is definitely a v2.0 idea. I'll open an issue to track it and add it to the "v2.0 ideas" project.

@2bndy5
Copy link
Member Author

2bndy5 commented Apr 1, 2024

I don't expect to actually remove anything until RPi7 gets produced. We can hard-code the build systems to select SPIDEV despite the presence of other need libraries. This should be the least breaking approach we can do to dissuade users from selecting other (seemingly unreliable) drivers.

For now, is this ok to merge?

@TMRh20
Copy link
Member

TMRh20 commented Apr 1, 2024

Yup

@2bndy5 2bndy5 merged commit eade71f into master Apr 1, 2024
19 checks passed
@2bndy5 2bndy5 deleted the bcm2xxx-isr branch April 1, 2024 11:03
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