Arduino_core_stm32: [I2C] Review I2C SLAVE IT Rx/Tx complete callback

[fpistm]

Refers to #216
Since #135 by @fprwi6labs (I2C IT mode)
HAL_I2C_SlaveRxCpltCallback and HAL_I2C_SlaveTxCpltCallback should be used instead of HAL_I2C_ListenCpltCallback

Several issue raised around I2C and related to IT mode implementation:
http://stm32duino.com/viewtopic.php?f=29&t=3210&start=10
http://stm32duino.com/viewtopic.php?f=48&t=3612

~http://stm32duino.com/viewtopic.php?f=48&t=3243&p=41486#p41486~ Addressed by #301

~Will also include #203~ Addressed by #305

[MauroMombelli]

Is there any ETA? this to me seems a pretty critical issue, i have been extremely lucky to read this issue before starting to use this core.
In my opinion the TwoWIre should be roll back pre-issue

[Testato]

I2C is one of the fundamental libraries for an Arduino core.
It can not remain damaged for so long.
If there are no resources to dedicate, at least go back to the previous version that was less faulty than this.

[fpistm]

I know this is a drawback issue. In several case I2C is working but some use case are broken (mainly master/slave use case).
I've planned to fix this issue in September. I've several stuff to perform before and it's hard to mitigate btw all those stuff. I try to do my best to answer/improve/fix/review/support all inputs.

This is an open source project, any contribution are welcome and really appreciated.

IMHO, rollback is not an option as it will break other use case.
If you want you can do it in your own.

[MauroMombelli]

In several case I2C is working but some use case are broken

"some user case" is a problem, I would like to start a project with F411 as master, how can i know if i will be affected? Is there any pre-analisis or patch working on or it is still all in the "todo" list?

IMHO, rollback is not an option as it will break other use case.
If you want you can do it in your own.

guess now is too late, is there any particular release known as working?

This is an open source project, any contribution are welcome and really appreciated.

Im implementing a new board and started using this core to create a variant; but i2c is a main core component, and sincerely as this project is version 1.3 and officially backed by ST i was expecting a stable product.
This will basically force me to use some alternative implementation, and im lucky i catch it early and if this is the case i will have "only" a couple of days of precious free time to throw away.

[Testato]

sincerely as this project is version 1.3 and officially backed by ST i was expecting a stable product.

+1

[fpistm]

@MauroMombelli, @Testato
Up to you to use another implementation. As I said open source project not ST dedicated project but supported by ST. I'm currently deploying CI (build and test) to avoid this kind of regression and provide a "_stable_" product.
Hope you will find one.
Sorry, for making you throw away your _precious_ free time.
Just a quick note: at this time, I'm working on my _precious_ free time...

Please, use the forum for this kind of discussion which do not help to solve this issue.

[MauroMombelli]

not ST dedicated project but supported by ST

By given their patron on a project, at my eyes they make this core "official" and they are responsible at the status of the project itself and its well being.

Just a quick not at this time, I'm working on my precious free time

your github account say you are part of STMicroelectronics and list a @st.com
Is just logical to assume this not your "free time"; but this is not even the point.
Simply having a release version with a core functionality not working properly and not signaled as beta/unstable, or literally 10 minute to add a warning in the readme, is quite frustrating for me and all the people trying to use this core.

Please, use the forum for this kind of discussion which do not help to solve this issue.

will do, thanks for listening my rants

[fpistm]

https://github.com/stm32duino/Arduino_Core_STM32/releases/tag/1.3.0

known issues

I2C: Several issue raised around I2C and related to IT mode implementation, see #217
[STLink] Failed to flash Nucleo L496ZG(-P) under Linux and Mac, see stm32duino/Arduino_Tools#23

[LMESTM]

@MauroMombelli, @Testato @fpistm
From the list of know limitations in the initial description of this issue, I understand that :

• I2C master use cases are mostly OK - we're using many I2C peripherals today without issue (temperature & humidity sensors, ...). 1 possible limitation in master mode is the I2C scanner functionality for some families (F4 being one).
• Implementing an I2C slave seems more problematic - there is at least 2 listed issues here, so slave mode is definitely an area of improvement.
  hope this helps
  Laurent

[LMESTM]

@MauroMombelli, @Testato @fpistm
I have proposed a PR #301 to address the scan limitation.

[LMESTM]

I think that with #301 and #305 the reported open issues on master use cases as covered.
So I updated this issue title to reflect that what's left is I2C SLAVE use case which is the one fully working under interrupt.

[LMESTM]

Hey I2C lovers, @MauroMombelli, @Testato @fpistm

I've just pushed a propose PR #306 that I've tested ok with master / slave examples.

Though there may be use cases that aren't properly covered yet, so I'd be glad if any of you could run some more tests with other boards or other examples to check for possible remaining issues.

Note that I used a X-NUCLEO-IKS01A1 to have proper pull-up resistors on the I2C lines.

[Squonk42]

I tested this PR#306 on the Bluepill STM32F103C8T6 and it fixes the bug:
https://github.com/stm32duino/Arduino_Core_STM32/pull/306#pullrequestreview-147803892

Thanks!

[fpistm]

Fixed with #306