arduino-nRF5
arduino-nRF5 copied to clipboard
sandeepmistry
In support of issue #98 (SoftwareSerial not working)
This PR includes a SoftwareSerial library, and is compatible with the arduino-org and adafruit nrf52 SoftwareSerial libraries.
Arduino.h
- fix errors in #defines for digitalPinToPort, portOutputRegister, portModeRegister
WInterrupts
- change attachInterrupt define to return the interrupt mask
- callbacksInt, channelMap now uses an initialiser list instead of memset
- GPIOTE_IRQn priority dropped from 1 to 3 (same as Uart priority)
- LOW and HIGH modes added to attachInterrupt for compatibility, which are the same as FALLING and RISING respectively as GPIOTE does not support HIGH and LOW modes
- To free up clock cycles between GPIOTE interrupts GPIOTE_IRQHandler now searches for the first GPIOTE event and breaks to execute, rather than staying in the interrupt and executing each GPIOTE callback that has triggered an interrupt.
I noticed a comment from @sandeepmistry that you only wanted to include the core libraries so I added my SoftwareSerial variant. Let me know if you prefer the one from arduino-org (which mine is based off) or adafruit and i can merge that in instead.
Im biased so ill say that my variant of the arduino-org one (which i added to this PR) is the best as it allows you to configure the buffer size without modifying the library, and you can configure it to be transmit only (saves a pin, a pin interrupt a little over 1kB of flash). To achieve this it is a header only library, so compile time is slightly longer.
The arduino-org one was first, the adafruit one looks to be a direct copy. All three work, but not as well as the tuned AVR original source as the tuning on the bit timing is courser.
@carlosperate - thankyou for your review! Sorry for not addressing each point in situ, my access is mobile only while on holidays.
First up - thanks for spotting the #ifdef bug, ill fix this!
Ive tested this against the nrf52832 and nrf51822, but it won't work for all pins of the nrf52840. The comment in Arduino.h is a hinter comment that more work is required for the nrf52840.
Copyright is different to licensing, I have not/cannot changed the original license. I dont believe it is possible to sublicense within a source file, but i may be correct. As per your suggestion I'm happy to just add my name to the list for clarity, i just wanted to imply that the prior work was far more significant than my own hacks :blush:
The library.properties file is for the Arduino library manager incase i decide to register this library with them. It also stops the library from being available to incompatible cores i.e. AVR. The Author is generally the most significant contribution, which is Arduino, and i will maintain modifications in my local copy unless it is incorporated in this core at which point the maintainer will need to be changed. I think the original source library version was 1.0.0, but i will check this.
I can confirm that the code in this PR works with the micro:bit (nRF51822) using the included SoftwareSerial library. I was able to drive a Serial MP3 Player board off the micro:bit via a SoftwareSerial interface. Assuming that the above issues are resolved, it would be great if this PR (including SoftwareSerial) could be merged into master, as I've been working on a forked branch that requires both SoftwareSerial as well as other changes. Thanks in advance!
@carlosperate @micooke Could you give me a status update on this PR? I am thinking we may be ready for another release in the near future.
@dlabun, @sandeepmistry - Travis build failed, not sure why. It failed when it tries to get any packages; ubuntu packages, this repo from github.com. Very strange. I made a small update to trigger the build again - hopefully it is resolved by now!?
I think it was just a temporary outage between Travis and Github, it's all good right now
@dlabun I'm unsure what to do about this one. Concerns:
- the priority of
attachInterrupt(...)has be lowered - when I started this project, one goal was to track the official SAMD core. In this case, it doesn't support
SoftwareSerial, but maybe we can diverge from this? - it's a lot more code to maintain
the priority of attachInterrupt(...) has be lowered
This was to put the priority the same as Uart, and lower than Wire interrupts. The concern is that GPIOTE can be triggered with a very high duty cycle, which could interfere with higher-priority tasking like I2C comms. On a related note, RTC interrupt priority is 15 in this core and 0 in the SAMD core. Just something to note as we are talking about interrupt priorities.
when I started this project, one goal was to track the official SAMD core. In this case, it doesn't support
SoftwareSerial, but maybe we can diverge from this? Maybe the SAMD core should be modified to support SoftwareSerial? I would say its a fairly essential library.
it's a lot more code to maintain
The changes i made to support SoftwareSerial are in line with modifications made by Arduino-org and Adafruit (who started with you core by the looks of it). You certainly dont need to use my variant of SoftwareSerial, the core changes make it compatible with the Adafruit and Arduino-org variants.