[embedded] Serial EEPROM IC driver feedback

help
1

Hi,
I have written an embedded-hal-based driver for serial EEPROM ICs of the 24x series (I2C).
You can find the crate here.

It would be nice if some of you could provide feedback, with special attention to this:

  1. Project / crate name: [SOLVED] The project is now called eeprom24x.
Summary

I wrote it for an AT24C256 but it is compatible with AT24C32...AT24C512 although it could be trivially extended for other ICs and hence the at24cx name.
However, the naming has changed: Microchip (formerly Atmel) 24xxxx.
There are also several other vendors that have their own versions that seem compatible and they appear to be named "24": e.g. OnSemi CAT24xxx, ST M24xxx.
Do you have any suggestions for a better crate name? eeprom-24x? serial-eeprom-24x? 24x-eeprom? 24x-serial-eeprom? 24x-eeprom-driver? 24x-driver?
24x seems too short.

  1. Interface: driver interface
    1. Alternative addresses: The lowest 3 bits of the 7-bit device address can be configured for these ICs. The Eeprom24x constructor expects an u8 slave address parameter. The configurable address bits can be configured through: SlaveAddr::Alternative(true, false, true) for A2, A1, A0.
      1. [SOLVED] device address as u8 parameter but actually I2C device addresses are 7-bit. Still, 7 boolean parameters would be worse.
      2. What do you think about specifying an alternative address like this? (syntax, bools for all configurable address bits,...)
    2. Support for several ICs
      1. Duplicated code: Depending on the memory size, the page size changes per IC. When writing a (variable size) page to the memory, I need to prepend the target address to the data. I solved this by creating a new array with a size of PAGE_SIZE+2 and then copying the data, as the array size must be known at compile time.
        This leads to repeated implementations of the write_page() method.
        This code and documentation duplication can only get worse as I add support for more ICs.
        As suggested below, this could be solved using a trait with an associated constant but this is blocked by this upstream issue at the moment.
        Do you have any better idea?
      2. Constructors: [SOLVED] There are methods like Eeprom24x::new_24x32(...) and Eeprom24x::new_24x256(...)
Summary

I created an implementation for a generic At24cx for each IC and provide new methods which contain the IC name. e.g. At24cx::new_at24c32(...) At24cx::new_at24c256(...).
This will need to change together with the project name.
What do you think of methods like At24cx::new_32kbit(...), At24cx::new_256kbit(...),...?

  1. General feedback
  2. Further improvements

I understand that this is a big post containing lots of requests but some discussion would be very interesting for me and also for other device drivers I may write in the future. I can also split this topic into separate ones if you want.
I look forward to your feedback.

2 Likes
2

pub fn new_at24c128(i2c: I2C, address: u8) -> Self {

Instead of taking an address as second argument, why don't you use your SlaveAddr enum? Typesafety ftw! Maybe adding an (unsafe?) ctor for arbitrary addresses?

#[derive(Debug)]
pub enum Error {
I2c(E),
[...]
}

Write that c big. I know it will issue a warning, but you can (in this case I think it is totaly legit) supress it with #[allow(non_camel_case_types)] I2C,.

pub mod ic;

I'm not sure, why you would define your structs in a seperate mod. Are they that different? If I see it correctly, they only differ in the PAGE_SIZE. You could let them impl a trait with an associated constant and then use that, instead of repeating yourself in code over and over again.

trait foo {
    const PAGE_SIZE: u8;
}
2 Likes
3

Thank you very much for your feedback.
I implemented your suggestion about the SlaveAddr parameter and the I2C casing.
Using an associated constant seems like the perfect solution for the duplicated write_page() implementations but at the moment I cannot use it because of this upstream issue. I will implement this once that is solved.

Thanks again!

1 Like
4

So, after mulling this over for a bit I think the creation of read/write functions that allow for sending and receiving an arbitrary number of buffers per message in the embedded-hal API is going to help the community here.

The only reason for the associated constant in this project is because a buffer needs to be created to send two buffers (one that’s constant, one that’s variable).

I have the same issue in my crates. I’ll need to find an issue for it and reference this problem in it.

Boop: https://github.com/rust-embedded/embedded-hal/issues/94

1 Like