Skip to content

Commit

Permalink
address feadback and add unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@vegano1
vegano1 committed Oct 3, 2023
1 parent 04880d0 commit c1bb445
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 47 deletions.
35 changes: 17 additions & 18 deletions api/src/opentrons/hardware_control/modules/module_calibration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,24 @@ def load_module_calibration_offset(
module_id: str,
) -> Optional[ModuleCalibrationOffset]:
"""Loads the calibration offset for a module."""
module_offset: Optional[ModuleCalibrationOffset] = None
module_offset_data = get_module_offset(module_type, module_id)
if module_offset_data:
module_offset = ModuleCalibrationOffset(
module=module_type,
module_id=module_id,
slot=module_offset_data.slot,
mount=module_offset_data.mount,
offset=module_offset_data.offset,
last_modified=module_offset_data.lastModified,
instrument_id=module_offset_data.instrument_id,
source=module_offset_data.source,
status=CalibrationStatus(
markedAt=module_offset_data.status.markedAt,
markedBad=module_offset_data.status.markedBad,
source=module_offset_data.status.source,
),
)
return module_offset
if not module_offset_data:
return None
return ModuleCalibrationOffset(
module=module_type,
module_id=module_id,
slot=module_offset_data.slot,
mount=module_offset_data.mount,
offset=module_offset_data.offset,
last_modified=module_offset_data.lastModified,
instrument_id=module_offset_data.instrument_id,
source=module_offset_data.source,
status=CalibrationStatus(
markedAt=module_offset_data.status.markedAt,
markedBad=module_offset_data.status.markedBad,
source=module_offset_data.status.source,
),
)


def save_module_calibration_offset(
Expand Down
42 changes: 23 additions & 19 deletions api/src/opentrons/protocol_engine/state/geometry.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,24 +194,28 @@ def _normalize_module_calibration_offset(
offset_data: Optional[ModuleOffsetData],
) -> ModuleOffsetVector:
"""Normalize the module calibration offset depending on the module location."""
offset = ModuleOffsetVector(x=0, y=0, z=0)
if offset_data:
offset = offset_data.moduleOffsetVector
calibrated_slot_column = self.get_slot_column(offset_data.location.slotName)
current_slot_column = self.get_slot_column(module_location.slotName)
# make sure that we have valid colums since we cant have modules in the middle of the deck
assert set([calibrated_slot_column, current_slot_column]).issubset(
{1, 3}
), f"Module is an invalid slot {module_location}"
if calibrated_slot_column != current_slot_column:
# The module has moved from one side of the deck to the other
# Since the module was rotated, the calibration offset vector needs to be rotated by 180 degrees along the z axis
saved_offset = array([offset.x, offset.y, offset.z])
rotation_matrix = array([[-1, 0, 1], [0, -1, 1], [0, 0, 1]])
new_offset = dot(saved_offset, rotation_matrix) # type: ignore[no-untyped-call]
offset = ModuleOffsetVector(
x=new_offset[0], y=new_offset[1], z=new_offset[2]
)
if not offset_data:
return ModuleOffsetVector(x=0, y=0, z=0)
offset = offset_data.moduleOffsetVector
calibrated_slot = offset_data.location.slotName
calibrated_slot_column = self.get_slot_column(calibrated_slot)
current_slot_column = self.get_slot_column(module_location.slotName)
# make sure that we have valid colums since we cant have modules in the middle of the deck
assert set([calibrated_slot_column, current_slot_column]).issubset(
{1, 3}
), f"Module calibration offset is an invalid slot {calibrated_slot}"

# Check if the module has moved from one side of the deck to the other
if calibrated_slot_column != current_slot_column:
print(f"ROTATE: {calibrated_slot_column} != {current_slot_column}")
# Since the module was rotated, the calibration offset vector needs to be rotated by 180 degrees along the z axis
saved_offset = array([offset.x, offset.y, offset.z])
rotation_matrix = array([[-1, 0, 0], [0, -1, 0], [0, 0, 1]])
new_offset = dot(saved_offset, rotation_matrix) # type: ignore[no-untyped-call]
offset = ModuleOffsetVector(
x=new_offset[0], y=new_offset[1], z=new_offset[2]
)
print(f"OFFSET: {offset}")
return offset

def _get_calibrated_module_offset(
Expand All @@ -221,7 +225,7 @@ def _get_calibrated_module_offset(
if isinstance(location, ModuleLocation):
module_id = location.moduleId
module_location = self._modules.get_location(module_id)
offset_data = self._modules.get_calibration_module_offset(module_id)
offset_data = self._modules.get_module_calibration_offset(module_id)
return self._normalize_module_calibration_offset(
module_location, offset_data
)
Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/protocol_engine/state/modules.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ def get_nominal_module_offset(
z=xformed[2],
)

def get_calibration_module_offset(
def get_module_calibration_offset(
self, module_id: str
) -> Optional[ModuleOffsetData]:
"""Get the calibration module offset."""
Expand Down
2 changes: 1 addition & 1 deletion api/tests/opentrons/protocol_engine/commands/calibration/test_calibrate_module.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ async def test_calibrate_module_implementation(
location
)
decoy.when(
subject._state_view.modules.get_calibration_module_offset(module_id)
subject._state_view.modules.get_module_calibration_offset(module_id)
).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=0, y=0, z=0),
Expand Down
69 changes: 61 additions & 8 deletions api/tests/opentrons/protocol_engine/state/test_geometry_view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def test_get_labware_parent_position_on_module(
"labware-id", ModuleModel.THERMOCYCLER_MODULE_V2
)
).then_return(OverlapOffset(x=1, y=2, z=3))
decoy.when(module_view.get_calibration_module_offset("module-id")).then_return(
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=2, y=3, z=4),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_3),
Expand Down Expand Up @@ -203,9 +203,6 @@ def test_get_labware_parent_position_on_labware(
decoy.when(labware_view.get_slot_position(DeckSlotName.SLOT_3)).then_return(
Point(1, 2, 3)
)
decoy.when(labware_view.get_slot_position(DeckSlotName.SLOT_3)).then_return(
Point(1, 2, 3)
)
decoy.when(labware_view.get("adapter-id")).then_return(adapter_data)
decoy.when(labware_view.get_dimensions("adapter-id")).then_return(
Dimensions(x=123, y=456, z=5)
Expand All @@ -230,7 +227,7 @@ def test_get_labware_parent_position_on_labware(
)
).then_return(OverlapOffset(x=-3, y=-2, z=-1))

decoy.when(module_view.get_calibration_module_offset("module-id")).then_return(
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=3, y=4, z=5),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_3),
Expand All @@ -242,6 +239,62 @@ def test_get_labware_parent_position_on_labware(
assert result == Point(9, 12, 15)


def test_module_calibration_offset_rotation(
decoy: Decoy,
labware_view: LabwareView,
module_view: ModuleView,
ot2_standard_deck_def: DeckDefinitionV3,
subject: GeometryView,
) -> None:
"""Return the rotated module calibration offset if the module was moved from one side of the deck to the other."""
labware_data = LoadedLabware(
id="labware-id",
loadName="b",
definitionUri=uri_from_details(namespace="a", load_name="b", version=1),
location=ModuleLocation(moduleId="module-id"),
offsetId=None,
)

decoy.when(labware_view.get("labware-id")).then_return(labware_data)
decoy.when(module_view.get_location("module-id")).then_return(
DeckSlotLocation(slotName=DeckSlotName.SLOT_D1)
)
decoy.when(module_view.get_connected_model("module-id")).then_return(
ModuleModel.TEMPERATURE_MODULE_V2
)
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=2, y=3, z=4),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_D1),
)
)

# the module has not changed location after calibration, so there is no rotation
result = subject._get_calibrated_module_offset(ModuleLocation(moduleId="module-id"))
assert result == ModuleOffsetVector(x=2, y=3, z=4)

# the module has changed from slot D1 to D3, so we should rotate the calibration offset 180 degrees along the z axis
decoy.when(module_view.get_location("module-id")).then_return(
DeckSlotLocation(slotName=DeckSlotName.SLOT_D3)
)
result = subject._get_calibrated_module_offset(ModuleLocation(moduleId="module-id"))
assert result == ModuleOffsetVector(x=-2, y=-3, z=4)

# attempting to load the module calibration offset from an invalid slot in the middle of the deck (A2, B2, C2, D2)
# is not be allowed since you can't even load a module in the middle to perform a module calibration in the
# first place. So if someone manually edits the stored module calibration offset we will throw an assert error.
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=2, y=3, z=4),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_D2),
)
)
with pytest.raises(AssertionError):
result = subject._get_calibrated_module_offset(
ModuleLocation(moduleId="module-id")
)


def test_get_labware_origin_position(
decoy: Decoy,
well_plate_def: LabwareDefinition,
Expand Down Expand Up @@ -344,7 +397,7 @@ def test_get_module_labware_highest_z(
)
).then_return(LabwareOffsetVector(x=4, y=5, z=6))
decoy.when(module_view.get_height_over_labware("module-id")).then_return(0.5)
decoy.when(module_view.get_calibration_module_offset("module-id")).then_return(
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=0, y=0, z=0),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_3),
Expand Down Expand Up @@ -651,7 +704,7 @@ def test_get_module_labware_well_position(
module_id="module-id", deck_type=DeckType.OT2_STANDARD
)
).then_return(LabwareOffsetVector(x=4, y=5, z=6))
decoy.when(module_view.get_calibration_module_offset("module-id")).then_return(
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=0, y=0, z=0),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_3),
Expand Down Expand Up @@ -1186,7 +1239,7 @@ def test_get_labware_grip_point_for_labware_on_module(
"labware-id", ModuleModel.MAGNETIC_MODULE_V2
)
).then_return(OverlapOffset(x=10, y=20, z=30))
decoy.when(module_view.get_calibration_module_offset("module-id")).then_return(
decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
ModuleOffsetData(
moduleOffsetVector=ModuleOffsetVector(x=100, y=200, z=300),
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4),
Expand Down

0 comments on commit c1bb445

Please sign in to comment.