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

feat(hardware): Added a driver to read and write data from the Flex eeprom #12847

Merged
merged 18 commits into from
Jul 7, 2023

Conversation

vegano1
Copy link
Contributor

@vegano1 vegano1 commented Jun 2, 2023

Overview

We need a way to read/write to the eeprom on the carrier board, this python module will let us do that from userspace.

Closes: RET-1346

Property Format
Data is written to the eeprom in the form of serialized property byte string with the following format
prop_id(1b) + data_length(1b) + data(1-253b)

  • The prop_id is a 1-byte 1-0xFE value and denotes the unique property being stored: NOTE: 0xFF is invalid
  • The data_length is a 1-byte 1-0xFD value and denotes the length of the data
  • The data is a 1-253b long hexadecimal value representing some data that corresponds to a prop_id.

Properties
Properties represent some piece of data stored on the eeprom, and are categorized by a prop_id, a data_type, and a value. When we find a valid serialized property byte string, which means we have a bytestring matching our format denoted above where the length byte corresponds to the actual size of the data. We deserialize this bytestring based on the specific prop_id, the type of data that prop_id is associated with, and how long the data should be. We use the data_type to encode/decode the data accordingly and create a Property object with the parsed data type.

API
In order to interact with the eeprom we have the concept of properties, and we use the methods self.property_write and self.property_read to read and write these properties to the eeprom.

The self.property_write method takes a set of property and value tuples Set[Tuple[property_id, Any]]) which are serialized, chunked up, and written to the eeprom. This method returns a set of PropId for the properties that were successfully written to the eeprom.

The self.property_read method takes a set of PropId enums and returns a set of Property objects which represent the eeprom data.

Test Plan

  • Make sure we can read/write data to/from the eeprom on the flex

Changelog

  • Added eeprom module that lets us read/write data to/from any eeprom device in /sys/bus/i2c/devices

Review requests

  • Does this approach make sense?
  • Am I missing something?

Risk assessment

Low, flex work

@vegano1 vegano1 requested a review from sfoster1 June 2, 2023 22:03
@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Merging #12847 (c082230) into edge (cba2f3c) will increase coverage by 0.00%.
The diff coverage is 83.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #12847      +/-   ##
==========================================
  Coverage   72.34%   72.35%              
==========================================
  Files        1548     2348     +800     
  Lines       50597    64379   +13782     
  Branches     3235     7120    +3885     
==========================================
+ Hits        36606    46580    +9974     
- Misses      13513    16127    +2614     
- Partials      478     1672    +1194     
Flag Coverage Δ
g-code-testing 96.44% <ø> (ø)
hardware 57.62% <83.10%> (-1.56%) ⬇️
notify-server 89.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...entrons/hardware_control/backends/ot3controller.py 66.50% <ø> (ø)
...pentrons/hardware_control/backends/ot3simulator.py 87.30% <ø> (ø)
...ardware/opentrons_hardware/drivers/eeprom/build.py 0.00% <0.00%> (ø)
...rdware/opentrons_hardware/drivers/gpio/__init__.py 46.77% <37.50%> (-1.38%) ⬇️
hardware/opentrons_hardware/drivers/__init__.py 66.66% <66.66%> (ø)
...ardware/opentrons_hardware/drivers/eeprom/utils.py 85.13% <85.13%> (ø)
...rdware/opentrons_hardware/drivers/eeprom/eeprom.py 92.00% <92.00%> (ø)
...ware/opentrons_hardware/drivers/eeprom/__init__.py 100.00% <100.00%> (ø)
...ardware/opentrons_hardware/drivers/eeprom/types.py 100.00% <100.00%> (ø)

... and 880 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

I think if we're doing TLV, we should do TLV all the way. We can drop the start and end tokens, because we have the type, length and value. Require the type to not be 255 and you can't be confused with erased data; then, you can read out size-1 bytes, nice and easy.

PropType.SHORT: 2,
PropType.INT: 4,
PropType.STR: 254,
PropType.BIN: 254,
Copy link
Member

Choose a reason for hiding this comment

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

same?

vegano1 added 11 commits June 12, 2023 14:29
…line SODIMM_222 low when writting to the eeprom
deal with overflowing incomplete data
cleaning up
…h for testing

- removed hard while loops when parsing data
- return set of PropertyId of the properties that were successfully written to the eeprom
- dropped the manufacturing_facility and unique_id values from the serial number
- removed unused PropertyId’s MACHINE_TYPE, MACHINE_VERSION, PROGRAMMED_DATE, UNIT_NUMBER
- added MAX_DATA_LEN so we can limit the size of a property to 255 bytes
- removed eeprom_writter.py script as this will be added in a separate pr
- added unit tests
…em_hardware to return one item

added defines to drivers.__init__
use gpiod passed down from ot3Controller when initializing the EEPROMDriver
return the EEPROMDriver module from the OT3Controller via a property
fix the eeprom_build_driver module
@vegano1 vegano1 marked this pull request as ready for review July 3, 2023 21:17
@vegano1 vegano1 requested a review from a team as a code owner July 3, 2023 21:17
@vegano1 vegano1 requested a review from sfoster1 July 3, 2023 21:17
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

hardware/opentrons_hardware/drivers/eeprom/eeprom.py Outdated Show resolved Hide resolved
@vegano1 vegano1 changed the title Ret 1346 add eeprom module feat(hardware): Added a driver to read and write data from the Flex eeprom Jul 7, 2023
@vegano1 vegano1 merged commit 056bdcf into edge Jul 7, 2023
@vegano1 vegano1 deleted the RET-1346_add_eeprom_module branch July 7, 2023 15:39
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