diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 78dac659e256..f03f8450e4c2 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -619,49 +619,35 @@ def get_slice( """Get a subset of commands around a given cursor. If the cursor is omitted, a cursor will be selected automatically - based on the currently running or most recently executed command. + based on the currently running or most recently executed command, + and the slice of commands returned is the previous `length` commands + inclusive of the currently running or most recently executed command. """ command_ids = self._state.command_history.get_filtered_command_ids( include_fixit_commands=include_fixit_commands ) - running_command = self._state.command_history.get_running_command() - queued_command_ids = self._state.command_history.get_queue_ids() total_length = len(command_ids) - # TODO(mm, 2024-05-17): This looks like it's attempting to do the same thing - # as self.get_current(), but in a different way. Can we unify them? if cursor is None: - if running_command is not None: - cursor = running_command.index - elif len(queued_command_ids) > 0: - # Get the most recently executed command, - # which we can find just before the first queued command. - cursor = ( - self._state.command_history.get(queued_command_ids.head()).index - 1 - ) - elif ( - self._state.run_result - and self._state.run_result == RunResult.FAILED - and self._state.failed_command - ): - # Currently, if the run fails, we mark all the commands we didn't - # reach as failed. This makes command status alone insufficient to - # find the most recent command that actually executed, so we need to - # store that separately. - cursor = self._state.failed_command.index + current_pointer = self.get_current() + + if current_pointer is not None: + cursor = current_pointer.index else: - cursor = total_length - length + cursor = total_length - 1 + + cursor = max(cursor - length + 1, 0) # start is inclusive, stop is exclusive - actual_cursor = max(0, min(cursor, total_length - 1)) - stop = min(total_length, actual_cursor + length) + start = max(0, min(cursor, total_length - 1)) + stop = min(total_length, start + length) commands = self._state.command_history.get_slice( - start=actual_cursor, stop=stop, command_ids=command_ids + start=start, stop=stop, command_ids=command_ids ) return CommandSlice( commands=commands, - cursor=actual_cursor, + cursor=start, total_length=total_length, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index de242d83f51f..6b0c5e3ee697 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -1000,8 +1000,8 @@ def test_get_slice_default_cursor_failed_command() -> None: result = subject.get_slice(cursor=None, length=3, include_fixit_commands=True) assert result == CommandSlice( - commands=[command_3, command_4], - cursor=2, + commands=[command_2, command_3, command_4], + cursor=1, total_length=4, ) @@ -1022,14 +1022,14 @@ def test_get_slice_default_cursor_running() -> None: result = subject.get_slice(cursor=None, length=2, include_fixit_commands=True) assert result == CommandSlice( - commands=[command_3, command_4], - cursor=2, + commands=[command_2, command_3], + cursor=1, total_length=5, ) def test_get_slice_without_fixit() -> None: - """It should select a cursor based on the running command, if present.""" + """It should filter out fixit commands when requested.""" command_1 = create_succeeded_command(command_id="command-id-1") command_2 = create_succeeded_command(command_id="command-id-2") command_3 = create_running_command(command_id="command-id-3") @@ -1071,3 +1071,44 @@ def test_get_slice_without_fixit() -> None: cursor=0, total_length=5, ) + + +def test_get_slice_large_length() -> None: + """It should handle cases where length is larger than available commands.""" + command_1 = create_succeeded_command(command_id="command-id-1") + command_2 = create_succeeded_command(command_id="command-id-2") + command_3 = create_running_command(command_id="command-id-3") + + subject = get_command_view( + commands=[command_1, command_2, command_3], + running_command_id="command-id-3", + ) + + result = subject.get_slice(cursor=None, length=10, include_fixit_commands=True) + + assert result == CommandSlice( + commands=[command_1, command_2, command_3], + cursor=0, + total_length=3, + ) + + +def test_get_slice_explicit_cursor_with_length() -> None: + """It should use the cursor as the start position when explicitly provided.""" + command_1 = create_succeeded_command(command_id="command-id-1") + command_2 = create_succeeded_command(command_id="command-id-2") + command_3 = create_succeeded_command(command_id="command-id-3") + command_4 = create_succeeded_command(command_id="command-id-4") + command_5 = create_succeeded_command(command_id="command-id-5") + + subject = get_command_view( + commands=[command_1, command_2, command_3, command_4, command_5], + ) + + result = subject.get_slice(cursor=1, length=3, include_fixit_commands=True) + + assert result == CommandSlice( + commands=[command_2, command_3, command_4], + cursor=1, + total_length=5, + ) diff --git a/robot-server/robot_server/commands/router.py b/robot-server/robot_server/commands/router.py index 4d23bb73fbe1..b8d935137c6a 100644 --- a/robot-server/robot_server/commands/router.py +++ b/robot-server/robot_server/commands/router.py @@ -135,7 +135,9 @@ async def get_commands_list( description=( "The starting index of the desired first command in the list." " If unspecified, a cursor will be selected automatically" - " based on the currently running or most recently executed command." + " based on the currently running or most recently executed command, " + " and the slice of commands returned is the previous `pageLength` commands" + " inclusive of the currently running or most recently executed command." ), ), ] = None, diff --git a/robot-server/robot_server/maintenance_runs/router/commands_router.py b/robot-server/robot_server/maintenance_runs/router/commands_router.py index 0bb3d64c1924..8903a74bfdb5 100644 --- a/robot-server/robot_server/maintenance_runs/router/commands_router.py +++ b/robot-server/robot_server/maintenance_runs/router/commands_router.py @@ -197,7 +197,9 @@ async def get_run_commands( description=( "The starting index of the desired first command in the list." " If unspecified, a cursor will be selected automatically" - " based on the currently running or most recently executed command." + " based on the currently running or most recently executed command, " + " and the slice of commands returned is the previous `length` commands" + " inclusive of the currently running or most recently executed command." ), ), ] = None, diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index 8e478c08de9a..7c33ac738507 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -473,7 +473,8 @@ async def get_run_commands_error( description=( "The starting index of the desired first command error in the list." " If unspecified, a cursor will be selected automatically" - " based on the last error added." + " based on the last error added, and the slice of errors returned " + " is the previous `pageLength` errors." ), ), ] = None, @@ -490,12 +491,9 @@ async def get_run_commands_error( all_errors_count = run_data_manager.get_command_errors_count(run_id=runId) if cursor is None: - if all_errors_count > 0: - # Get the most recent error, - # which we can find just at the end of the list. - cursor = all_errors_count - 1 - else: - cursor = 0 + cursor = max(all_errors_count - 1, 0) + cursor = max(cursor - pageLength + 1, 0) + cursor = min(cursor, all_errors_count) command_error_slice = run_data_manager.get_command_error_slice( run_id=runId, diff --git a/robot-server/robot_server/runs/router/commands_router.py b/robot-server/robot_server/runs/router/commands_router.py index b2cbb79b2bbf..6bbeaaca6291 100644 --- a/robot-server/robot_server/runs/router/commands_router.py +++ b/robot-server/robot_server/runs/router/commands_router.py @@ -276,7 +276,9 @@ async def get_run_commands( description=( "The starting index of the desired first command in the list." " If unspecified, a cursor will be selected automatically" - " based on the currently running or most recently executed command." + " based on the currently running or most recently executed command, " + " and the slice of commands returned is the previous `pageLength` commands" + " inclusive of the currently running or most recently executed command." ), ), ] = None, diff --git a/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml index 27f894fe4115..6478e6a64899 100644 --- a/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml @@ -91,9 +91,36 @@ stages: key: !anystr createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" meta: - cursor: 3 + cursor: 0 totalLength: 4 data: + - id: !anystr + key: !anystr + commandType: home + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: succeeded + params: !anydict + notes: !anylist + - id: !anystr + key: !anystr + commandType: loadLabware + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: succeeded + params: !anydict + notes: !anylist + - id: !anystr + key: !anystr + commandType: loadPipette + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: succeeded + params: !anydict + notes: !anylist - id: !anystr key: !anystr commandType: aspirate @@ -115,12 +142,6 @@ stages: pipetteId: pipetteId labwareId: tipRackId wellName: A1 - wellLocation: - origin: bottom - offset: - x: 0 - y: 0 - z: 1 - volumeOffset: 0 - flowRate: 3.78 - volume: 100 + wellLocation: !anydict + flowRate: !anyfloat + volume: !anyfloat \ No newline at end of file diff --git a/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml index 804b0b0e620d..281188dc9eb8 100644 --- a/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml @@ -92,17 +92,51 @@ stages: key: !anystr createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" meta: - cursor: 3 + cursor: 0 totalLength: 4 data: - id: !anystr key: !anystr + commandType: home + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: succeeded + params: !anydict + notes: !anylist + - id: !re_fullmatch "commands\\.LOAD_LABWARE-\\d+" + key: !re_fullmatch "commands\\.LOAD_LABWARE-\\d+" + commandType: loadLabware + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: succeeded + params: + location: + slotName: "1" + loadName: opentrons_96_tiprack_300ul + namespace: opentrons + version: 1 + notes: !anylist + - id: !re_fullmatch "commands\\.LOAD_PIPETTE-\\d+" + key: !re_fullmatch "commands\\.LOAD_PIPETTE-\\d+" + commandType: loadPipette + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: succeeded + params: + pipetteName: p300_single + mount: right + notes: !anylist + - id: !re_fullmatch "command\\.ASPIRATE-\\d+" + key: !re_fullmatch "command\\.ASPIRATE-\\d+" commandType: aspirate createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" startedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" completedAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" status: failed - notes: [] + notes: !anylist error: id: !anystr errorType: LegacyContextCommandError @@ -119,9 +153,9 @@ stages: wellLocation: origin: top offset: - x: 0 - y: 0 - z: 0 - volumeOffset: 0 - flowRate: 150 - volume: 100 + x: !anyfloat + y: !anyfloat + z: !anyfloat + volumeOffset: !anyfloat + flowRate: !anyfloat + volume: !anyfloat \ No newline at end of file