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(shared-data, api): add absorbance plate reader definition and module control #15167

Merged
merged 20 commits into from
May 15, 2024

Conversation

ahiuchingau
Copy link
Contributor

@ahiuchingau ahiuchingau commented May 10, 2024

Overview

This PR adds support for the Absorbance Plate Reader in the hardware controller API. We can now see the plate reader as an attached module on the backend and use the AbsorbanceReaderDriver to communicate with the plate reader like we would with the other modules.

Caveat

The driver is not fully finished - there are some functions that still need to be implemented and I expect the format of the response will change depending on how we implement the engine commands.

We still need to add some changes on oe-core side to make sure this runs on all robot - however i'm happy to show you the plate reader in action at this time.

@ahiuchingau ahiuchingau marked this pull request as ready for review May 13, 2024 15:31
@ahiuchingau ahiuchingau requested review from a team as code owners May 13, 2024 15:31
Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (1dede8b) to head (eeffc32).
Report is 34 commits behind head on edge.

❗ Current head eeffc32 differs from pull request most recent head 32ab370. Consider uploading reports for the commit 32ab370 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #15167   +/-   ##
=======================================
  Coverage   92.43%   92.43%           
=======================================
  Files          77       77           
  Lines        1283     1283           
=======================================
  Hits         1186     1186           
  Misses         97       97           
Flag Coverage Δ
g-code-testing 92.43% <ø> (ø)

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

) -> Tuple[ErrorCode, bool]:
...

def byonoy_abs96_get_available_wavelengths(
Copy link
Contributor

Choose a reason for hiding this comment

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

are these all commands you access through the hid interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes they are! So we'd have to update this file and the AsyncByonoy whenever the byonoy library gets updated.

Copy link
Contributor

@caila-marashaj caila-marashaj left a comment

Choose a reason for hiding this comment

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

Nice! Congrats on getting this up and running

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.

This looks pretty good, this seems like a very annoying interface to deal with.

Let's get those error strings defined as a Literal enum or something

@@ -280,6 +280,15 @@
],
"version": "==4.2.2"
},
"pyusb": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to include all these changes to the lock file or just whats required to include pyusb?

Copy link
Contributor

@vegano1 vegano1 left a comment

Choose a reason for hiding this comment

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

This looks good,
Just have a comment on whether we want to update all the Pipfile.lock files, which will update the version of a few other modules or just add whats required for pyusb .

@vegano1 vegano1 self-requested a review May 15, 2024 16:37
@ahiuchingau ahiuchingau merged commit 643a9dd into edge May 15, 2024
42 checks passed
@ahiuchingau ahiuchingau deleted the PLAT-191-add-plate-reader-type branch May 15, 2024 18:38
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
…dule control (#15167)

<!--
Thanks for taking the time to open a pull request! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview
This PR adds support for the Absorbance Plate Reader in the hardware
controller API. We can now see the plate reader as an attached module on
the backend and use the AbsorbanceReaderDriver to communicate with the
plate reader like we would with the other modules.

# Caveat
The driver is not fully finished - there are some functions that still
need to be implemented and I expect the format of the response will
change depending on how we implement the engine commands.

We still need to add some changes on oe-core side to make sure this runs
on all robot - however i'm happy to show you the plate reader in action
at this time.
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

4 participants