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

Simplify interrupt handling #32

Merged
merged 7 commits into from
Mar 19, 2024
Merged

Simplify interrupt handling #32

merged 7 commits into from
Mar 19, 2024

Conversation

TMRh20
Copy link
Member

@TMRh20 TMRh20 commented Mar 17, 2024

  • Simplify interrupt handling for RF24Gateway:
    a: Change interrupt to toggle a variable instead of actually handling radio data
    b: Move radio handling into the poll() function which is called anyway from the main loop
  • Update examples to remove interrupt enable/disabling
  • Formatting

- Simplify interrupt handling for RF24Gateway
- Update examples to remove interrupt enable/disabling
github-actions[bot]

This comment was marked as resolved.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot dismissed their stale review March 17, 2024 21:39

outdated suggestion

github-actions[bot]

This comment was marked as resolved.

@github-actions github-actions bot dismissed their stale review March 17, 2024 21:40

outdated suggestion

@TMRh20 TMRh20 requested a review from 2bndy5 March 17, 2024 21:41
@@ -83,11 +82,9 @@ int main(int argc, char** argv)
mesh.begin();
}

gw.interrupts(); // Re-enable interrupts when done accessing the radio
Copy link
Member

Choose a reason for hiding this comment

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

This kind of API change implies a feature (aka minor) version bump. We should probably add something to CMakeLists.txt that checks RF24 lib version. CMake's find_library() doesn't seem to care about the found lib's version, so I might have to dig deeper into how to mandate a certain RF24 version -- find_library()'s VALIDATOR arg (new in CMake v3.25) seems like a strong lead.

BTW, I was thinking this new Linux kernel IRQ support would be released under RF24 v1.5.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well most users should be using the installer, using the latest versions, but I see where you're going and its probably a good idea. Although these changes should still work with previous RF24 versions in theory.

Copy link
Member

Choose a reason for hiding this comment

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

CMake's find_package() has a version verification if a required version is specified! 👀 I'm not sure if that will work with RF24 as a shared library though.

@2bndy5
Copy link
Member

2bndy5 commented Mar 17, 2024

I see now why you were so enthusiastic about Linux kernel IRQ support. Hopefully they don't break our implementation of it anytime soon.

@TMRh20
Copy link
Member Author

TMRh20 commented Mar 17, 2024

Haha, that was kind of an epic thread and coding session, but I think we are good for a while.

- Add a longer delay similar to the polling example to minimize CPU usage
TMRh20 added 2 commits March 18, 2024 01:52
- Only delay if no IRQ and no radio data available
- When using interrupts, still use the old waitDelay variable to set the delay in the poll() function
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'll experiment with the version checking in CMake during a different PR.

@TMRh20 TMRh20 merged commit 2fd377f into master Mar 19, 2024
8 checks passed
@TMRh20 TMRh20 deleted the IRQ-FIxes branch March 19, 2024 09:45
2bndy5 pushed a commit that referenced this pull request Jun 11, 2024
* Simplify interrupt handling
- Simplify interrupt handling for RF24Gateway
- Update examples to remove interrupt enable/disabling

* Modify delay
- Add a longer delay similar to the polling example to minimize CPU usage

* Only delay if no action
- Only delay if no IRQ and no radio data available

* Make delay configurable by users
- When using interrupts, still use the old waitDelay variable to set the delay in the poll() function
@2bndy5
Copy link
Member

2bndy5 commented Jun 11, 2024

cherry-picked this change to v1.x branch

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