ARMmbed / mbed-hal-nrf51822-mcu

mbed HAL port for nRF51822
9 stars 12 forks source link

PwmOut never sleeps #47

Open Timmmm opened 8 years ago

Timmmm commented 8 years ago

If I have a PwmOut object and set it to output to 0, I'd expect it to turn off the PWM system. Otherwise there is no way to wake up, output some PWM, then go to sleep again.

Actually however it seems there is no code at all to turn off PWM. Once you use it once, you are committed, and it draws about 1 mA which makes it useless for battery operated devices.

ciarmcom commented 8 years ago

ARM Internal Ref: IOTSFW-2006

0xc0170 commented 8 years ago

nrf51 hal provides _free function: https://github.com/ARMmbed/mbed-hal-nrf51822-mcu/blob/master/source/pwmout_api.c#L242

However, there's no destructor for PWMOut in mbed-drivers, there's one pull request opened recently: https://github.com/ARMmbed/mbed-drivers/pull/147

Timmmm commented 8 years ago

The _free() function doesn't actually stop the timer though. The easiest way I can think to do that is like this (not tested!):

void TIMER2_IRQHandler(void)
{
    NRF_TIMER2->EVENTS_COMPARE[3] = 0;
    NRF_TIMER2->CC[3]             =  PERIOD;

    if (PWM_taken[0]) {
        NRF_TIMER2->CC[0] = PULSE_WIDTH[0];
    }
    if (PWM_taken[1]) {
        NRF_TIMER2->CC[1] = PULSE_WIDTH[1];
    }
    if (PWM_taken[2]) {
        NRF_TIMER2->CC[2] = PULSE_WIDTH[2];
    }

    if (PWM_taken[0] || PWM_taken[1] || PWM_taken[2]) // Added this line.
        NRF_TIMER2->TASKS_START = 1;
}

That should in theory stop the timer if all the PwmOuts are freed. I don't know if the PPI or GPIOTE need to be reconfigured to sleep properly, but I'd assume not.

Then all that is required is a way to call _free(). It should be fine (and desirable) to put that in the destructor of PwmOut. In fact I'm suprised it isn't already. This should definitely be done.

However, I think my original code should still work - PwmOut::write(0.0f) should set the pin to always off and disable the timer. In the current code it actually does neither - it always sets the pin on for at least 4 us, and write(1.0f) always sets it off for at least 4 us. The documentation suggests this is wrong - write(0.0f) should mean 0% on, and write(1.0f) should mean 100% on. I think that's what people would expect to (I obviously did):

/** Set the ouput duty-cycle, specified as a percentage (float)
 *
 *  @param value A floating-point value representing the output duty-cycle,
 *    specified as a percentage. The value should lie between
 *    0.0f (representing on 0%) and 1.0f (representing on 100%).
 *    Values outside this range will be saturated to 0.0f or 1.0f.
 */
void write(float value) {
    pwmout_write(&_pwm, value);
}

In the nRF51 code:

if (PULSE_WIDTH[obj->pwm] == 0) {
    PULSE_WIDTH[obj->pwm] = 1; // <----- No!
    setModulation(obj, 0, 0);
} else if (PULSE_WIDTH[obj->pwm] == PERIOD) {
    PULSE_WIDTH[obj->pwm] = PERIOD - 1; // <------ Oh the humanity!
    setModulation(obj, 0, 1);
} else if ((oldPulseWidth == 0) || (oldPulseWidth == PERIOD)) {
    setModulation(obj, 1, oldPulseWidth == PERIOD);
}
shirish47 commented 7 years ago

does anyone got solution to this? I also face same problem with nrf51822 using PwmOut for buzzer and it does not let the code sleep. I use pwm only for buzzer when I press a button.

nvlsianpu commented 7 years ago

@anangl could you look at this?