Page 1 of 1
AVR ICU module calling width_cb instead of period_cb
Posted: Sun Nov 12, 2017 8:32 pm
by orion
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.
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Tue Nov 14, 2017 9:27 am
by tfAteba
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.
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Sat Nov 18, 2017 2:21 pm
by orion
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;
}
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Sun Nov 19, 2017 11:41 am
by tfAteba
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

Re: AVR ICU module calling width_cb instead of period_cb
Posted: Sun Nov 19, 2017 6:43 pm
by orion
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.
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Sun Nov 19, 2017 9:09 pm
by tfAteba
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.
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Mon Nov 20, 2017 10:51 am
by orion
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.
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Fri Oct 26, 2018 11:01 am
by orion
Hi Theo,
Is there any news on this issue on the AVR port of the ICU module?
Thanks.
Re: AVR ICU module calling width_cb instead of period_cb
Posted: Mon Nov 05, 2018 1:34 pm
by tfAteba
Hello Orion,
Sorry I have been busy for a while now,
- Lost my father
- Mariage
- Vaccation
I hope I will have more time to work on ChibiOS again.