arduino-nRF5
arduino-nRF5 copied to clipboard
sandeepmistry
Fix bug #169 - analogWrite
wiring_analog_* : fallback to digitalWrite if no available PWM channel (copied from AVR core) wiring_analog_nRF52.c : convert pwmChannelPins & pwmChannelSequence -> pwmContext to semi-standardise pwm pin allocation and pwm status between nRF51 and nRF52 wiring_private.h : Move pwm structures defines out of wiring_analog_* into wiring_private and make the instantiation of these structure externs instead of statics wiring_digital.c : disable the appropriate pwm timer when a digitalWrite is sent to an allocated (from analogWrite) pwm pin, and free up the pwm channel for re-allocation
Ok, can of worms officially opened :boom: nrf51 is done, but i noticed something i should have noticed earlier - the nRF52 only uses 3 out of 12 PWM channels. It has 3 PWM/TIMERS with 4 compare channels each, and in the core each compare channel output pin points to the same pin - so 3 outputs max (currently). This will need some work to fix it up, and luckily now i have the hardware to test it on!
Sorry i should have asked, @sandeepmistry , @dlabun - do you want me to fix the nRF52 PWM channel issue here or in a separate PR?
The two issues are somewhat linked in that to enable the extra pwm i need to add another parameter in the PWMContext structure, which will then require me to update how the TIMER/PWM modules are turned off.
Update :
- 6x analog/PWM signals enabled for nRF51 (up from 3)
- 12x analog/PWM signals enabled for nRF52 (up from 3)
The nRF51 now uses TIMER2 as well as TIMER1. As i have said before, the softdevice uses TIMER0 so i didn't touch that.
The nRF5x documentation is lacking pretty severely compared to the AVR chips I have dealt with. Im very surprised but the implementation for the nRF51 uses software to set/clear the hardware pins. It also uses the first channel as the period interrupt, rather than having a separate overflow interrupt (which can be seen in the RBL core and several Nordic source code).
It basically looks like PWM is implemented rather poorly in the nRF51, and i guess that is the reason that the nRF52 has dedicated PWM channels.
Hi @sandeepmistry, is there an issue with this PR I haven't addressed or does it just need more testing?
@micooke @dlabun this one needs some more testing.
I'm concerned of the performance implications of calling analogWrite(pin, 0) for every digitalWrite(pin, value) call.
The idea for that was actually copied from the Arduino AVR core https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/wiring_analog.c#L112-L119
Which uses digitalWrite() if 0 or 255 are passed to analogWrite()