Page 1 of 1

[TODO] ICU not working in 2.6.3 without period CB

Posted: Wed Mar 05, 2014 11:07 pm
by MarkusS
Hi,

I have problems with 2.6.3 using the ICU wihtout a period CB (on a STM32F4Discovery board). If I define a dummy period CB, 2.6.3 works as 2.6.2.Without the periodCB, the widthCB does not get called. Here is a modified PWM_ICU example showing the problem:

Code: Select all

#include "ch.h"
#include "hal.h"

static void pwmpcb(PWMDriver *pwmp)
{
  (void)pwmp;
  palClearPad(GPIOD, GPIOD_LED5);
}

static void pwmc1cb(PWMDriver *pwmp)
{
  (void)pwmp;
  palSetPad(GPIOD, GPIOD_LED5);
}

static PWMConfig pwmcfg = {
  10000,                                    /* 10kHz PWM clock frequency.   */
  10000,                                    /* Initial PWM period 1S.       */
  pwmpcb,
  {
   {PWM_OUTPUT_ACTIVE_HIGH, pwmc1cb},
   {PWM_OUTPUT_DISABLED, NULL},
   {PWM_OUTPUT_DISABLED, NULL},
   {PWM_OUTPUT_DISABLED, NULL}
  },
  0,
  0
};

icucnt_t last_width, last_period;

static void icuwidthcb(ICUDriver *icup)
{
  palTogglePad(GPIOD, GPIOD_LED4);
  last_width = icuGetWidth(icup);
}

static void icuperiodcb(ICUDriver *icup)
{
  last_period = icuGetPeriod(icup);
}

static ICUConfig icucfg = {
  ICU_INPUT_ACTIVE_HIGH,
  10000,                                    /* 10kHz ICU clock frequency.   */
  icuwidthcb,
  NULL, //icuperiodcb, // without the period cb, the widht cb does NOT get called in 2.6.3???
  NULL,
  ICU_CHANNEL_1,
  0
};

/*
 * Application entry point.
 */
int main(void)
{
  /*
   * System initializations.
   * - HAL initialization, this also initializes the configured device drivers
   *   and performs the board-specific initializations.
   * - Kernel initialization, the main() function becomes a thread and the
   *   RTOS is active.
   */
  halInit();
  chSysInit();

  /*
   * Initializes the PWM driver 2 and ICU driver 3.
   * GPIOA15 is the PWM output.
   * GPIOC6 is the ICU input.
   * The two pins have to be externally connected together.
   */
  pwmStart(&PWMD2, &pwmcfg);
  palSetPadMode(GPIOA, 15, PAL_MODE_ALTERNATE(1));
  icuStart(&ICUD3, &icucfg);
  palSetPadMode(GPIOC, 6, PAL_MODE_ALTERNATE(2));
  icuEnable(&ICUD3);
  chThdSleepMilliseconds(2000);

  /*
   * Starts the PWM channel 0 using 75% duty cycle.
   */
  pwmEnableChannel(&PWMD2, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD2, 7500));
  chThdSleepMilliseconds(5000);

  /*
   * Changes the PWM channel 0 to 50% duty cycle.
   */
  pwmEnableChannel(&PWMD2, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD2, 5000));
  chThdSleepMilliseconds(5000);

  /*
   * Changes the PWM channel 0 to 25% duty cycle.
   */
  pwmEnableChannel(&PWMD2, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD2, 2500));
  chThdSleepMilliseconds(5000);

  /*
   * Changes PWM period to half second the duty cycle becomes 50%
   * implicitly.
   */
  pwmChangePeriod(&PWMD2, 5000);
  chThdSleepMilliseconds(5000);

  /*
   * Disables channel 0 and stops the drivers.
   */
  pwmDisableChannel(&PWMD2, 0);
  pwmStop(&PWMD2);
  icuDisable(&ICUD3);
  icuStop(&ICUD3);
  palClearPad(GPIOD, GPIOD_LED4);
  palClearPad(GPIOD, GPIOD_LED5);

  /*
   * Normal main() thread activity, in this demo it does nothing.
   */
  while (TRUE) {
    chThdSleepMilliseconds(500);
  }
  return 0;
}

Re: ICU not working in 2.6.3 without period CB

Posted: Thu Mar 06, 2014 9:47 am
by Giovanni
Hi,

The callbacks should not be NULL in the ICU driver because there is no check on the pointer value before calling them. Are you sure this used to work?

Giovanni

Re: ICU not working in 2.6.3 without period CB

Posted: Fri Mar 07, 2014 3:47 pm
by MarkusS
Yes, it is working with NULL in 2.6.1 and 2.6.2 (at least for me on an STM32F4 and an STM32F1...). And the overflow cb is NULL in the example as well, so how should one know that the other cb's can't be NULL? If it is that way, perhaps we should add an assert there so we can catch it?

Re: ICU not working in 2.6.3 without period CB

Posted: Fri Mar 07, 2014 5:21 pm
by Giovanni
Hi,

I found the problem, it is a side effect of the last bug fix performed on that driver (for a valid reason). The driver now checks the state machine before invoking the period callback in order to suppress width callbacks before a period callback happens.

The problem is that if some callback is NULL then the state machine does not "move" between the ICU_ACTIVE and ICU_IDLE states because the related interrupts are disabled, see icu_lld_enable().

Now, is it acceptable to have a driver that does not update its state machine as is supposed to do? not enabling the interrupts does exactly this. On the other hand forcing the interrupts would limit the capture performance at high frequencies.

I have to rethink this, probably the best approach would be to not track ICU_IDLE and ICU_ACTIVE states anymore and detect those conditions using the timer status registers somehow and introducing a function like icuGetCaptureState() or similar.

Giovanni

Re: [TODO] ICU not working in 2.6.3 without period CB

Posted: Tue Apr 14, 2015 12:56 pm
by russian
Looks like I am experiencing the same issue now. I've disabled period callbacks for performance reasons and suddenly I am not getting width callbacks.

Re: [TODO] ICU not working in 2.6.3 without period CB

Posted: Tue Apr 14, 2015 1:20 pm
by Giovanni
Hi,

The driver has been reworked in 3.0.0, not it is possible to enable/disable callbacks (and their IRQ) at runtime. Back-porting it to 2.6.x is not planned but you could try that yourself or use 3.0.0.

Giovanni

Re: [TODO] ICU not working in 2.6.3 without period CB

Posted: Tue Apr 14, 2015 1:24 pm
by russian
Giovanni wrote:not it is possible to enable/disable callbacks (and their IRQ) at runtime.

I am not disabling anything at runtime: I've changed the code, re-compiled the code, flashed the new binary without period callback and lost width callback

Re: [TODO] ICU not working in 2.6.3 without period CB

Posted: Tue Apr 14, 2015 2:10 pm
by Giovanni
I wrote "not" instead of "now"...

now it is possible to enable/disable callbacks (and their IRQ) at runtime.


The driver is now able to do that.

Giovanni