General Improvements

[AEFeinstein]

Initialize variables to 0 to fix warnings from -Wuninitialized. I don't know if the variables were initialized within the assembly, but GCC didn't know either, and was throwing a bunch of warnings. I figured 0 is a safe value to start with.

Made data parameter to brzo_i2c_write() const. This data should not be modified by the function, so const seems appropriate.

Add brzo_i2c_get_error() function. I used this to debug errors from my main program.

My project uses ESP NON-OS SDK 3.0.1, and brzo_i2c works fine, so I added a note to README.md

[pasko-zh]

Many thanks! I will have a look at it ASAP!

[AEFeinstein]

Also, I don't think you keep track of working I2C devices, but I've been doing my testing with an SSD1306 128x64 display and MMA8452Q accelerometer.

[pasko-zh]

Thanks again. I do not track all compatible devices explicitly (because basically my library is supposed to work with all standard i2c devices), it is the other way round: I.e., if some devices should not be compatible, I will list it them, e.g., the AM2320 (see here with reference to this).

[pasko-zh]

I've accepted the initialization of variables. But did not go for the const, I prefer to use it only very very seldom. The debugging function of the returning the error code, I didn't move it into the library, since every read or write returns the error anyway. If there will be more votes to include it again, I will follow them 😄

[AEFeinstein]

Thanks for accepting initialization. I understand not accepting const, that's a personal preference and it's your project 😃

I'm still not sure about the error code. Only brzo_i2c_end_transaction() returns i2c_error, even though it seems like it can be set in the following functions which return void.

void brzo_i2c_write(const uint8_t* data, uint32_t no_of_bytes, bool repeated_start);
void brzo_i2c_read(uint8_t* data, uint32_t nr_of_bytes, bool repeated_start);
void brzo_i2c_ACK_polling(uint16_t ACK_polling_time_out_usec);

Do you think users would want to detect errors earlier than brzo_i2c_end_transaction(), for instance if there are a large number of reads or writes combined into one transaction, and the first read/write fails?

[pasko-zh]

Well, the idea is that you group any number of "unseful" i2c commands (i.e. reads/writes) together into a (brzo i2c) transaction. And on the level of such a transaction you can then tell wether it has failed or succeeded by evaluating the error code returned by brzo_i2c_end_transaction. An i2c transaction only succeeds if all i2c commands were executed withour errors. And thus, handling errors should be done outside of an i2c transaction.

However, I agree, for debugging a long transaction with many reads/writes, it could be useful to have the error code already inside the transaction. Because otherwise you might not know wich of the commands actually failed.