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 script write and interact with the Flex eeprom. #13054

Merged
merged 24 commits into from
Jul 7, 2023

Conversation

vegano1
Copy link
Contributor

@vegano1 vegano1 commented Jul 6, 2023

Overview

We need to store the serial information for the Flex on the eeprom, this script will be used to write and verify that value along with any other data we might want to write to the eeprom.

Closes: RET-1347 & RET-1349

Test Plan

  • Write the serial number a Flex and verify that the value is stored after reboot

Changelog

  • added eeprom_writter.py script to read/write/print eeprom data on the Flex

Review requests

  • usage should be straightforward but let me know if there's anything we might need

Risk assessment

Low

Sorry, something went wrong.

vegano1 added 21 commits May 30, 2023 11:00
fix an issue when reading properties from the eeprom
…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 requested a review from a team as a code owner July 6, 2023 18:27
@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #13054 (bd03c8d) into edge (056bdcf) will decrease coverage by 0.66%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13054      +/-   ##
==========================================
- Coverage   72.34%   71.68%   -0.66%     
==========================================
  Files        2379     1560     -819     
  Lines       65363    51427   -13936     
  Branches     7289     3256    -4033     
==========================================
- Hits        47284    36868   -10416     
+ Misses      16375    14077    -2298     
+ Partials     1704      482    -1222     
Flag Coverage Δ
app 43.69% <ø> (-27.58%) ⬇️
components 64.42% <ø> (ø)
hardware 56.60% <0.00%> (-1.05%) ⬇️
labware-library 49.61% <ø> (ø)
notify-server 89.13% <ø> (ø)
protocol-designer 44.06% <ø> (ø)
step-generation 84.28% <ø> (ø)

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

Impacted Files Coverage Δ
...rdware/opentrons_hardware/scripts/eeprom_writer.py 0.00% <0.00%> (ø)

... and 820 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.

eeprom_writer not eeprom_writter and some nits, ut otherwise looks good!

@@ -0,0 +1,293 @@
"""This script is able to read/write to/from the Flex EEPROM.
Copy link
Member

Choose a reason for hiding this comment

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

nit: all of the detail about what arguments do should be in the argument help text instead of here so we don't have to write them twice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We print out this module doc string using the doc variable as part of the argParser description whenever we pass in --help.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah but --help will also print out the helpstring for each argument on its own, so itll be duplicated

hardware/opentrons_hardware/scripts/eeprom_writter.py Outdated Show resolved Hide resolved
raise errors instead of calling exit()
add serial number format description
only write FORMAT_VERSION if we have at least one other property to write
@vegano1 vegano1 requested review from sfoster1 and laviera July 7, 2023 15:31
Base automatically changed from RET-1346_add_eeprom_module to edge July 7, 2023 15:39
@laviera
Copy link

laviera commented Jul 7, 2023

@vegano1 - Should there be a "print raw" - allowing us to just dump the raw eeprom

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 overall!

@vegano1 vegano1 merged commit 8fb3d31 into edge Jul 7, 2023
@vegano1 vegano1 deleted the RET-1349_eeprom_writter_script branch July 7, 2023 17:45
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.

None yet

3 participants