Skip to content

Conversation

@spiderkeys
Copy link
Contributor

When working with our SAMD21J18A variant, we implement three I2C interfaces, and I found that the extern declarations were missing when I tried to use Wire1 and Wire2. Added support here for all six potential I2C instances.

@drewfish
Copy link

Do Serial and SPI take a similar approach?

@spiderkeys
Copy link
Contributor Author

SPI does, although I see it utilizes the same #ifdef conditionals as the .cpp files for both SPI and the Wire library when it comes to actually creating the instance:

#if SPI_INTERFACES_COUNT > 0
  extern SPIClass SPI;
#endif
#if SPI_INTERFACES_COUNT > 1
  extern SPIClass SPI1;
#endif
etc...

There is no harm adding that here for the Wire library as well, so I'll do that.

As for the Serial class (Uart), it doesn't manifest itself as an external library and exists as part of the board core. The externs for the Uart objects live in variant.h and the instancing happens in variant.cpp. Leads me to wonder if Serial should be brought out and treated as a library the same way as SPI and Wire. I wouldn't argue for bringing Wire and SPI into the core. Either way, the use of the SERCOM instances is currently somewhat inconsistent.

…onditionally declare Wire interface externs
@sandeepmistry
Copy link
Contributor

@spiderkeys do you have a link to your "SAMD21J18A variant"?

@spiderkeys
Copy link
Contributor Author

Still a work in progress, but it currently produces working sketches with all peripherals verified except SPI:
https://github.com/OpenROV/OROV-ArduinoCores

It has some very hacky recipes with hardcoded paths, because we build with arduino-builder on an ARM computer, which currently isn't automatically detecting paths for runtime tools and libs correctly.

Also, the branch in that repo currently only implements two Wire interfaces, though this still prompts the issue.

@sandeepmistry sandeepmistry merged commit 5c19b45 into arduino:master Jan 20, 2016
@sandeepmistry
Copy link
Contributor

@spiderkeys thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants