Skip to content

Conversation

Louis31423142
Copy link
Collaborator

No description provided.

Comment on lines 6 to 8
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);
Copy link
Contributor

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.

Comment on lines +25 to +36
uint8_t byte1 = (uint8_t)(address >> 8);
uint8_t byte2 = (uint8_t)(address);

uint8_t buf[2] = {byte1, byte2};
Copy link
Contributor

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?

Copy link
Contributor

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?

@lurch
Copy link
Contributor

lurch commented Sep 8, 2025

Could you rename READ_ME.aoc to README.adoc please?

@lurch
Copy link
Contributor

lurch commented Sep 8, 2025

Eeek, looks like you've overwritten 24lc32_i2c_lib.c with the contents of example.c ?!

@Louis31423142 Louis31423142 force-pushed the 24lc32 branch 3 times, most recently from a0147e0 to 7514659 Compare September 8, 2025 15:09
Comment on lines +5 to +9
// 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
Copy link
Contributor

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? 🤷‍♂️

Copy link
Contributor

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

@lurch
Copy link
Contributor

lurch commented Sep 9, 2025

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arguably missing an "a "

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants