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

[OrangePi Zero] Linux chip enable conditional statement #904

Merged
merged 3 commits into from
Mar 29, 2023

Conversation

fertinator
Copy link
Contributor

@fertinator fertinator commented Mar 27, 2023

Issue: RF24 radio(10, 10, 1000000)
OrangePi Zero with GPIO10 as chip enable. OrangePi Zero uses spidev1.0 because spidev0.0 is used for flash. Then if statement is true and does not enable TX on nRF module. Conditional statement fixes this issue; The gettingstarted.cpp program works now with the modification

See #903 for the discussion

Issue: RF24 radio(10, 10, 1000000) 
OrangePi with GPIO10 as chip enable. SPIDEV1.0 is used. Then if statement is true and does not enable TX on nRF module.
Conditional statement fixes this
@fertinator fertinator changed the title Linux chip enable conditional statement [OrangePi Zero] Linux chip enable conditional statement Mar 27, 2023
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 checked the various driver's RF24_arch_config.h, and this only affects Linux: WiringPi, MRAA, RPi, SPIDEV - only the platforms where the CSN number is used as a SPI bus+device number (not an actual GPIO number).

@2bndy5
Copy link
Member

2bndy5 commented Mar 27, 2023

Something is up with github's servers. I'll have to let this sit for a day and re-run the Arduino CI when the servers aren't suffering from whatever it is.

@2bndy5
Copy link
Member

2bndy5 commented Mar 27, 2023

Arduino CI is passing except the delta size report is failing to post to the PR thread (probably a user permission problem due to a third party fork). All looks good to me though.

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.

Please revert the changes about using wiringPi.

  1. WiringPi support is now deprecated in RF24 because it no longer ships with RPi OS (since bullseye builds). It was appropriately removed from distribution because the author of wiringPi issued an end-of-life notice in August 2019.
  2. After some research, I see that wiringPi is still officially recommended for use on OrangePi boards via their own customized fork dubbed WiringOP. However, the wiringPi support in RF24 was not designed to accommodate unofficial modifications (which is one of the reasons why the author stopped maintaining the original wiringPi lib). Ultimately, we have no way to guarantee support for customized forks of wiringPi.

I'm open to discuss these changes further in a different issue/PR because I understand the need for using RF24 with wiringOP 's interrupt support. Additionally, there are other compensations that must be made to RF24's build system because CMake is now the recommended build system (actually required for a 64-bit OS).

@fertinator
Copy link
Contributor Author

Ah okay. Sorry about that. I am new to git.
My intention was to modify my own fork so I could get it to work for the OrangePi. And indeed I am using that wiringOP, but it shows up as libwiringPi. Maybe that's the confusion there, but I want to be able to use the IRQ functionality, so that's why I implemented the wiringOP.
The idea was to use it just for myself at this point.

Should I revert the changes? And do you have suggestions how to keep my changes without interfering with the main? I thought a fork would be sufficient?

@2bndy5
Copy link
Member

2bndy5 commented Mar 29, 2023

Should I revert the changes?

Yes.

I am new to git. [...] And do you have suggestions how to keep my changes without interfering with the main? I thought a fork would be sufficient?

You can have a look at how to use branches. I didn't say anything before when I noticed this patch was coming from your fork's master branch. Typically, you should submit a patch upstream from a non-default branch on your fork; there are a number of good reasons why this is best practice.

As for keeping the changes you already pushed: There are ways to use git for this (eg cherry-picking commits), but the easiest for beginners is to create a new branch, and re-commit the changes to the new branch.

@fertinator
Copy link
Contributor Author

Alright I will revert changes.

Are you saying that I should then branch my fork?

@2bndy5 2bndy5 merged commit 0d101ae into nRF24:master Mar 29, 2023
@2bndy5 2bndy5 added the bugfix label Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants