Usage of Wire library.

[Koepel]

I have read your notes, but if you want to make the code better, there are some issues with the usage of the Wire library.

There is no need to wait after a Wire.requestFrom().
A Wire.requestFrom() should not be followed by a Wire.endTransmission().
Explanation: Common-mistakes, number 1 and 2.
See also my explanation of the functions.

When reading 32 bits, you do a Wire.request() for only 2 bytes.

Reading 16 bits could be like this:

// Read 2 byte from the FDC at 'address'
uint16_t FDC2214::read16FDC(uint16_t address) {
    uint16_t data = 0;     // default zero

    Wire.beginTransmission(_i2caddr);
    Wire.write(address);
    Wire.endTransmission(false); //restart

    Wire.requestFrom(_i2caddr, (uint8_t) 2);
    if (Wire.available() == 2) {
        data = Wire.read();
        data <<= 8;
        data |= Wire.read();
    }
    return data;
}

Reading 32 bits could be like this:

// Read 4 bytes from the FDC at 'address'
uint32_t FDC2214::read32FDC(uint16_t address) {
    uint32_t retVal = 0;

    Wire.beginTransmission(_i2caddr);
    Wire.write(address);
    Wire.endTransmission(false);

    Wire.requestFrom(_i2caddr, (uint8_t) 4);   // read 4 bytes
    if (Wire.available() == 4) {
        retVal |= (uint32_t) Wire.read() << 24; 
        retVal |= (uint32_t) Wire.read() << 16;
        retVal |= (uint32_t) Wire.read() << 8;
        retVal |= (uint32_t) Wire.read();
    }
    return retVal;
}

The extra variable "data" is not needed. If there are four bytes available, the Wire.read() returns a signed integer with a value of 0 to 255. It is never -1 when only those four (available) bytes are read.

[zharijs]

Hi.
In case anyone wants to do so - feel free to optimize it and I can merge if your branch will work.

As it works and I am not personally using either Wire or Arduino anyway (I rather use my own low level libs on STM32 with STDPeriphLib) I don't think I will have much time for optimization.

[rharrison]

koepel you have identified a couple of problems here well done. I'm going to make the changes and do a pull request.

[rharrison]

Whilst looking at the code:

uint32_t FDC2214::read32FDC(uint16_t address) {
    ...
    Wire.beginTransmission(_i2caddr);
    Wire.write(address);
    Wire.endTransmission(false);
    ...

I see the address in held in a 2 byte integer uint16_t
As I understand, Wire.write only writes one byte.

Should the code not be

    ...
    Wire.beginTransmission(_i2caddr);
    Wire.write(address >> 8);
    Wire.write(address);
    Wire.endTransmission(false);
    ...

[Koepel]

Could you talk about the "i2c address" or "register address" ?
According to the datasheet: "This sequence follows the standard I2C 7-bit slave address followed by an 8-bit pointer register byte to set the register address".
The "register address" is 8 bits (the registers are located from 0x00 to 0x7F).
The "register data width" is 16 bits.

It would be easier if the library uses uint8_t for the "register address".

Beside the problems with waiting after a Wire.requestFrom(), and a Wire.endTransmission() after a Wire.requestFrom(), I also noticed these problems:
The read8FDC() function writes 16 bits of the "register address", I think that is not okay.
The read32FDC() function requests 2 bytes and tries to read 4 bytes.
The write8FDC() function also tries to write 16 bits of the "register address".

[zharijs]

Cool, Thanks @rharrison. I'll verify that it still works on a devkit and update.
Sorry for being so slow, much work backlogged.