wrong error management in i2C functions?

[camelator]

I have an issue I don't know where it comes from... After some seconds, the I2C line remains high... but calling I2C functions always return good results.

1/ During debugging my problem I can see that the i2c_master_read function returns I2C_OK although the I2C line is crashed:
if the result of the first receive call is not HAL_OK, it returns I2C_OK because there is no else block attached to the first 'if' , instead of returning an error code.

2/ and it is the same issue within the function i2c_master_write:
it returns I2C_OK although the result of the first call to Transmit_IT is not HAL_OK

I am not sure but I think the impact of the change on the read function is neutral but some other changes should be done in the requestFrom function:

3/ if the i2c_master_write function returns an error code, it is well managed in the endTransmission function but not in the request_from function.
having endTransmission(false) without error management means you can try to read data even the write is not done.

i2c_status_e i2c_master_read(i2c_t *obj, uint8_t dev_address, uint8_t *data, uint16_t size)
{
  i2c_status_e ret = I2C_OK;
  uint32_t tickstart = HAL_GetTick();
  uint32_t delta = 0;
  uint32_t err = 0;

#if defined(I2C_OTHER_FRAME)
  uint32_t XferOptions = obj->handle.XferOptions; // save XferOptions value, because handle can be modified by HAL, which cause issue in case of NACK from slave
#endif

#if defined(I2C_OTHER_FRAME)
  if (HAL_I2C_Master_Seq_Receive_IT(&(obj->handle), dev_address, data, size, XferOptions) == HAL_OK) {
#else
  if (HAL_I2C_Master_Receive_IT(&(obj->handle), dev_address, data, size) == HAL_OK) {
#endif
    // wait for transfer completion
    while ((HAL_I2C_GetState(&(obj->handle)) != HAL_I2C_STATE_READY) && (delta < I2C_TIMEOUT_TICK)) {
      delta = (HAL_GetTick() - tickstart);
      if (HAL_I2C_GetError(&(obj->handle)) != HAL_I2C_ERROR_NONE) {
        break;
      }
    }

    err = HAL_I2C_GetError(&(obj->handle));
    if ((delta >= I2C_TIMEOUT_TICK)
        || ((err & HAL_I2C_ERROR_TIMEOUT) == HAL_I2C_ERROR_TIMEOUT)) {
      ret = I2C_TIMEOUT;
    } else {
      if ((err & HAL_I2C_ERROR_AF) == HAL_I2C_ERROR_AF) {
        ret = I2C_NACK_DATA;
      } else if (err != HAL_I2C_ERROR_NONE) {
        ret = I2C_ERROR;
      }
    }
  }
  return ret;
}

[fpistm]

Hi @camelator

I've made a fix. Could you try it with your use case?

Thanks

[camelator]

I can't see your changes. Where did you put it? Which branch?

[ABOSTM]

Fix is available in a Github Pull Request:
#1775

[fpistm]

And it is referenced by the issue 😉

[camelator]

Sorry for the delay, but to me the fix is nott complete:
as I wrote, the function "request_from" should be changed as well, as it can read data although wrong data sent. So the data may be inconsistent.