Hi, I just wrote a basic driver for the MMA8452 accelerometer (see http://mbed.org/users/ashleymills/code/MMA8452/file/7620a11149b8/MMA8452.cpp, and
A test program for the MMA8452 accelerometer
While I was using the mbed I2C interface a couple of things occurred to me that I would like to share:
Inconsistent return values from functions
The long versions of read and write return 0 on success and 1 on failure. But the single byte write returns 1 on success and 0 on failure. So the functions are inconsistent. This is probably difficult to change if people already built libraries based on this, but I thought I'd mention it anyway.
Inconvenient way to write multiple bytes to a register or vwrite style chains
If you look at the function definition for write:
int write(int address, const char * data, int length, bool repeated = false)
The way this is designed assumes you will prepend the register address to data.
Normally however, if you are writing a library for an I2C device, the *device* address above will be fixed, whereas the register address will change often. So it would be good if a convenience function existed for this. For example, here is the one I used in my library:
int MMA8452::writeRegister(char addr, char *data, int nbytes) {
// writing multiple bytes is a little bit annoying because
// the I2C library doesn't support sending the address separately
// so we just do it manually
// 1. tell I2C bus to start transaction
_i2c.start();
// 2. tell slave we want to write (slave address & write flag)
if(_i2c.write(_writeAddress)!=1) {
return 1;
}
// 3. send the write address
if(_i2c.write(addr)!=1) {
return 1;
}
// 4. send the data to write
for(int i=0; i<nbytes; i++) {
if(_i2c.write(data[i])!=1) {
return 1;
}
}
// 5. tell I2C bus to end transaction
_i2c.stop();
return 0;
}
You will notice that I can't use two calls to the multi-byte write function in my convenience function, because the multi-byte write pre-pends it's own START condition on the I2C bus.
If you added a flag that allowed supression of the START condition (like you do for the END condition), then it would be much cleaner to push multiple data buffers, since the calls could be chained. For example, the function could look like this:
int write(int address, const char * data, int length, bool noStart = false, bool noStop=false)
An alternative would be to have an additional function like this:
int write(int deviceAddress, int registerAddress, const char * data, int length, bool repeated = false)
And abstract like I have above. But the supression of START would still be handy for writev style usage which is pretty common in programming. So I think it would make sense to have the following two functions:
int write(int deviceAddress, const char * data, int length, bool noStart = false, bool noStop=false)
int write(int deviceAddress, int registerAddress, const char * data, int length, bool noStart = false, bool noStop=false)
Then this would cover most use cases for I2C.
Slightly odd typing
address is always an int in your functions, but an I2C address is a maximum of 10 bits in extended addressing mode, but it is never signed.
I don't understand why you would use a signed integer for an unsigned value. It would make more sense IMO for it to be a uint16_t since that's what is actually being used. Note that you'd have to be careful in any register writing function to get the endianess correct for the 10 bit addresses.
Also, why is the data buffer a char buffer?
char in c is actually an integer type, and it can be signed in some implementations. Again this makes no sense for raw data for which a sign is meaningless. I think a more sensible type would be uint8_t. But perhaps you did it like this for compatibility with string manipulation functions, IDK.
BR
Ashley
Hi, I just wrote a basic driver for the MMA8452 accelerometer (see http://mbed.org/users/ashleymills/code/MMA8452/file/7620a11149b8/MMA8452.cpp, and
Import programMMA8452_Test
A test program for the MMA8452 accelerometer
Last commit 07 Mar 2014 by Ashley Mills
While I was using the mbed I2C interface a couple of things occurred to me that I would like to share:
Inconsistent return values from functions
The long versions of read and write return 0 on success and 1 on failure. But the single byte write returns 1 on success and 0 on failure. So the functions are inconsistent. This is probably difficult to change if people already built libraries based on this, but I thought I'd mention it anyway.
Inconvenient way to write multiple bytes to a register or vwrite style chains
If you look at the function definition for write:
The way this is designed assumes you will prepend the register address to data.
Normally however, if you are writing a library for an I2C device, the *device* address above will be fixed, whereas the register address will change often. So it would be good if a convenience function existed for this. For example, here is the one I used in my library:
You will notice that I can't use two calls to the multi-byte write function in my convenience function, because the multi-byte write pre-pends it's own START condition on the I2C bus.
If you added a flag that allowed supression of the START condition (like you do for the END condition), then it would be much cleaner to push multiple data buffers, since the calls could be chained. For example, the function could look like this:
An alternative would be to have an additional function like this:
And abstract like I have above. But the supression of START would still be handy for writev style usage which is pretty common in programming. So I think it would make sense to have the following two functions:
Then this would cover most use cases for I2C.
Slightly odd typing
address is always an int in your functions, but an I2C address is a maximum of 10 bits in extended addressing mode, but it is never signed.
I don't understand why you would use a signed integer for an unsigned value. It would make more sense IMO for it to be a uint16_t since that's what is actually being used. Note that you'd have to be careful in any register writing function to get the endianess correct for the 10 bit addresses.
Also, why is the data buffer a char buffer?
char in c is actually an integer type, and it can be signed in some implementations. Again this makes no sense for raw data for which a sign is meaningless. I think a more sensible type would be uint8_t. But perhaps you did it like this for compatibility with string manipulation functions, IDK.
BR
Ashley