Review interrupt in STM32F4 i2c_api

[arnaudrichard]

About: targets/hal/TARGET_STM/TARGET_STM32F4/i2c_api.c

In function serial_irq_set(), the tx interrupt enabled and disabled are not the same:

line 764: HAL_UART_ENABLE_IT(handle, UART_IT_TC);

mismatch with

line 768: HAL_UART_DISABLE_IT(handle, UART_IT_TXE);

My guess is that the same interrupt should be used for enable and disable cases.

Now when trying to understand which one is good, the confusion between UART_IT_TC and UART_IT_TXE seems broader:
•In function serial_clear(), UART_FLAG_TXE is cleared (and not UART_FLAG_TC)
•In function uart_irq(), UART_FLAG_TC is tested (and not UART_FLAG_TXE)

I stopped here investigations and leave it to your comments

[0xc0170]

Isn't this for serial_api ?

@bcostm @adustm

[bcostm]

Yes it is for serial.
Anyway, did you see a problem with the actual code ? What is the behavior you're observing ?

[arnaudrichard]

Yes it is serial_api.c sorry for wrong path.

When building code on top of that using interrupts, the interrupt handler is sometimes interrupted during its execution which was not expected and thus broke everything.
More specifically, when using Sam Grove's BufferedSerial
https://developer.mbed.org/users/sam_grove/code/BufferedSerial/

The function void BufferedSerial::txIrq(void) is interrupted while it is running when serial transmission finished whereas it was explicitely disabled with "RawSerial::attach(NULL, RawSerial::TxIrq);" .

[bcostm]

Ok thanks. We'll look at it but if you have already tried to change the UART FLAGs and if it works don't hesitate to send a pull request.

[arnaudrichard]

Yes I did fix our issue by selecting arbitrarily UART_IT_TC for both enabling and disabling.
However, I didn't modified serial_clear() because I don't have an overall vision of the file.

Could you please explain what you mean by sending a pull request?
You may contact me directly with ST mail.

[0xc0170]

Could you please explain what you mean by sending a pull request?

Clone this repository (fork suggested), and edit the files, commit + push, and send the pull request here. Or you can share diff here, and @bcostm can quickly test + do it

[arnaudrichard]

I did a pull request "Fix IRQ enabled in serial_irq_set() #1603"
It's my first pull request; hopefully I did it right...