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

Reworked the utility/rp2 SPI class and removed static members #892

Merged
merged 2 commits into from
Jan 23, 2023

Conversation

TheGarkine
Copy link
Contributor

In the discussion in #891, we came to the conclusion, that the members in the rp2 utility SPI class should not be static, since it is otherwise not possible to connect two (or more) radios on multiple SPI busses.

I refactored this behavior and added an example of how to set up multiple radios on different SPI buses.

…be used to connect multiple Radios. Also added an example for this use-case
@2bndy5
Copy link
Member

2bndy5 commented Jan 20, 2023

I'm not sure we need a new example to demo this. I'd actually prefer to have the necessary code as an example snippet in the /docs/pico_sdk.md at the bottom in a new section titled "## Multi-Radio Scenario". My reason here is because the examples use examples_pico/defaultPins.h to declare the CE, CSN, and IRQ pins used for a single radio. I use defaultPins.h to build the Pico SDK examples for multiple RP2040 boards (some of which don't have enough pins exposed for 2 radios).

I can force push this change if you want (not sure if you're comfortable with force pushed commits). You do seem very proficient in English writing 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.

I moved the example into the pico_sdk doc because it wasn't actually doing anything with the radio objects.

@TheGarkine
Copy link
Contributor Author

Sorry, I was a bit busy on the weekend. Yeah, that's totally understandable reasoning, and thank you for your initiative! Do you need any other changes in order to move forward with this PR?

@2bndy5
Copy link
Member

2bndy5 commented Jan 23, 2023

No, everything LGTM.

@2bndy5 2bndy5 merged commit b5a905b into nRF24:master Jan 23, 2023
2bndy5 added a commit that referenced this pull request Mar 3, 2023
a regression from #892 about
using the default instance of the SPI wrapping class for PicoSDK
2bndy5 pushed a commit that referenced this pull request Mar 11, 2023
@XDeschuyteneer

This comment was marked as off-topic.

@2bndy5
Copy link
Member

2bndy5 commented Aug 12, 2023

@XDeschuyteneer I moved your comment to a separate thread #912. Please continue your inquiry there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants