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

Update pico_sdk.md to reflect changes to SPI with user-specified pins #900

Merged
merged 1 commit into from
Mar 11, 2023
Merged

Update pico_sdk.md to reflect changes to SPI with user-specified pins #900

merged 1 commit into from
Mar 11, 2023

Conversation

matt-walker0
Copy link
Contributor

@matt-walker0 matt-walker0 commented Mar 11, 2023

Hi,

Updated some of my code that uses the RF24 library, guess there's been some changes in the meantime as the previous code no longer compiles. Quick look suggests now the SPI needs be declared seperately and cannot be reference just by importing RF24 and using spi.begin

Previous code:

    spi.begin(spi_bus, sck_pin, tx_pin, rx_pin);

    if(radio.begin(&spi, ce_pin, csn_pin) == false) {     // Setup and configure rf radio
        return(false);
    }

Seems to be fixed by declaring a SPI object outside the function that initialises the radio. (could be a better way to do this?)

SPI spi;

Hence have suggested a change to documentation to reflect this.

@2bndy5
Copy link
Member

2bndy5 commented Mar 11, 2023

Please use latest commit on master. I fixed this problem in 557d165

@2bndy5
Copy link
Member

2bndy5 commented Mar 11, 2023

Oh, I see what you mean now.

Yes, to change the pin numbers from their defaults, you need to declare an SPI object and pass it to RF24::begin().

Before, this wasn't needed because the wrapping SPI class had only static functions. I never liked this approach because it hides the declaration from the user. The way it is now should be clearer to C++ learners.

@2bndy5 2bndy5 changed the title Update pico_sdk.md to reflect changes to SPI Update pico_sdk.md to reflect changes to SPI with user-specified pins Mar 11, 2023
@2bndy5 2bndy5 merged commit 6ad390f into nRF24:master Mar 11, 2023
@matt-walker0
Copy link
Contributor Author

Cheers, it was more a case of it fixed things on my end, but indeed does make it clearer for those not using default pins.

@matt-walker0 matt-walker0 deleted the patch-2 branch March 11, 2023 15:00
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