Skip to content
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

Update 0292-EEPROM-use.md #5

Closed
wants to merge 10 commits into from
Closed

Update 0292-EEPROM-use.md #5

wants to merge 10 commits into from

Conversation

CleaLCo
Copy link
Collaborator

@CleaLCo CleaLCo commented Nov 5, 2024

Proposal document for the use of EEPROM chip to store data in the PS HAT.

For PlanktoScope/PlanktoScope#292

Supercedes #3

Copy link
Member

@ethanjli ethanjli left a comment

Choose a reason for hiding this comment

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

Nice work! Here's my feedback.

design/00292: EEPROM storage.md Outdated Show resolved Hide resolved
design/00292: EEPROM storage.md Outdated Show resolved Hide resolved
design/00292: EEPROM storage.md Outdated Show resolved Hide resolved
design/00292: EEPROM storage.md Outdated Show resolved Hide resolved
design/00292: EEPROM storage.md Outdated Show resolved Hide resolved
## Compatibility

The main compatibility issue concerns the adafruit HAT users.
For those, a simple solution would be to continue selecting their configuration in the GUI, or adapt the script to store informations in the Raspberry pi EEPROM chip in case of not finding the PlanktoScope HAT EEPROM adress.
Copy link
Member

Choose a reason for hiding this comment

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

Based on https://www.raspberrypi.com/documentation/computers/raspberry-pi.html , I believe the Raspberry Pi's EEPROM chip may already be used/reserved by the Raspberry Pi as part of how it boots up. If this is a correct understanding of the Raspberry Pi, then we cannot use the Raspberry Pi's EEPROM chip to store our own data.

When there is no PlanktoScope HAT with EEPROM, the other alternative would be to use a JSON file on the SD card as a substitute for storing data on EEPROM. I recommend you to describe this option in this design document.

The main compatibility issue concerns the adafruit HAT users.
For those, a simple solution would be to continue selecting their configuration in the GUI, or adapt the script to store informations in the Raspberry pi EEPROM chip in case of not finding the PlanktoScope HAT EEPROM adress.
The PlanktoScope versions V2.5 and V2.6 do not natively support writing data to the EEPROM chip because the write-protect (WP) pin of the EEPROM is enabled. To disable write protection, a solder point must be applied to the WP pin.

Copy link
Member

Choose a reason for hiding this comment

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

This design document also needs to talk about data schema compatibility: if/when the metadata we store on the EEPROM changes (e.g. by adding or removing fields, or by adding new enum values for strings), how will we ensure backwards compatibility and/or forwards compatibility? I believe a common approach is to store a data schema version number as part of the data, as a way to detect when we might need to handle certain backwards/forwards-compatibility situations in certain ways.

Since there will be an MQTT API associated with this functionality, I also recommend addressing the question of API compatibility. For example, maybe this design document say the following things in the compatibility section:

  1. This proposal will not modify any existing MQTT API routes, but it will add some new MQTT API routes related to EEPROM functionality.
  2. All MQTT API routes added by this proposal are to be considered experimental, with no guarantees of compatibility in the future.

* LED reference

Those informations could be used to feed the metadata stored in the hardware.json file as described in the issue 290 (https://github.com/PlanktoScope/PlanktoScope/issues/290).

Copy link
Member

Choose a reason for hiding this comment

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

Since you are adding MQTT API routes, please also describe the design of the MQTT API routes you are adding.

design/00292: EEPROM storage.md Outdated Show resolved Hide resolved
design/00292: EEPROM storage.md Show resolved Hide resolved
@ethanjli ethanjli mentioned this pull request Nov 5, 2024
@CleaLCo CleaLCo closed this Nov 6, 2024
@ethanjli
Copy link
Member

ethanjli commented Nov 6, 2024

Note: this PR is superceded by #6

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.

2 participants