-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
There was a problem hiding this 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
## 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. |
There was a problem hiding this comment.
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. | ||
|
There was a problem hiding this comment.
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:
- This proposal will not modify any existing MQTT API routes, but it will add some new MQTT API routes related to EEPROM functionality.
- 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). | ||
|
There was a problem hiding this comment.
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.
Co-authored-by: Ethan Li <[email protected]>
Co-authored-by: Ethan Li <[email protected]>
Co-authored-by: Ethan Li <[email protected]>
Co-authored-by: Ethan Li <[email protected]>
Co-authored-by: Ethan Li <[email protected]>
Co-authored-by: Ethan Li <[email protected]>
Co-authored-by: Ethan Li <[email protected]>
Note: this PR is superceded by #6 |
Proposal document for the use of EEPROM chip to store data in the PS HAT.
For PlanktoScope/PlanktoScope#292
Supercedes #3