RobTillaart/I2C_EEPROM

should the library control the Write Protect pin?

RobTillaart opened this issue · 5 comments

It would add five (simple) functions to the class

  • void setWriteProtectPin(uint8_t pin) set pinMode(_wpPin, OUTPUT) + defaults enable write ?
  • uint8_t getWriteProtectPin() returns _wpPin
  • uint8_t enableWrite() digitalWrite(_wpPin, LOW);
  • uint8_t disableWrite() digitalWrite(_wpPin, HIGH);
  • uint8_t isEnabledWrite() return (digitalRead(_wpPin) == LOW);

These 5 functions can be kept disjunct from the existing class. so if not used no code increment.
If merged into e.g. writeBlock() - checking isEnabledWrite() - it would always add to the footprint.

integration could also be in begin(), default pin -1?
(constructor would interfere with default Wire parameter)

low prio


As many devices have a WriteProtect or enable pin this could be a minimal class.
Other classes could inherit this WriteProtect class.
As naming + function and polarity etc differ it looks like overkill to "classify" this idea.

Note the writeProtectPin could also be controlled by a 595 shift register or a PCF8574.
Note: most EEPROMs are used as always writeable (assumption) so no control needed.

https://github.com/RobTillaart/AnalogUVSensor

Has functions to control the power pin, is a bit similar.

  • void setPowerPin(uint8_t powerPin, bool invert = false)
  • void switchOff()
  • void switchOn()

If the pin is not set there is a possible ambiguity. One does not know if the WP is GND or VCC.
So best strategy is to assume write enabled and let the hardware decide.

uint8_t isEnabledWrite()
{
  if (_wpPin == -1) return true;    //   in fact unknown
  return (digitalRead(_wpPin) == LOW);
}

test needed if footprint increases

  • not used not integrated
  • not used integrated

test code

  //  WRITE PROTECT PIN experimental
  void    setWriteProtectPin(uint8_t pin) { _wpPin = pin; };
  uint8_t getWriteProtectPin() { return _wpPin; };
  void    enableWrite()  { digitalWrite(_wpPin, LOW); };
  void    disableWrite() { digitalWrite(_wpPin, HIGH); };
  bool    isEnabledWrite() 
  {
    // if (_wpPin == 255) return true;
    return (digitalRead(_wpPin) == LOW); 
  };

  uint8_t _wpPin = 255;

observations

  • not using the code adds 1 byte RAM for _wpPin + 4 bytes for the assignment = 255;
  • call added ee.isEnabledWrite() adds a 100 - 120 bytes footprint. (depending on adding the test for _wpPin == -1 )
  • ee.isEnabledWrite() does not sound good in code => ee.isWriteEnabled()
  • it would be great if EEPROMS would have a register to see the status of the chips own WP pin

Conclusions

The addition of the write protect pin logic would add a performance gain in the write and update functions as it can be easily checked if the EEPROM is write protected.

However the logic to add is very simple, one might ask if it should be in the library when a user can easily add it.
Mind you, it would add to the footprint of the library also for all the users that would not use it.
Also it expands the width of the interface.

soft write enabled ?

An even simpler way might be a software only write enable flag.

  //  SOFT WRITE PROTECT PIN experimental
  void    enableWrite()  { _writeEnabled = true; };
  void    disableWrite() { _writeEnabled = false; };
  bool    isWriteEnabled() { return _writeEnabled; };

  bool    _writeEnabled = true;

This only works if integrated in the write /set / update functions, and it would add only very small tests.
Functionally it would prevent writing although not backed by hardware.

The question is if the default should be true or false?
For now the answer is true as that will not break the current behavior.

Functions that need test

  • writeByte()
  • writeBlock()
  • setBlock()
  • updateByte()
  • updateBlock()

update verify functions would follow naturally.

Question is what they should return, there is no consistency in the interfaces.
=> error flag

  • does a readEnable flag makes sense?

test software write disable

  • adds 42 bytes on an UNO to test the 5 above functions.
  • substantial less than the hardware controlled

conclusion (for now)

The functionality to prevent writing by implementing a write protect mechanism will not be added.
The same argument holds here:
.., the logic to add is very simple, one might ask if it should be in the library when a user can easily add it.