wrong error management in i2C functions - part 2

[camelator]

Well...
I don't know how the tests are done as master, but to me, it may work with fast MCU but not with CPU as I2S slave:

1/ To me, the HAL functions are not correct and for that STM32Duino must deliver / correct the bad error management from HAL.
Example in function HAL_StatusTypeDef HAL_I2C_Master_Seq_Receive_IT
This function has a bad mix of error management and state and return value.
Then, it is possible (and because of other errors in STM32duino) to have this function returning HAL_OK but reading nothing (for example, if the line is busy)
=> STM32Duino should not only manage the return value of the HAL functions but as well error codes and state values, for each call o HAL function, mainly in master mode.

2/ In master mode, The philosophy of the requestFrom is we have to send a request, wait the slave to be ready, and read the data and check errors during the process.
2a/ There is no error check after the TX in function requestFrom
2b/
There is no waiting code in STM32Duino resulting errors returned, or random results, or 0 data, or n random data.
There is something that look like waiting code in function i2c_master_read but it is not well managed as the errors, states and return values from HAL functions are too bad.
For example it is clearly possible that the RX become ready after a while but before the timeout. In this condition, the reading is not repeated resulting wrong results and random behaviors like this one:
https://community.st.com/s/question/0D53W00000EnOPYSA3/stm32l4-hali2cstatebusyrx-issue
but it is not for a short time, when it happens it is stay.

I am not sure part 1/ is related to master as the code on my slave can't see the errors on the line.

I am sure there are other mistakes as I can see sometime strange state values with strange logic state on SCL and SDA with a logic analyzer.

[camelator]

Discussion related to my comment:
My understanding:

• The Arduino class is Wire.h/.cpp
• The file twi.c contains main calls to HAL functions => This is the bridge between the Arduino class and the HAL functions.
• Twi is working in blocking mode for master and in non blocking mode for slave
• STM32Duino is not sticky to blocking mode or not but it should stick to the Arduino way
• It means: Wire.h/.cpp is not sticky to blocking mode or not but it should stick to the Arduino way, and not depend on how twi is working (hal bridge).

=> Based on my understanding (may be wrong), my opinion (may be wrong too):

twi.c is a bad word in this situation. Twi is a protocol very close to I2C protocol. word was defined for copyright reasons. But here the architecture of this file is not related to a dedicated protocol. To me, a better name should be hal_wire_bridge or something like that.

For simplicity reasons Twi should work in the same mode (non blocking) for both slave and master:

• it should manage errors related to I/O functions and hardware lines. Delay management, tempo, etc... Should not be managed here.
• Wire should manage errors related to messages,,delay, tempo, etc... and manage I/O errors coming from twi. Wire can/should work in blocking mode.

To me, the current situation (a mix between non blocking mode for slave, blocking mode for master in file twi) is driving complexity to maintain the code.

delay management in twi may be moved to wire for easy maintenance, and clear separation of tasks.

[ABOSTM]

@camelator,
Few remarks:

I don't know how the tests are done as master, but to me, it may work with fast MCU but not with CPU as I2S slave:
I don't catch the point, I guess you mean I2C slave instead of I2S slave (note that I2S bus also exist).
I don't understand why you oppose fast MCU to CPU: by CPU you mean host computer ? to me generally MCU are slower that CPU.
At the end I tested 2 STM32 one as master(Nucleo-STM32l476RG), one as slave (Nucleo-F103RB and Nucleo-G474RE), and it works fine.

1/ Aim of Stm32duino is not to fix HAL bad error management:
If there are bug in HAL, please report them to HAL official github like:
https://github.com/STMicroelectronics/STM32CubeF7
(by the way I don't know which board/MCU you are using)

Then, about function `HAL_I2C_Master_Seq_Receive_IT()

Then, it is possible (and because of other errors in STM32duino) to have this function returning HAL_OK but reading nothing

It looks normal to me, ..._IT means we use interrupt mode, so aim of this function is not to receive data, it will be handle by IRQ handler.

2a/
At the end, Arduino API: https://www.arduino.cc/reference/en/language/functions/communication/wire/requestfrom/
doesn't offer ability to return an error code to application. Everything is well working when expected read bytes correspond to effective read bytes, else there is probably an error (potentially within the application).

2b/

There is no waiting code in STM32Duino ....
For example it is clearly possible that the RX become ready after a while but before the timeout. 

Hardware clock stretching is there for that purpose: Slave keeps clock low until data is ready to be send. No need for extra software. If Slave doesn't use stretch capability, the master will not wait.
Note: stretch mode is configurable at begin():
void begin(uint8_t, bool generalCall = false, bool NoStretchMode = false);

For simplicity reasons Twi should work in the same mode (non blocking) for both slave and master:

I disagree: aim is not only simplicity, but also efficiency, and respect of Arduino API.
This is necessary for slave to work in non blocking, because it rely on master to decide when it will initiate a transfer.
And Arduino API onreceive() and onrequest() clearly works with callback and non blocking mode.
So, Wire cannot work in blocking for slave.
Whereas in master mode, user expect data to be effectively send when returning from endTransmission(). And it is mandatory as Arduino API expect to return the status of the transmission.

Please note that I convert this thread to discussion, as I see no clear failing use case.
But if you have one, please report it with details to reproduce.