Add support for Generic NRF52805 #442
Conversation
There was a problem hiding this comment.
@nikolac great work!
I do not have an nRF52085 based device board to try these changes out with. However, I made a few inline comments.
boards.txt
Outdated
|
|
||
| Generic_nRF52805.upload.tool=sandeepmistry:openocd | ||
| Generic_nRF52805.upload.target=nrf52 | ||
| Generic_nRF52805.upload.maximum_size=192000 |
There was a problem hiding this comment.
should this be:
0x30000 = 196608
?
libraries/SPI/SPI.cpp
Outdated
| @@ -256,7 +256,7 @@ void SPIClass::detachInterrupt() { | |||
| } | |||
|
|
|||
| #if SPI_INTERFACES_COUNT > 0 | |||
| #if defined(NRF52_SERIES) | |||
| #if defined(NRF52_SERIES) && !defined(NRF52805_XXAA) | |||
There was a problem hiding this comment.
Maybe this could be #if defined(NRF_SPI2)?
There was a problem hiding this comment.
+1 That would make it work on nrf52810 as well.
| @@ -401,17 +401,34 @@ void TwoWire::onService(void) | |||
| } | |||
| } | |||
|
|
|||
| TwoWire Wire(NRF_TWIM1, NRF_TWIS1, SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQn, PIN_WIRE_SDA, PIN_WIRE_SCL); | |||
| #if defined(NRF52805_XXAA) | |||
There was a problem hiding this comment.
Could also consider #if defined(NRF_TWIM1 here as well.
There was a problem hiding this comment.
I believe the NRF52805 has completely different handler names (TWIM0_TWIS0_TWI0_IRQn vs SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQn) which was part of the reason for using the board specific logic.
There was a problem hiding this comment.
Got it, sorry for not checking this out before!
| @@ -64,12 +64,22 @@ void Uart::begin(unsigned long baudrate) | |||
|
|
|||
| void Uart::begin(unsigned long baudrate, uint16_t /*config*/) | |||
| { | |||
| #ifdef NRF52805_XXAA | |||
There was a problem hiding this comment.
Should we be touching code in the SDK? Does that introduce potential upgrade complexity?
There was a problem hiding this comment.
Good question for @sandeepmistry :)
It would be nice to have this consistency between compat .h files, like nrf52_to_nrf52840.h referenced above.
There was a problem hiding this comment.
Should we be touching code in the SDK? Does that introduce potential upgrade complexity?
It would be better to avoid doing this, as it would be one more thing to manage when updating the SDK.
That said if it's the best approach wrapping changes in #ifdef ARDUINO would be best.
There was a problem hiding this comment.
Am I good to leave this as-is for now? There is already series and, in some special cases, chip specific logic in the .cpp library files. I would prefer to leave this condition where it is until we decide on the best way/place to extend SDK definitions without modifying the SDK.
However, I'll do whatever you feel is best. There is increased interest to get this merged in, so whatever I can do to help expedite.
|
Just wanted to follow up on this. I've been using this branch for ~2 years. |
|
What is the status of this pull-request? |
I made a couple of questionable decisions: