AVR ICU module calling width_cb instead of period_cb

ChibiOS public support forum for topics related to the Atmel AVR family of micro-controllers.

Moderators: utzig, tfAteba

orion
Posts: 15
Joined: Sun Nov 12, 2017 7:56 pm
Has thanked: 4 times
Been thanked: 2 times

AVR ICU module calling width_cb instead of period_cb

Postby orion » Sun Nov 12, 2017 8:32 pm

Hi there,

I'm reading the source code for the icu module of the avr port (Chibios 17.6.3) but I don't think I get how the function handle_capture_isr() (line 93 of file hal_icu_lld.c) works. On page 133 of the ATMega328p datasheet, it says:
When the ICES1 bit is written to zero, a falling (negative) edge is used as trigger, and when the ICES1 bit is written to one, a rising (positive) edge will trigger the capture.

From this I understand that line 99 of hal_icu_lld.c,

Code: Select all

uint8_t rising = (*tccrb & (1 << ICES1)) ? 1 : 0;

sets the variable rising to 1 if a rising edge occurs in pin ICP1, or to 0 if a falling edge occurs. Hence, the following if on lines 101-105,

Code: Select all

  if ((icup->config->mode == ICU_INPUT_ACTIVE_HIGH && rising) ||
      (icup->config->mode == ICU_INPUT_ACTIVE_LOW  && !rising)) {
   icup->width = value;
   if (icup->config->width_cb != NULL)
     icup->config->width_cb(icup);

calls the width_cb callback function if a rising (activation) edge occurs and the icu module is configured to active high.

However, the Chibios documentation for the ICU module shows a state diagram that indicates that in this situation the period_cb callback should be called instead.

Am I misunderstanding something here?

Thanks.

User avatar
tfAteba
Posts: 511
Joined: Fri Oct 16, 2015 11:03 pm
Location: Chartres, France
Has thanked: 75 times
Been thanked: 44 times

Re: AVR ICU module calling width_cb instead of period_cb

Postby tfAteba » Tue Nov 14, 2017 9:27 am

Hi Orion,

Sorry I did not have the time this week, to look at your question.

I have never use this driver so I will take a look to it and also to the documentation.

Sorry to be unable to help you right now.
regards,

Theo.

orion
Posts: 15
Joined: Sun Nov 12, 2017 7:56 pm
Has thanked: 4 times
Been thanked: 2 times

Re: AVR ICU module calling width_cb instead of period_cb

Postby orion » Sat Nov 18, 2017 2:21 pm

Hello Theo,

Thanks for your reply. I've written a little program to demonstrate the issue I'm talking about. I've tested it in an Arduino Nano and a Uno. The program basically sets up the ICU module to call two callback functions, one for the width notification and the other for the period notification. The first simply turns off the led on the board, and the other turns it on. Hence, I'd expect the led to blink according to the duty cycle of the square wave on pin ICP1 (the capture pin). I then use the ATMega328p feature of triggering the ICU interrupt if the ICP1 pin is configured as output and is toggled. In the main function there is an infinite loop setting first ICP1 to 1 for 0.1 s and then to 0 for 0.9 s, hence generating a 1 Hz square wave on ICP1 with a 10% duty cycle. The led, however, is on much longer than it is off, and I assume its duty cycle is 90%. This would be caused by the current code in the AVR port of the ICU module calling the width callback instead of the period callback and vice versa.

The code is the following.

Code: Select all

#include <ch.h>
#include <hal.h>

void width_cb(ICUDriver *icup) {
    palClearPad(IOPORT2, PORTB_LED1);
}

void period_cb(ICUDriver *icup) {
    palSetPad(IOPORT2, PORTB_LED1);
}

int main(void) {
    static ICUConfig icucfg = {ICU_INPUT_ACTIVE_HIGH, 0, width_cb, period_cb, 0};

    halInit();
    chSysInit();
    palSetPadMode(IOPORT2, 0, PAL_MODE_OUTPUT_PUSHPULL);
    icuStart(&ICUD1, &icucfg);
    icuStartCapture(&ICUD1);

    while (true) {
        /* Set the ICP1 pin for 0.1s, then turn it off for 0.9s */
        palSetPad(IOPORT2, 0);
        chThdSleepMilliseconds(100);
        palClearPad(IOPORT2, 0);
        chThdSleepMilliseconds(900);
    }

    return 0;
}

User avatar
tfAteba
Posts: 511
Joined: Fri Oct 16, 2015 11:03 pm
Location: Chartres, France
Has thanked: 75 times
Been thanked: 44 times

Re: AVR ICU module calling width_cb instead of period_cb

Postby tfAteba » Sun Nov 19, 2017 11:41 am

Hi Orion,

Could you please tell me what exactly is the problem that you are facing in simple words?

Thanks for your application example.

There is a demo showing how the module works under testhal/AVR/ICU did you take a look at this example? It can help you to understand how the ICU works.

I'm going to look at you example and make some measurement to see if any thing is strange.
I haven't use this module before so I have also to understand how it works :D
regards,

Theo.

orion
Posts: 15
Joined: Sun Nov 12, 2017 7:56 pm
Has thanked: 4 times
Been thanked: 2 times

Re: AVR ICU module calling width_cb instead of period_cb

Postby orion » Sun Nov 19, 2017 6:43 pm

Hello Theo,

Thanks for pointing me to the icu test program. I'm still working through the code tree.

I took a look at the file main.c in the directory /home/hcabral/Workspaces/RTOS/ChibiOS_17.6.3/testhal/AVR/ICU. The ICU3 driver is configured as ICU_INPUT_ACTIVE_HIGH in line 93, and hence, as I understand, the variable width refers to the time duration the input signal is high.

On the other hand, the output_single_cycle() function signature is in line 49:

Code: Select all

void output_single_cycle(const uint16_t low, const uint16_t high)
From this we can see that the first parameter is the duration in which the generated square wave is low, and the second is the duration it is high.

Further on, in lines 123-125, we have

Code: Select all

    chprintf(serp, "Testing 25 duty cycle\r\n");
    icuEnable(&ICUD3);
    output_single_cycle(250, 750);

It says it is testing a 25% duty cycle square wave but from the above we can see it is actually generating a 75% duty cycle wave! This inversion undoes the inversion in the ICU module.

It just proves what I initially said: the AVR port of the ICU module is calling width_cb() when it should be calling period_cb(), and vice-versa. That is, width_cb() is being called on the rising edge and period_cb on the falling edge of the input signal, the opposite of what it should be for the active high mode. The end result is that width is being calculated as the period value minus the real width value, or, in other words, the time duration where the signal is low.

I don't have access right now to an arduino so I can't test, but I'd bet the led would be on 75% of the time.

Thanks for your time looking into this.

User avatar
tfAteba
Posts: 511
Joined: Fri Oct 16, 2015 11:03 pm
Location: Chartres, France
Has thanked: 75 times
Been thanked: 44 times

Re: AVR ICU module calling width_cb instead of period_cb

Postby tfAteba » Sun Nov 19, 2017 9:09 pm

Hi Orion,

Thanks to you to clarify what you found, I will look at this carefully and make some test.

But I have encounter some compilations problem for the arduino board. So I will try to resolve this other wise I will make test with the arduino mega board to quickly make the verification of your affirmation.

For the moment thanks for your feedback.
regards,

Theo.

orion
Posts: 15
Joined: Sun Nov 12, 2017 7:56 pm
Has thanked: 4 times
Been thanked: 2 times

Re: AVR ICU module calling width_cb instead of period_cb

Postby orion » Mon Nov 20, 2017 10:51 am

Hi Theo,

Thanks for your reply. I think you can use the arduino mega to verify this issue. The description of the ICESn bit in the control register TCCRnB on page 156 of the ATmega1280 datasheet is identical to the description of the same bit on page 173 of the ATMega328p datasheet. This means the problem in the function handle_capture_isr() in the hal_icu_lld.c file which I described in the beginning of this thread should also happen for the mega.

Please also check the issue on the duty cycle in the icu test program you pointed me to just in case I'm being blindly stupid.

Thanks.


Return to “AVR Support”

Who is online

Users browsing this forum: No registered users and 1 guest