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

refactor(hardware): constants from firmware #9016

Merged
merged 6 commits into from
Dec 8, 2021
Merged

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Dec 7, 2021

New version of #8995 . Don't merge until api tests pass.

  • chore(hardware): add firmware python module as dep

Pipenv can add git repo dependencies, so we can add the
opentrons_ot3_firmware repo here.

If you want to switch branches you can edit the pipfile (there's not
really a better way to do this).

This will become a problem if we ever want to depend on this module and
get its transitive dependencies because that stuff only reads setup.py,
but we don't really do that yet.

  • chore(hardware): rely on constants from firmware

This lets us take a bunch of code out how nice

  • add pushing to ot3

* chore(hardware): add firmware python module as dep

Pipenv can add git repo dependencies, so we can add the
opentrons_ot3_firmware repo here.

If you want to switch branches you can edit the pipfile (there's not
really a better way to do this).

This will become a problem if we ever want to depend on this module and
get its transitive dependencies because that stuff only reads setup.py,
but we don't really do that yet.

* chore(hardware): rely on constants from firmware

This lets us take a bunch of code out how nice

* add pushing to ot3
@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #9016 (92dec4d) into edge (224f4c2) will increase coverage by 0.02%.
The diff coverage is 85.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #9016      +/-   ##
==========================================
+ Coverage   74.71%   74.74%   +0.02%     
==========================================
  Files        1861     1862       +1     
  Lines       49479    49567      +88     
  Branches     4869     4884      +15     
==========================================
+ Hits        36969    37049      +80     
- Misses      11662    11666       +4     
- Partials      848      852       +4     
Flag Coverage Δ
api 84.75% <100.00%> (+0.04%) ⬆️
app 69.88% <ø> (+<0.01%) ⬆️
robot-server 91.99% <ø> (ø)

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

Impacted Files Coverage Δ
hardware/opentrons_hardware/scripts/move.py 0.00% <0.00%> (ø)
...pi/src/opentrons/hardware_control/ot3controller.py 42.30% <100.00%> (ø)
...are/opentrons_hardware/drivers/can_bus/__init__.py 100.00% <100.00%> (ø)
...pentrons_hardware/drivers/can_bus/can_messenger.py 86.27% <100.00%> (ø)
...dware/opentrons_hardware/drivers/can_bus/driver.py 83.63% <100.00%> (ø)
...ons_hardware/hardware_control/move_group_runner.py 94.82% <100.00%> (ø)
hardware/opentrons_hardware/scripts/can_comm.py 62.50% <100.00%> (ø)
...tup/LabwarePositionCheck/ConfirmPickUpTipModal.tsx 33.33% <0.00%> (-66.67%) ⬇️
app/src/organisms/RunTimeControl/index.tsx 84.48% <0.00%> (-7.19%) ⬇️
...ganisms/ProtocolUpload/hooks/useCloseCurrentRun.ts 36.00% <0.00%> (-0.37%) ⬇️
... and 20 more

@sfoster1 sfoster1 marked this pull request as ready for review December 8, 2021 18:51
@sfoster1 sfoster1 requested review from a team as code owners December 8, 2021 18:51
$(sphinx_build) -b latex -d docs/build/doctrees $< $(@D)
$(sphinx_build_allow_warnings) -b latex -d docs/build/doctrees $< $(@D)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bummer, but 👍 if this fixes spurious/uncontrollable CI stuff.

(Cross-reference for posterity: #6135)

@sfoster1 sfoster1 merged commit 7cbcbec into edge Dec 8, 2021
@sfoster1 sfoster1 deleted the use-ot3firmware-constants branch December 8, 2021 21:05
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.

3 participants