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(api): add protocol api and engine support for absorbance plate reader #15266

Merged
merged 28 commits into from
Jun 12, 2024

Conversation

ahiuchingau
Copy link
Contributor

@ahiuchingau ahiuchingau commented May 24, 2024

Overview

This PR adds protocol api and engine support for absorbance plate reader.

Requires OE-core changes in Opentrons/oe-core#151

Test Plan

Changelog

Backend changes:

  • Added AbsorbanceReaderCore as a new module core in the protocol api
  • Added two functions for the abs reader core
    • def initialize(wavelength: int): to allow for reference wavelengths
    • def initiate_read(): to actually get a 96 sample absorbance reading from the plate reader
  • Added absorbance plate reader to module context
  • Added protocol engine commands to execute initialize and initiate_read
  • Updated protocol engine absorbance reader substate to reflect changes
  • Created live data class in robot server to receive plate reader statuses

Frontend changes:

  • Added AbsorbanceReader to ModuleCard
  • Added deck configurator support for absorbance plate reader
  • Protocol-Designer * see review requests below
    • Added module data and state to compoenents
    • Updated labware selection modal for module
    • You can now add absorbance plate reader to the protocol, but you still can't add a labware just yet

Shared-data changes:

  • Added absorbance reader fixtures in deck definitions to allow module to be loaded in column 3
  • Removed some copied info from the module definition file and updated dimensions

closes PLAT-336

@ahiuchingau ahiuchingau force-pushed the abs96_protocol-engine branch 2 times, most recently from 18945e6 to 4d43e40 Compare May 24, 2024 21:01
@ahiuchingau ahiuchingau force-pushed the abs96_protocol-engine branch from 4d43e40 to 0c7d7d8 Compare May 28, 2024 18:28
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.48%. Comparing base (2afa859) to head (acdf35b).
Report is 248 commits behind head on edge.

Current head acdf35b differs from pull request most recent head b269f43

Please upload reports for the commit b269f43 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #15266       +/-   ##
===========================================
+ Coverage   63.20%   81.48%   +18.28%     
===========================================
  Files         287      118      -169     
  Lines       14891     4078    -10813     
===========================================
- Hits         9412     3323     -6089     
+ Misses       5479      755     -4724     
Flag Coverage Δ
g-code-testing 92.43% <ø> (ø)
hardware ?
shared-data 76.45% <ø> (+0.38%) ⬆️
system-server ?
update-server ?
usb-bridge ?

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

see 202 files with indirect coverage changes

@ahiuchingau ahiuchingau marked this pull request as ready for review May 30, 2024 18:18
@ahiuchingau ahiuchingau requested review from a team as code owners May 30, 2024 18:18
@ahiuchingau ahiuchingau requested review from smb2268 and removed request for a team May 30, 2024 18:18
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.

I reviewed the hardware controller and protocol engine integration, that looks good to me.
As for the docs failing, I think you just have to add the AbsorbanceReaderContext to the returns list of module context in the load_module docstring here.

MeasureAbsorbanceParams, SuccessData[MeasureAbsorbanceResult, None]
]
):
"""Execution implementation of a Thermocycler's run profile command."""
Copy link
Contributor

Choose a reason for hiding this comment

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

copypasta comment needs replacing

"properties": {
"moduleId": {
"title": "Moduleid",
"description": "Unique ID of the Thermocycler.",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pulled from the docstring in the protocol engine command, so it should be fixed there

Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

🍽️ 🔖

@ahiuchingau ahiuchingau force-pushed the abs96_protocol-engine branch from 12cb466 to acdf35b Compare June 11, 2024 19:22
@ahiuchingau ahiuchingau requested a review from jerader June 12, 2024 15:21
Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

PD stuff looks great! thanks for putting it behind the ff 😄

@ahiuchingau ahiuchingau merged commit 3c8df23 into edge Jun 12, 2024
56 checks passed
@ahiuchingau ahiuchingau deleted the abs96_protocol-engine branch June 12, 2024 17:41
aaron-kulkarni pushed a commit that referenced this pull request Jun 13, 2024
…eader (#15266)

# Overview
This PR adds protocol api and engine support for absorbance plate
reader.

Requires OE-core changes in
Opentrons/oe-core#151
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.

4 participants