-
Notifications
You must be signed in to change notification settings - Fork 955
Added 24LC32 EEPROM I2C example #709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
i2c/24lc32_i2c/24lc32_i2c_lib.h
Outdated
void read_eeprom(uint16_t address, uint8_t *result, uint16_t num, uint8_t i2c_addr); | ||
void byte_write_eeprom(uint16_t address, uint8_t data, uint8_t i2c_addr); | ||
bool page_write_eeprom(uint16_t address, uint8_t *data, uint8_t num, uint8_t i2c_addr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a gut-feeling, but I wonder if uint8_t i2c_addr
ought to be the first parameter? (Or the second parameter, if you also add i2c_inst_t *i2c_instance
?)
Probably worth checking with @peterharperuk what style he prefers.
uint8_t byte1 = (uint8_t)(address >> 8); | ||
uint8_t byte2 = (uint8_t)(address); | ||
|
||
uint8_t buf[2] = {byte1, byte2}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterharperuk Should these variables be declared outside of the for
loop, or does that not really matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it makes more sense as it is here?
Could you rename |
b7de1f4
to
021bbec
Compare
Eeek, looks like you've overwritten |
021bbec
to
9d44cad
Compare
a0147e0
to
7514659
Compare
7514659
to
335ef9c
Compare
335ef9c
to
7ee16d8
Compare
// This is the max number of bytes which can be written at once during a page write | ||
#define MAX_PAGE_WRITE 32 | ||
|
||
// This is the 24LC32's capacity in bytes | ||
#define EEPROM_SIZE 4096 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterharperuk Would it make sense for these to be in the .h
file rather than the .c
file? 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be a way to query the hardware for these values rather than hard code them. But maybe this is good enough for this example
Looks pretty good to me now 👍 |
@@ -159,6 +159,7 @@ App|Description | |||
[ht16k33_i2c](i2c/ht16k33_i2c) | Drive a 4 digit 14 segment LED with an HT16K33. | |||
[slave_mem_i2c](i2c/slave_mem_i2c) | i2c slave example where the slave implements a 256 byte memory. | |||
[slave_mem_i2c_burst](i2c/slave_mem_i2c) | i2c slave example where the slave implements a 256 byte memory. This version inefficiently writes each byte in a separate call to demonstrate read and write burst mode. | |||
[24lc32_i2c](i2c/24lc32_i2c) | Read and write data to 24LC32 EEPROM via I2C. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arguably missing an "a "
No description provided.