Target: STM i2c_byte_write returns value not consistent with documentation

[omdathetkan]

Description

    /** Write to an I2C slave
     *
     * Performs a complete write transaction. The bottom bit of
     * the address is forced to 0 to indicate a write.
     *
     *  @param address 8-bit I2C slave address [ addr | 0 ]
     *  @param data Pointer to the byte-array data to send
     *  @param length Number of bytes to send
     *  @param repeated Repeated start, true - do not send stop at end
     *         default value is false.
     *
     *  @returns
     *       0 on success (ack),
     *       nonzero on failure (nack)
     */
int write(int address, const char *data, int length, bool repeated = false);

int I2C::write(int data) {
    lock();
    int ret = i2c_byte_write(&_i2c, data);
    unlock();
    return ret;
}

Then in i2c_byte_write it always returns 1;

ACK should be checked and the result should be returned

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-775

[0xc0170]

@ARMmbed/team-st-mcd Please review

[LMESTM]

@jorisa
You're mentioning the function

int write(int data);

but you're pointing documentation of:

I2C::write(int data) (i.e; not int write(int address, const char *data, int length, bool repeated = false);

the actual documentation of int write(int data) is:

    /** Write single byte out on the I2C bus
     *
     *  @param data data to write out on bus
     *
     *  @returns
     *    '0' - NAK was received
     *    '1' - ACK was received,
     *    '2' - timeout
     */

And I think that STM implementation complies with this defintiion.
The i2c_byte_write will not always return 1, please have a look here:
https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/i2c_api.c#L682
https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/i2c_api.c#L543

It will return 1 in case of ACK as documented, or in case the ack is not received it will return 2 to report the timeout. I don't think we have an easy way to differentiate NACK from timeout in the write operation.

Let me know if I misunderstood something
Hope this helps
Laurent

[LMESTM]

@0xc0170 @jorisa
Please let me know if my explanation makes sense.
If no feedback or further explanation, I'd propose to close

[LMESTM]

@TacoGrandeTX @0xc0170 I think I addressed the comments. @jorisa hasn't answered back since my answer more than 1 month ago. Could you close it ?
ST_TO_BE_CLOSED