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

pigpio spi driver not using CSN value properly #894

Closed
2bndy5 opened this issue Feb 11, 2023 · 0 comments · Fixed by #895
Closed

pigpio spi driver not using CSN value properly #894

2bndy5 opened this issue Feb 11, 2023 · 0 comments · Fixed by #895

Comments

@2bndy5
Copy link
Member

2bndy5 commented Feb 11, 2023

I've been reviewing the portability drivers (using a different Arduino lib to implement them), and I found that the pigpio/spi.cpp doesn't appropriately parse the CSN value (which should correspond to /dev/spidevA.B) like the SPIDEV driver does.

See pigpio docs about spiOpen()

If a user specifies csn_pin = [1 | 0], then the correct SPI bus is initialized, but using the secondary SPI bus requires the user to specify 1x where x is the spiChan param. I believe the correct way to specify the secondary SPI bus in pigpio is to use bit 8 in the spiFlags param.

Additional context

If RF24 allowed for using RF24::begin(_SPI*) in Linux, then it might be easier to use a user-instantiated SPI-wrapping object, but that's more of a breaking change 👎🏼 .

preludes to separate issues I have yet to open (still researching/testing)

I've also been exploring how to restructure the lib using a src folder. This would require extra compiler guards in our drivers' srcs since the Arduino IDE compiles everything found in a lib's src folder. The build system wouldn't suffer breaking changes since that stuff is flexible enough. The #include <RF24.h> would still work as it does now. This seems unimportant, but our root folder is getting rather cluttered (and people browsing GitHub aren't readily seeing our README).

It also occurred to me that our drivers' wrapping classes (tersely named) may conflict with a 3rd party lib's classes. But a namespace (ie RF24_arduino_wrappers) can be used to avoid that. Furthermore, we can add using namespace in the scope of RF24 functions that use the namespaced declarations.

void ce(bool level)
{
#ifndef ARDUINO
    using namespace RF24_arduino_wrappers;
#endif
    digitalWrite(ce_pin, level); // uses RF24_arduino_wrappers::GPIO::write() declared in driver's gpio.h
}

This will ensure our wrapper API would only get used internally (as opposed to polluting the global namespace). This is important because some of our Arduino-wrapping implementations are designed specifically for our use case, not for general usage by other Arduino libs (which might require unexposed functionality).

2bndy5 added a commit that referenced this issue Feb 11, 2023
@2bndy5 2bndy5 linked a pull request Feb 11, 2023 that will close this issue
2bndy5 added a commit that referenced this issue Feb 12, 2023
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 a pull request may close this issue.

1 participant