Skip to content

Fix bug #169 - analogWrite #195

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix bug #169 - analogWrite #195

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ddb539
Fix bug #169
micooke Sep 29, 2017
532d7d6
* split PWM_COUNT to PWM_MODULE_COUNT and PWM_CHANNEL_COUNT (N module…
micooke Oct 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 17 additions & 22 deletions cores/nRF5/wiring_analog_nRF51.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,16 @@
extern "C" {
#endif

#define PWM_COUNT 3
#define PIN_FREE 0xffffffff

struct PWMContext {
uint32_t pin;
uint32_t value;
uint32_t channel;
uint32_t mask;
uint32_t event;
};
static uint32_t adcReference = ADC_CONFIG_REFSEL_SupplyOneThirdPrescaling;
static uint32_t adcPrescaling = ADC_CONFIG_INPSEL_AnalogInputOneThirdPrescaling;

static struct PWMContext pwmContext[PWM_COUNT] = {
struct PWMContext pwmContext[PWM_COUNT] = {
{ PIN_FREE, 0, 1, TIMER_INTENSET_COMPARE1_Msk, 1 },
{ PIN_FREE, 0, 2, TIMER_INTENSET_COMPARE2_Msk, 2 },
{ PIN_FREE, 0, 3, TIMER_INTENSET_COMPARE3_Msk, 3 }
};

static int timerEnabled = 0;

static uint32_t adcReference = ADC_CONFIG_REFSEL_SupplyOneThirdPrescaling;
static uint32_t adcPrescaling = ADC_CONFIG_INPSEL_AnalogInputOneThirdPrescaling;
struct PWMStatus pwmStatus[PWM_TIMER_COUNT] = {0, TIMER1_IRQn};

static uint32_t readResolution = 10;
static uint32_t writeResolution = 8;
Expand Down Expand Up @@ -229,7 +218,7 @@ void analogWrite( uint32_t ulPin, uint32_t ulValue )

ulPin = g_ADigitalPinMap[ulPin];

if (!timerEnabled) {
if (pwmStatus[0].numActive == 0) {
NVIC_SetPriority(TIMER1_IRQn, 3);
NVIC_ClearPendingIRQ(TIMER1_IRQn);
NVIC_EnableIRQ(TIMER1_IRQn);
Expand All @@ -245,8 +234,6 @@ void analogWrite( uint32_t ulPin, uint32_t ulValue )
NRF_TIMER1->INTENSET = TIMER_INTENSET_COMPARE0_Msk;

NRF_TIMER1->TASKS_START = 0x1UL;

timerEnabled = true;
}

for (int i = 0; i < PWM_COUNT; i++) {
Expand All @@ -259,17 +246,25 @@ void analogWrite( uint32_t ulPin, uint32_t ulValue )
| ((uint32_t)GPIO_PIN_CNF_DRIVE_S0S1 << GPIO_PIN_CNF_DRIVE_Pos)
| ((uint32_t)GPIO_PIN_CNF_SENSE_Disabled << GPIO_PIN_CNF_SENSE_Pos);

ulValue = mapResolution(ulValue, writeResolution, 8);

pwmContext[i].value = ulValue;

NRF_TIMER1->CC[pwmContext[i].channel] = ulValue;

NRF_TIMER1->INTENSET = pwmContext[i].mask;

break;
pwmStatus[0].numActive++;
return;
}
}

// fallback to digitalWrite if no available PWM channel
if (ulValue < 128)
Copy link
Owner

Choose a reason for hiding this comment

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

Can the write resolution be higher than 8-bits? If so, we need to adjust this.

Copy link
Contributor Author

@micooke micooke Oct 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, and on it!

One thing i noticed is that the read and write resolution isnt bounded and for the NRF52 they are stored as signed integers so you could set the read or write resolution to +/-2e9!

If its alright ill add a static int with the half max value and a switch statement to set it in writeResolution so that valid values are used (and so they arent re-calculated on each analogWrite)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference;

  • nRF51 TIMER1 (the only timer currently used for PWM) is either 8b or 16b write resolution
  • nRF52 is up to 32b write resolution for all 12 PWM channels

{
digitalWrite(ulPin, LOW);
}
else
{
digitalWrite(ulPin, LOW);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be HIGH - copy/paste error

}
}

void TIMER1_IRQHandler(void)
Expand Down
37 changes: 23 additions & 14 deletions cores/nRF5/wiring_analog_nRF52.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,19 @@ extern "C" {
static uint32_t saadcReference = SAADC_CH_CONFIG_REFSEL_Internal;
static uint32_t saadcGain = SAADC_CH_CONFIG_GAIN_Gain1_5;

#define PWM_COUNT 3

static NRF_PWM_Type* pwms[PWM_COUNT] = {
NRF_PWM_Type* pwms[PWM_COUNT] = {
NRF_PWM0,
NRF_PWM1,
NRF_PWM2
};

static uint32_t pwmChannelPins[PWM_COUNT] = {
0xFFFFFFFF,
0xFFFFFFFF,
0xFFFFFFFF
struct PWMContext pwmContext[PWM_COUNT] = {
{ PIN_FREE, 0 },
{ PIN_FREE, 0 },
{ PIN_FREE, 0 }
};
static uint16_t pwmChannelSequence[PWM_COUNT];

struct PWMStatus pwmStatus[PWM_TIMER_COUNT] = {0, 0};

static int readResolution = 10;
static int writeResolution = 8;
Expand Down Expand Up @@ -220,9 +219,9 @@ void analogWrite( uint32_t ulPin, uint32_t ulValue )
ulPin = g_ADigitalPinMap[ulPin];

for (int i = 0; i < PWM_COUNT; i++) {
if (pwmChannelPins[i] == 0xFFFFFFFF || pwmChannelPins[i] == ulPin) {
pwmChannelPins[i] = ulPin;
pwmChannelSequence[i] = ulValue | bit(15);
if (pwmContext[i].pin == PIN_FREE || pwmContext[i].pin == ulPin) {
pwmContext[i].pin = ulPin;
pwmContext[i].value = ulValue | bit(15);

NRF_PWM_Type* pwm = pwms[i];

Expand All @@ -236,15 +235,25 @@ void analogWrite( uint32_t ulPin, uint32_t ulValue )
pwm->COUNTERTOP = (1 << writeResolution) - 1;
pwm->LOOP = 0;
pwm->DECODER = ((uint32_t)PWM_DECODER_LOAD_Common << PWM_DECODER_LOAD_Pos) | ((uint32_t)PWM_DECODER_MODE_RefreshCount << PWM_DECODER_MODE_Pos);
pwm->SEQ[0].PTR = (uint32_t)&pwmChannelSequence[i];
pwm->SEQ[0].PTR = (uint32_t)&(pwmContext[i].value);
pwm->SEQ[0].CNT = 1;
pwm->SEQ[0].REFRESH = 1;
pwm->SEQ[0].ENDDELAY = 0;
pwm->TASKS_SEQSTART[0] = 0x1UL;

break;
pwmStatus[0].numActive++;
return;
}
}

// fallback to digitalWrite if no available PWM channel
if (ulValue < 128)
{
digitalWrite(ulPin, LOW);
}
else
{
digitalWrite(ulPin, LOW);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same copy/paste error again

}
}

#ifdef __cplusplus
Expand Down
39 changes: 35 additions & 4 deletions cores/nRF5/wiring_digital.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
#include "nrf.h"

#include "Arduino.h"
#include "wiring_private.h"

extern struct PWMContext pwmContext[PWM_COUNT];
extern struct PWMStatus pwmStatus[PWM_TIMER_COUNT];
#ifdef NRF52
extern NRF_PWM_Type* pwms[PWM_COUNT];
#endif

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -86,18 +93,42 @@ void digitalWrite( uint32_t ulPin, uint32_t ulVal )

ulPin = g_ADigitalPinMap[ulPin];

for (uint8_t i = 0; i < PWM_COUNT; i++)
Copy link
Owner

Choose a reason for hiding this comment

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

What do you think about moving this to a private analogWriteOff (or something with a better name) function in cores/nRF5/wiring_analog_nRF5x.c?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather than creating another function I could change analogWrite so that it turns off PWM when a duty cycle of zero is passed?

Its up to what you prefer as you are the owner and this affects the API. Either implementation is easy to implement.

{
if (pwmContext[i].pin == ulPin)
{
pwmContext[i].pin = PIN_FREE;
#ifdef NRF52
// Disable the PWM
NRF_PWM_Type* pwm = pwms[i];
pwm->ENABLE = (PWM_ENABLE_ENABLE_Disabled << PWM_ENABLE_ENABLE_Pos);
#endif
pwmStatus[0].numActive--;
}
}

#ifdef NRF51
// Turn off the Timer if no pwm signals are allocated
for (uint8_t i = 0; i < PWM_TIMER_COUNT; i++)
{
if (pwmStatus[i].numActive == 0)
{
NVIC_ClearPendingIRQ(pwmStatus[i].irqNumber);
NVIC_DisableIRQ(pwmStatus[i].irqNumber);
}
}
#endif

switch ( ulVal )
{
case LOW:
NRF_GPIO->OUTCLR = (1UL << ulPin);
break ;
break;

default:
NRF_GPIO->OUTSET = (1UL << ulPin);
break ;
break;
}

return ;
}

int digitalRead( uint32_t ulPin )
Expand Down
19 changes: 18 additions & 1 deletion cores/nRF5/wiring_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,26 @@
extern "C" {
#endif


#include "wiring_constants.h"

#define PWM_COUNT 3
#define PWM_TIMER_COUNT 1 // 3 channels of TIMER1 are used. TIMER2 also could be used for PWM
#define PIN_FREE 0xffffffff

struct PWMContext {
uint32_t pin;
uint32_t value;
#ifdef NRF51
uint32_t channel;
uint32_t mask;
uint32_t event;
#endif
};

struct PWMStatus {
int8_t numActive;
int8_t irqNumber;
};

#ifdef __cplusplus
} // extern "C"
Expand Down