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

Add SPISettings while keeping __SPISettings as an alias for wiring layer #2138

Merged
merged 1 commit into from
Jun 25, 2020

Conversation

keeramis
Copy link
Contributor

Problem

__SPISettings naming is not compatible with Arduino layer

Solution

Rename __SPISettings to SPISettings and still keep __SPISettings as an alias

Steps to Test

Existing unit tests, but also something like this app should compile without errors

#include "application.h"

void setup() {
    int32_t r = SPI.beginTransaction(__SPISettings(SPI_CLK_SYSTEM, SPI_MODE0, MSBFIRST));
    (void)r;
    int32_t r2 = SPI.beginTransaction(SPISettings(SPI_CLK_SYSTEM, SPI_MODE0, MSBFIRST));
    (void)r2;
}

void loop() {
}

References

ch55527
ch54115


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@keeramis
Copy link
Contributor Author

There are also multiple locations __SPISettings can be changed, and potentially add unit tests for SPISettings instead of / in addition to __SPISettings, but don't think it is needed to change/add this every where.

Copy link
Member

@avtolstoy avtolstoy left a comment

Choose a reason for hiding this comment

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

I'm approving the change, but we should fix all the tests that use __SPISettings as well.

@keeramis keeramis merged commit d1d4c02 into develop Jun 25, 2020
@keeramis keeramis deleted the ch55527/SPISettings_alias branch June 25, 2020 22:10
@keeramis keeramis added ready to merge PR has been reviewed and tested and removed needs review labels Jun 25, 2020
@keeramis keeramis restored the ch55527/SPISettings_alias branch June 26, 2020 17:12
@keeramis keeramis mentioned this pull request Jun 26, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ready to merge PR has been reviewed and tested
Projects
None yet
2 participants