ARMmbed / mbed-hal-st-stm32f4

mbed HAL for ST STM32F4-series microcontrollers
Other
7 stars 21 forks source link

gpio_irq edge bug #28

Closed antevir closed 8 years ago

antevir commented 8 years ago

Let's say I have the following code:

    gpio_irq_init(&irqGpio, pin, myIsrHandler, 1 /* Do not use 0 */);
    gpio_irq_set(&irqGpio, IRQ_FALL, 1);

If, for instance, the pin is connected to a GPIO with poor debounce or to a chip with a fast IRQ pulse the myIsrHandler() may be called with event=IRQ_RISE even though I have only registered a falling edge IRQ.

The problem is that handle_interrupt_in() in gpio_irq_api.c reads out the current state of the GPIO in order to determine if it was a rising or falling edge. However, since the isr rutine takes some time to execute the GPIO might have already changed state.

I realize that there is no easy fix since the MCU only have one pending interrupt flag per GPIO, hence not indicating if its rising/falling. But in the current state the mbed driver InterruptIn class will result in a NULL pointer call when this happens.

betzw commented 8 years ago

Isn't a glitch a HW bug?

antevir commented 8 years ago

OK.. I have renamed the issue.. Maybe we should focus on the problem instead?

0xc0170 commented 8 years ago

I realize that there is no easy fix since the MCU only have one pending interrupt flag per GPIO, hence not indicating if its rising/falling. But in the current state the mbed driver InterruptIn class will result in a NULL pointer call when this happens.

If you registered that event, it will be called. The gpio handler in the hal checks the current pin state, reports it to the higher level, which should handle this (reference: https://github.com/ARMmbed/mbed-drivers/blob/master/source/InterruptIn.cpp#L60). The NULL pointer call should not happen. IF this is not the case, we should fix it

The HAL currently reports the measured event. Are you considering filtering it in the hal layer, thus in your described case, IRQ_NONE would be reported as IRQ_RISE was not registered and FALL was read.

betzw commented 8 years ago

Is your irq interrupt-level or -edge sensitive?

antevir commented 8 years ago

Here is how the issue is triggered at the mbed driver API level. I have this code:

InterruptIn irqTest(BUTTON);

void irqHandler() {
}

void app_start(int, char**) {
    irqTest.fall(irqHandler);
}

I press the button connected to irqTest a few times, and a callback that is set to NULL is called. As you can se here the rise callback is called even though I have not called irqTest.rise(). image

The interrupt is edge sensitive

0xc0170 commented 8 years ago

I'll have a look. The FunctionPointer was changed, I recall call method included a check if there was a registered a callback, I filed an issue for that : https://github.com/ARMmbed/mbed-drivers/issues/83

To fix this, InterruptIn could check if event was registered. @antevir you can send fix

curiosity, what IDE are you using for debugging ?

betzw commented 8 years ago

When I stumbled over this issue I had for some time a solution like this in mind. But I than deprecated the idea in favour of a workaround in the affected expansion board SW.

Obviously, it would be much better if such a check would be done in the mbed code in InterruptIn! @0xc0170, how good are the chances for getting this check done in InterruptIn?

0xc0170 commented 8 years ago

@antevir Please can you test this patch: https://github.com/ARMmbed/mbed-drivers/compare/master...0xc0170:fix_interruptin

antevir commented 8 years ago

@0xc0170 I am using VisualGDB with Visual Studio. The patch you sent should do the trick - I will try it when I am back in office tomorrow. My only concern is that filtering out the event might result in no callback at all for fast pulses. But I guess that you can always register both rising and falling edge in this case.

antevir commented 8 years ago

@0xc0170 I tried the patch and it solves the problem!

0xc0170 commented 8 years ago

@antevir I sent 2 pull requests, one documentation (id !=0) and one for fixing InterruptIn bug with registered events.

How did you set up that visual studio with yotta? Using cmake to export to visual studio?

antevir commented 8 years ago

Thanks for the help @0xc0170!

I basically just call yotta to build the binary, then I have more or less manually added the include paths to get IntelliSense to work: image I am sure this can be done in a better way with a proper Makefile, but I just needed to get it up and running.

0xc0170 commented 8 years ago

@antevir How are you exporting to visual studio? I can't find the -G option to VS with yotta. You can find my email in the git log in this repo, got few questions about the setup you are using :-) I can't find yours. Thanks