refactor(api): Emit proper comment commands from legacy protocols (#…

…15426)

api/src/opentrons/protocol_runner/legacy_command_mapper.py

@@ -236,6 +236,15 @@ def map_command( # noqa: C901
  "notes": [],
  }
  )
+ elif isinstance(running_command, pe_commands.Comment):
+ completed_command = running_command.copy(
+ update={
+ "result": pe_commands.CommentResult.construct(),
+ "status": pe_commands.CommandStatus.SUCCEEDED,
+ "completedAt": now,
+ "notes": [],
+ }
+ )
  elif isinstance(running_command, pe_commands.Custom):
  completed_command = running_command.copy(
  update={
@@ -246,6 +255,9 @@ def map_command( # noqa: C901
  }
  )
  else:
+ # TODO(mm, 2024-06-13): This looks potentially wrong.
+ # We're creating a `SUCCEEDED` command that does not have a `result`,
+ # which is not normally possible.
  completed_command = running_command.copy(
  update={
  "status": pe_commands.CommandStatus.SUCCEEDED,
@@ -332,6 +344,21 @@ def _build_initial_command(
  )
  )
  return wait_for_resume_create, wait_for_resume_running
+ elif command["name"] == legacy_command_types.COMMENT:
+ comment_running = pe_commands.Comment.construct(
+ id=command_id,
+ key=command_id,
+ status=pe_commands.CommandStatus.RUNNING,
+ createdAt=now,
+ startedAt=now,
+ params=pe_commands.CommentParams.construct(
+ message=command["payload"]["text"],
+ ),
+ )
+ comment_create = pe_commands.CommentCreate.construct(
+ key=comment_running.key, params=comment_running.params
+ )
+ return comment_create, comment_running
  else:
  custom_running = pe_commands.Custom.construct(
  id=command_id,

api/tests/opentrons/cli/test_cli.py

@@ -152,15 +152,7 @@ def test_analysis_deck_definition(
  "commands"
  ]
 
- # ["params"]["message"] for Protocol Engine (PAPIv≥2.14),
- # ["params"]["legacyCommandText"] for the legacy backend (PAPIv≤2.13).
- # Eventually the legacy backend should change to match Protocol Engine.
- comment_message = (
- comment_command["params"]["message"]
- if "message" in comment_command["params"]
- else comment_command["params"]["legacyCommandText"]
- )
- assert comment_message == expected_point
+ assert comment_command["params"]["message"] == expected_point
 
 
 # TODO(mm, 2023-08-12): We can remove this test when we remove special handling for these

api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py

@@ -797,3 +797,30 @@ def run(protocol):
  # TODO(mm, 2024-04-23): This commands.Custom looks wrong. This should be a commands.MoveToWell.
  assert isinstance(move_to_well, commands.Custom)
  assert isinstance(air_gap_aspirate, commands.Aspirate)
+
+
+async def test_comment(tmp_path: Path) -> None:
+ """A `ProtocolContext.comment()` should be mapped to a `comment` command."""
+ path = tmp_path / "protocol.py"
+ path.write_text(
+ dedent(
+ """\
+ metadata = {"apiLevel": "2.13"}
+ def run(protocol):
+ protocol.comment("oy.")
+ """
+ )
+ )
+ result_commands = await simulate_and_get_commands(path)
+ [initial_home, comment] = result_commands
+ assert comment == commands.Comment.construct(
+ status=commands.CommandStatus.SUCCEEDED,
+ params=commands.CommentParams(message="oy."),
+ notes=[],
+ result=commands.CommentResult(),
+ createdAt=matchers.Anything(),
+ startedAt=matchers.Anything(),
+ completedAt=matchers.Anything(),
+ id=matchers.Anything(),
+ key=matchers.Anything(),
+ )

api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py

@@ -33,7 +33,6 @@
 from opentrons.protocol_runner.legacy_command_mapper import (
  LegacyContextCommandError,
  LegacyCommandMapper,
- LegacyCommandParams,
 )
 from opentrons_shared_data.labware.dev_types import LabwareDefinition
 from opentrons_shared_data.module.dev_types import ModuleDefinitionV3
@@ -73,11 +72,10 @@ def test_map_before_command() -> None:
  pe_actions.QueueCommandAction(
  command_id="command.COMMENT-0",
  created_at=matchers.IsA(datetime),
- request=pe_commands.CustomCreate(
+ request=pe_commands.CommentCreate(
  key="command.COMMENT-0",
- params=LegacyCommandParams(
- legacyCommandType="command.COMMENT",
- legacyCommandText="hello world",
+ params=pe_commands.CommentParams(
+ message="hello world",
  ),
  ),
  request_hash=None,
@@ -114,18 +112,17 @@ def test_map_after_command() -> None:
  assert result == [
  pe_actions.SucceedCommandAction(
  private_result=None,
- command=pe_commands.Custom.construct(
+ command=pe_commands.Comment.construct(
  id="command.COMMENT-0",
  key="command.COMMENT-0",
  status=pe_commands.CommandStatus.SUCCEEDED,
  createdAt=matchers.IsA(datetime),
  startedAt=matchers.IsA(datetime),
  completedAt=matchers.IsA(datetime),
- params=LegacyCommandParams(
- legacyCommandType="command.COMMENT",
- legacyCommandText="hello world",
+ params=pe_commands.CommentParams(
+ message="hello world",
  ),
- result=pe_commands.CustomResult(),
+ result=pe_commands.CommentResult(),
  notes=[],
  ),
  )
@@ -212,11 +209,10 @@ def test_command_stack() -> None:
  pe_actions.QueueCommandAction(
  command_id="command.COMMENT-0",
  created_at=matchers.IsA(datetime),
- request=pe_commands.CustomCreate(
+ request=pe_commands.CommentCreate(
  key="command.COMMENT-0",
- params=LegacyCommandParams(
- legacyCommandType="command.COMMENT",
- legacyCommandText="hello",
+ params=pe_commands.CommentParams(
+ message="hello",
  ),
  ),
  request_hash=None,
@@ -227,11 +223,10 @@ def test_command_stack() -> None:
  pe_actions.QueueCommandAction(
  command_id="command.COMMENT-1",
  created_at=matchers.IsA(datetime),
- request=pe_commands.CustomCreate(
+ request=pe_commands.CommentCreate(
  key="command.COMMENT-1",
- params=LegacyCommandParams(
- legacyCommandType="command.COMMENT",
- legacyCommandText="goodbye",
+ params=pe_commands.CommentParams(
+ message="goodbye",
  ),
  ),
  request_hash=None,
@@ -241,18 +236,17 @@ def test_command_stack() -> None:
  ),
  pe_actions.SucceedCommandAction(
  private_result=None,
- command=pe_commands.Custom.construct(
+ command=pe_commands.Comment.construct(
  id="command.COMMENT-0",
  key="command.COMMENT-0",
  status=pe_commands.CommandStatus.SUCCEEDED,
  createdAt=matchers.IsA(datetime),
  startedAt=matchers.IsA(datetime),
  completedAt=matchers.IsA(datetime),
- params=LegacyCommandParams(
- legacyCommandType="command.COMMENT",
- legacyCommandText="hello",
+ params=pe_commands.CommentParams(
+ message="hello",
  ),
- result=pe_commands.CustomResult(),
+ result=pe_commands.CommentResult(),
  notes=[],
  ),
  ),