Inconsistent implementation of I2C::write

[matthewelse]

Description

• Type: Bug
• Related issues: I2C byte write return value #1670, i2c - fix return value description i2c_byte_write() #1694
• Priority: Minor

Currently, the I2C class is implemented such that I2C::write returns different values to indicate ACK in different overloads. This seems counter-intuitive and should probably be made consistent. In #1670, the documentation was corrected to reflect the documentation, however nothing was done about the implementation.

cc: @0xc0170

[ciarmcom]

ARM Internal Ref: IOTMORF-477

[0xc0170]

Thanks @matthewelse for reporting.

I made first corrections in the doc to map what is should be :

diff --git a/drivers/I2C.h b/drivers/I2C.h
index da09d94..84f1d89 100644
--- a/drivers/I2C.h
+++ b/drivers/I2C.h
@@ -117,8 +117,8 @@ public:
      *  @param repeated Repeated start, true - do not send stop at end
      *
      *  @returns
-     *       0 on success (ack),
-     *   non-0 on failure (nack)
+     *      0 or non-zero - written number of bytes,
+     *      negative - I2C_ERROR_XXX status
      */
     int write(int address, const char *data, int length, bool repeated = false);

@@ -127,8 +127,9 @@ public:
      *  @param data data to write out on bus
      *
      *  @returns
-     *    '1' if an ACK was received,
-     *    '0' otherwise
+     *    '0' - NAK was received
+     *    '1' - ACK was received,
+     *    '2' - timeout
      */
     int write(int data);

diff --git a/hal/i2c_api.h b/hal/i2c_api.h
index 677596d..41aef38 100644
--- a/hal/i2c_api.h
+++ b/hal/i2c_api.h
@@ -117,7 +117,9 @@ int i2c_read(i2c_t *obj, int address, char *data, int length, int stop);
  *  @param data    The buffer for sending
  *  @param length  Number of bytes to write
  *  @param stop    Stop to be generated after the transfer is done
- *  @return Number of written bytes
+ *  @return
+ *      zero or non-zero - Number of written bytes
+ *      negative - I2C_ERROR_XXX status
  */
 int i2c_write(i2c_t *obj, int address, const char *data, int length, int stop);

The further would be to align write() methods (one does 0,1,2 statuses, the other one bytes written or failures (negative values). As this issue states, we should focus on targets and fix them.

I'll send PR with this patch, tag there maintainers for review, we can write test to see which ones fail and start patching.