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

GPIO changes (RPi5 support) #942

Merged
merged 18 commits into from
Feb 18, 2024
Merged

GPIO changes (RPi5 support) #942

merged 18 commits into from
Feb 18, 2024

Conversation

TMRh20
Copy link
Member

@TMRh20 TMRh20 commented Feb 17, 2024

I think this is pretty much ready to go now, I've been testing it out on all my RPis with various code/RF24 layers and it seems to work fine.

Re: #932

@TMRh20
Testing GPIO
879432f 
Very rudimentary changes for testing #932
@TMRh20
Add error handling for GPIO in SPIDEV
6a78ecf
@TMRh20
Minor update
edab1cd 
- Try using gpiochip4 then 0 if that fails to support RPi5
- Remove old GPIO code
@TMRh20
Formatting
ded775b
@TMRh20 @2bndy5
Testing GPIO
51cccd9 
Very rudimentary changes for testing #932
@TMRh20 @2bndy5
Add error handling for GPIO in SPIDEV
752c9b6
@TMRh20 @2bndy5
Minor update
d068e75 
- Try using gpiochip4 then 0 if that fails to support RPi5
- Remove old GPIO code
@TMRh20 @2bndy5
Formatting
86d0c38
@TMRh20
Merge branch 'GPIO_Changes' of https://github.com/nRF24/RF24 into GPI…
f6d8c42 
…O_Changes
github-actions[bot]

This comment was marked as duplicate.

Sign in to view

2bndy5
2bndy5 requested changes Feb 17, 2024
View reviewed changes
Copy link
Member

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

I haven't tested this on any of my own hardware yet. This review is just my first glance reactions.

utility/SPIDEV/gpio.h Outdated Show resolved Hide resolved
utility/SPIDEV/gpio.cpp Outdated Show resolved Hide resolved
utility/SPIDEV/gpio.cpp Show resolved Hide resolved
utility/SPIDEV/gpio.cpp Outdated Show resolved Hide resolved
2bndy5
2bndy5 reviewed Feb 17, 2024
View reviewed changes
utility/SPIDEV/gpio.cpp Outdated Show resolved Hide resolved
2bndy5
2bndy5 reviewed Feb 17, 2024
View reviewed changes
utility/SPIDEV/gpio.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot dismissed their stale review February 17, 2024 21:12

outdated suggestion

github-actions[bot]

This comment was marked as duplicate.

Sign in to view

@github-actions github-actions bot dismissed their stale review February 17, 2024 23:35

outdated suggestion

github-actions[bot]

This comment was marked as duplicate.

Sign in to view

@github-actions github-actions bot dismissed their stale review February 17, 2024 23:52

outdated suggestion

github-actions[bot]

This comment was marked as outdated.

Sign in to view

TMRh20 and others added 4 commits February 17, 2024 17:57
@TMRh20
Revert renaming of gpio functions
da504fc
@TMRh20 @github-actions
Update utility/SPIDEV/gpio.cpp
8a3be52 
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@TMRh20 @github-actions
Update utility/SPIDEV/gpio.cpp
8d577f3 
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@TMRh20
Merge branch 'GPIO_Changes' of https://github.com/nRF24/RF24 into GPI…
155657a 
…O_Changes
@github-actions github-actions bot dismissed their stale review February 17, 2024 23:58

outdated suggestion

github-actions[bot]

This comment was marked as outdated.

Sign in to view

@github-actions github-actions bot dismissed their stale review February 17, 2024 23:58

outdated suggestion

github-actions[bot]

This comment was marked as duplicate.

Sign in to view

@github-actions github-actions bot dismissed their stale review February 17, 2024 23:59

outdated suggestion

github-actions[bot]

This comment was marked as duplicate.

Sign in to view

@TMRh20
Copy link
Member Author

TMRh20 commented Feb 18, 2024

Gotta say as someone who struggles with formatting and syntax, this Linter Bot is great. Hehehe.

@2bndy5
Copy link
Member

2bndy5 commented Feb 18, 2024

I figured you'd enjoy it 😉. I was worried the bot might be perceived as spamming the conversation... I opened cpp-linter/cpp-linter#77 to prevent the duplicated LGTM reviews.

@github-actions github-actions bot dismissed their stale review February 18, 2024 00:16

outdated suggestion

github-actions[bot]

This comment was marked as duplicate.

Sign in to view

@github-actions github-actions bot dismissed their stale review February 18, 2024 00:34

outdated suggestion

github-actions[bot]
github-actions bot approved these changes Feb 18, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

No concerns from clang-format.

Great job! 🎉

Have any feedback or feature suggestions? Share it here.

2bndy5
2bndy5 approved these changes Feb 18, 2024
View reviewed changes
Copy link
Member

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

This worked on my RPi3 & RPi4 (both using RPi OS bookworm with latest updates).

Maybe in the future we can use this character device API to implement IRQ using the Linux kernel... For now this solves #932

This was linked to issues Feb 18, 2024
RPi5 uses different gpio pin numbers (per Linux kernel update) #932
Closed
ESP32 Core Panic on radio 0; No panic on Radio number 1, same wiring. How to troubleshoot?[Question] #939
Closed
@2bndy5 2bndy5 added the bugfix label Feb 18, 2024
@2bndy5 2bndy5 removed a link to an issue Feb 18, 2024
ESP32 Core Panic on radio 0; No panic on Radio number 1, same wiring. How to troubleshoot?[Question] #939
Closed
@TMRh20
Copy link
Member Author

TMRh20 commented Feb 18, 2024

Maybe in the future we can use this character device API to implement IRQ using the Linux kernel...

I like the "maybe in the future" part. The rest sounds like a lot of work. 😜

Really though, might be something to look into.

@TMRh20 TMRh20 merged commit ee9c017 into master Feb 18, 2024
19 checks passed
@TMRh20 TMRh20 deleted the GPIO_Changes branch February 18, 2024 03:13
@2bndy5
Copy link
Member

2bndy5 commented Feb 18, 2024

This guide does mention how IRQ could be done using some poll() function (which is unknown to me), but it uses a blocking example.

The main obstacle (for me) is using a separate processor thread to poll the GPIO for IRQ edge detection.

@TMRh20
Copy link
Member Author

TMRh20 commented Feb 18, 2024

Hmm, maybe u should open an issue for this enhancement. I played around with threads before, the IRQ stuff has gone through a bunch of changes. It can't be that hard. hehe.

This was referenced Feb 18, 2024
Closed
Closed
TMRh20 added a commit that referenced this pull request Feb 19, 2024
@TMRh20
GPIO V2 API changes
183e159 
#942 #943
@TMRh20 TMRh20 mentioned this pull request Feb 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@2bndy5 2bndy5 2bndy5 approved these changes

@github-actions github-actions[bot] github-actions[bot] approved these changes

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

Successfully merging this pull request may close these issues.

RPi5 uses different gpio pin numbers (per Linux kernel update)
2 participants
@TMRh20 @2bndy5