Code critique -- feedback

Domanik

I'd like to post some C code on OBEX but first it needs another set of eyes. The I2C routine here dumps the eeprom contents of the MLX90614 IR temperature sensor and displays 32 values. Nothing fancy, a hundred lines of code. Your constructive feedback will help me write better code in the future and hopefully help others to get their projects going.
Thanks... Dom..:)

[php]/*
Simple I2C Project to learn I2C bus and IR Temp Sensor.
Read MLX eeprom contents in MLX90614 IR Temp Sensor into a
buffer, strBuf, and dump data to terminal. Once the contents
are in strBuf it can be written to Activity Board eeprom as
a backup. The backup code is not included here.
Can also retreive a 2 byte word from eeprom or RAM.
Uses busy_Wait with stop to check status and timeout.

*/

#include "simpletools.h"
#include "simplei2c.h"

i2c *imuBus;

char strIn[8], strBuf[1000];

int val_I2C;

int i2c_get_eeprom(i2c *busID, int add_SA);
int i2c_get_word(i2c *busID, int add_SA, int location, int memtype);
int dmp_eeI2C(void);
int i2c_busy_wait(i2c *busID, int add_SA, int hwait);

int main()
{
imuBus = i2c_newbus( 9, 10, 0); // P9=SCL, P10=SDA, Mode=0, I2C bus for MLX, Compass, GPS,
pause(300); // wait for terminal before using printf
i2c_get_eeprom(imuBus, 0x5A);
dmp_eeI2C();
}



/* read 32 eeprom locations (64 bytes) from MLX40914 into char array strBuf
*/
int i2c_get_eeprom(i2c *busID, int add_SA)
{
memset(strBuf, 0, 100);
int ackI2C = 0;
for (int loc = 0; loc<=31; loc++)
{
i2c_in(busID, add_SA, 0x20 + loc, 1, strIn, 3 ); // get 2 bytes at a time, ignore PEC
strBuf[loc*2] = strIn[0];
strBuf[loc*2+1] = strIn[1];
val_I2C = (strBuf[loc*2+1] << 8) + strBuf[loc*2];
// printf("eeLoc = %02X, val = %04X \n", loc, val_I2C);
}
return ackI2C;
}


int dmp_eeI2C(void)
{
print("\n");
for (int loc = 0; loc<=31; loc++)
{
val_I2C = (strBuf[loc*2+1] << 8) + strBuf[loc*2];
printf("eeLoc = %02X, val = %04X \n", loc, val_I2C);
}
return val_I2C;
}


/* return a single int value from the MLX eeprom or RAM:
loc is memory location as per datasheet, (note: config1 reg is 0x05 not 0x25)
memtype = 1 for eeprom and 0 for RAM
*/
int i2c_get_word(i2c *busID, int add_SA, int loc, int memtype)
{
memset(strIn, 0, 8);
if (memtype) loc += 0x20;
i2c_in(busID, add_SA, 0x20 + loc, 1, strIn, 3 ); // get 2 bytes at a time, ignore PEC
val_I2C = (strIn[1] << 8) + strIn[0];

return val_I2C;
}


/* i2c: wait for device to be available, timeout
* if status = 0 then return,
* if hwait = 0 then return bus status
* if hwait <> 0 then poll for ACK every ms. After timeout (hwait=0) return -1 error
*/
int i2c_busy_wait(i2c *busID, int add_SA, int hwait)
{
add_SA <<= 1;
add_SA &= -2;
int status = i2c_poll(busID, add_SA); i2c_stop(busID);
if(hwait > 2000) hwait=10;
if (hwait <= 0 || status==0) return status;
for (int n=hwait; n !=0 ; n--)
{
pause(1);
status = i2c_poll(busID, add_SA); i2c_stop(busID);
if (status==0) return status;
}
return -1;
}


[/php]

Andy Lindsay (Parallax)

Hi Domanik,

Nice work! It's great to see example code for that sensor.

If you post to obex in this form, I'd recommend setting the description to something along the lines of: "Application that demonstrates measuring temperature with the MLX90614 temperature sensor using a combination of simpletools and simplei2c library functions." Another option would be to try out the Library Studies guide, and then set up a package with a small main file that calls MLX90614 library functions. In that case, the description might be more along the lines of: "Bundle with MLX90614 example code and library."

As for coding suggestions, here are a few:

1) Most of the Simple Libraries use prototypes with deviceName_functionName. If applied to your app, it could be mlx90614_getEeprom, mlx90614_getWord, etc.

2) The Library Studies guide has a Document Your Simple Library page on setting up code comments for creating Doxygen auto-generated web documentation.

3) I usually use print instead of printf in code examples. With print, there is a fixed payload, with no program memory usage surprises. In contrast, programs with printf seem reasonable when only int values are used, but as soon as floating point variables are added, there is an unexpected program memory usage surprise that can be reduced with print. By only using print, it helps reduce the load on our Tech Support Staff. P.S. Print also has an automatic pause built-in for first run. So you will not need a pause before the first print.

Hope this helps...

Andy