mattairtech / EEPROM_CAT25

Driver for On Semiconductor CAT25 SPI EEPROM chips for AVR, SAM3X (Due), and SAM M0+ (SAMD, SAML, SAMC) microcontrollers on the Arduino platform.
6 stars 4 forks source link

Could easily support other chips as well #4

Open matthijskooijman opened 4 years ago

matthijskooijman commented 4 years ago

This library is named EEPROM_CAT25, indicating support for the CAT25xxx series of chips, but those chips implement a pinout and protocol that is standardized across a broader range of chips and manufacturers (commonly referred to as "Standard SPI EEPROM" or "Standard Serial EEPROM").

I've been using this library (with minor changes) succesfully using an ST M95640 chip.

Currently, the library is configured by passing the chip model number using CAT25xxx constants. From that model number, the capacity and page size is derived and stored and most of the code functions based on these values.

I initially tried treating my M95640 as a CAT25640, which has the same size, but then discovered that the page size is different, so apparently that part is not standardized. This leads me to believe that it might be more flexible to configure devices by passing a capacity and a page size, rather than (or in addition to) passing a model number. Locally, I've modified the library to accept a capacity and pagesize, which makes it work with my M95640 chip.

In addition to determining the quantity and pagesize, the library also uses the chip size to determine the exact protocol to use. In particular (and this is the only exception to the standard protocol), for the CAT25040, the protocol is slightly changed to store A8 in the command byte rather than in a second address byte. However, looking at the datasheet for the similar ST M95040 or the Microchip AT25040B, I see that the same protocol exception is made for these devices. In general, it seems that this is simply the protocol used for all 4Kbit devices, so you could simply look at the capacity to determine whether or not to put this extra address bit inside the command byte.

In summary, this needs that to configure the library, you indeed only need to pass the capacity and pagesize.

However, I believe it might still make sense to use presets for specific chips, which makes the library easier to use (and also keeps compatibility with existing sketches). So, I would suggest something like this:

struct EEPROM_Settings {
    size_t capacity;
    size_t page_size;
};

EEPROM_Settings CAT25640 = {
    .capacity = 0x2000,
    .page_size = 64,
}

These could be defined in the library .h file, and then passed to a modified constructor:

 EEPROM_CAT25(SPIClass * spi, const uint8_t chipSelect, const EEPROM_settings settings) {
   _capacity = settings.capacity;
   _pageSize = settings.page_size;
   ...
}

This would keep existing code working but also make it very easy for sketches to define their own chips if the library does not offer the one they need. Additionally, this mechanism could be extended later using additional fields in the structs, which would default to 0 if not specified.

How does something like this look to you? We will be needing this for our project in any case, so we will be building something like this in the coming days. If you're interested, we're happy to contribute the changes back (as well as changes for some of the other issues I've been created). Let me know what you think :-)

matthijskooijman commented 4 years ago

Additionally, this mechanism could be extended later using additional fields in the structs, which would default to 0 if not specified.

This approach is a bit more limited than I had hoped. It works, but missing fields raise a warning (if those are enabled), so this works fine for adding new fields that are expected to be added, but does not work so well for optional fields that are expected to be omitted.

mattairtech commented 4 years ago

It would be great if you could create the pull request (I can add documentation after). Otherwise, I can make the changes myself, but not until I have more time.

matthijskooijman commented 4 years ago

It would be great if you could create the pull request (I can add documentation after). Otherwise, I can make the changes myself, but not until I have more time.

Ok, willdo. I already did most of the work (for this and the other issues I reported) in the past days, see this branch for a preview:

https://github.com/3devo/EEPROM_CAT25/tree/improvements

I haven't been able to test most of that yet (no hardware at home), and still need to add support for protection bits, but I expect to be able to do this when I'm in the office tomorrow and submit a PR then.