No Wire.endTransmission() after a Wire.requestFrom() please.

[Koepel]

In the file "MPR121.cpp" in the function getRegister(), there is a Wire.endTransmission() directly after a Wire.requestFrom().

The Wire.endTransmission() should only be used when writing data and only in combination with Wire.beginTransmission() and almost always with Wire.write().

To test if valid data was received, the return value could be used, as is done in the other functions. However the function getRegister() can not return a 'false' like the other function.

This is not okay:

    Wire.requestFrom(address,(unsigned char)1);  // just a single byte
    if(Wire.endTransmission()!=0){
    	error |= 1<<ADDRESS_UNKNOWN_BIT;
    } else {
    	error &= ~(1<<ADDRESS_UNKNOWN_BIT);
    }
    scratch = Wire.read();

Perhaps this is better:

    if(Wire.requestFrom(address,(unsigned char)1) == 1){  // just a single byte
      error &= ~(1<<ADDRESS_UNKNOWN_BIT);
      scratch = Wire.read();
    } else {
      error |= 1<<ADDRESS_UNKNOWN_BIT;
    }

But I'm not sure if that is how you want to deal with an error.

[stefandz]

Thanks for raising this issue. FYI, this code has been out there and running in the wild for 3 years so quite amazed we're only seeing this now! I think the original logic for this was to create a stop condition - but reading the docs it's clear that it's redundant (and not the intended use of the function).

It's also unlikely to actually get a real error here as ADDRESS_UNKNOWN errors will flag up during MPR121::begin() and get caught there (triggered by the first MPR121::setRegister() call) - but we'll likely leave it for completeness.

We'll take a proper look at this soon - maybe this week but more likely early next year.

[Koepel]

Well, it is not really redundant. In my opinion it is wrong.
It can not be used to create a stop condition, because that is not what Wire.endTransmission() is doing. You should rely on the Wire.requestFrom() for the complete I2C transaction.

The Wire.beginTransmission() sets a few variables.
The Wire.write() fills a buffer with data.
The Wire.endTransmission() issues a start, transmits the data and issues a stop.

What the Wire.endTransmission() will do on its own is undocumented and uncertain. That depends on the implementation of that specific Wire library.

For the AVR Wire library, a Wire.endTransmission on its own does no harm. It issues a start with the previous used address, the slave acknowledges and a stop is issued. It only takes time, and it is not needed. However, that is only for the AVR Wire library and only at this moment.