From f7893326f155bea83f7d1c8008d246101c1eb086 Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Thu, 29 Aug 2024 13:47:43 -0400 Subject: [PATCH] fix(app, robot-server): notify on maintenance run status change (#16150) # Overview closes [RQA-2941](https://opentrons.atlassian.net/browse/RQA-2941) and [RQA-2940](https://opentrons.atlassian.net/browse/RQA-2940). listen to run status change for maintenance runs and show error in app when failed calibration. ## Test Plan and Hands on Testing explanation in ticket. - [x] test gripper calibration with estop - [x] test pipette calibration with estop - [x] test module calibration with estop ## Changelog - added a callback to handle status change for a maintenance runs. - pass in status to gripper wizard flow. - check for status in module/pipette calibration and if failed show error ## Review requests changes make sense? ## Risk assessment low. [RQA-2941]: https://opentrons.atlassian.net/browse/RQA-2941?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RQA-2940]: https://opentrons.atlassian.net/browse/RQA-2940?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Seth Foster --- .../organisms/GripperWizardFlows/index.tsx | 16 ++++- app/src/organisms/ModuleWizardFlows/index.tsx | 7 +- .../organisms/PipetteWizardFlows/index.tsx | 13 +++- .../maintenance_run_data_manager.py | 4 +- .../publishers/maintenance_runs_publisher.py | 69 ++++++++++++++++++- .../test_maintenance_runs_publisher.py | 18 +++-- 6 files changed, 112 insertions(+), 15 deletions(-) diff --git a/app/src/organisms/GripperWizardFlows/index.tsx b/app/src/organisms/GripperWizardFlows/index.tsx index aafc36acf9e..219fd687771 100644 --- a/app/src/organisms/GripperWizardFlows/index.tsx +++ b/app/src/organisms/GripperWizardFlows/index.tsx @@ -42,7 +42,9 @@ import type { InstrumentData, MaintenanceRun, CommandData, + RunStatus, } from '@opentrons/api-client' +import { RUN_STATUS_FAILED } from '@opentrons/api-client' import type { Coordinates, CreateCommand } from '@opentrons/shared-data' const RUN_REFETCH_INTERVAL = 5000 @@ -108,6 +110,7 @@ export function GripperWizardFlows( } }, [ maintenanceRunData?.data.id, + maintenanceRunData?.data.status, createdMaintenanceRunId, monitorMaintenanceRunForDeletion, closeFlow, @@ -160,6 +163,7 @@ export function GripperWizardFlows( flowType={flowType} createdMaintenanceRunId={createdMaintenanceRunId} maintenanceRunId={maintenanceRunData?.data.id} + maintenanceRunStatus={maintenanceRunData?.data.status} attachedGripper={attachedGripper} createMaintenanceRun={createTargetedMaintenanceRun} isCreateLoading={isCreateLoading} @@ -183,6 +187,7 @@ export function GripperWizardFlows( interface GripperWizardProps { flowType: GripperWizardFlowType maintenanceRunId?: string + maintenanceRunStatus?: RunStatus createdMaintenanceRunId: string | null attachedGripper: InstrumentData | null createMaintenanceRun: UseMutateFunction< @@ -212,6 +217,7 @@ export const GripperWizard = ( const { flowType, maintenanceRunId, + maintenanceRunStatus, createMaintenanceRun, handleCleanUpAndClose, handleClose, @@ -266,6 +272,7 @@ export const GripperWizard = ( } const sharedProps = { + maintenanceRunStatus, flowType, maintenanceRunId: maintenanceRunId != null && createdMaintenanceRunId === maintenanceRunId @@ -283,7 +290,7 @@ export const GripperWizard = ( let onExit if (currentStep == null) return null let modalContent: JSX.Element =
UNASSIGNED STEP
- if (showConfirmExit) { + if (showConfirmExit && maintenanceRunId !== null) { modalContent = ( ) - } else if (isExiting && errorMessage != null) { + } else if ( + (isExiting && errorMessage != null) || + maintenanceRunStatus === RUN_STATUS_FAILED + ) { onExit = handleClose modalContent = ( ) } else if (currentStep.section === SECTIONS.BEFORE_BEGINNING) { diff --git a/app/src/organisms/ModuleWizardFlows/index.tsx b/app/src/organisms/ModuleWizardFlows/index.tsx index 9cc33d05688..daa1a8b0208 100644 --- a/app/src/organisms/ModuleWizardFlows/index.tsx +++ b/app/src/organisms/ModuleWizardFlows/index.tsx @@ -39,6 +39,7 @@ import { useNotifyDeckConfigurationQuery } from '../../resources/deck_configurat import { useNotifyCurrentMaintenanceRun } from '../../resources/maintenance_runs' import type { AttachedModule, CommandData } from '@opentrons/api-client' +import { RUN_STATUS_FAILED } from '@opentrons/api-client' import type { CreateCommand, CutoutConfig, @@ -271,7 +272,11 @@ export const ModuleWizardFlows = ( })} /> ) - } else if (prepCommandErrorMessage != null || errorMessage != null) { + } else if ( + prepCommandErrorMessage != null || + errorMessage != null || + maintenanceRunData?.data.status === RUN_STATUS_FAILED + ) { modalContent = ( UNASSIGNED STEP - if (isExiting && errorMessage != null) { + if ( + (isExiting && errorMessage != null) || + maintenanceRunData?.data.status === RUN_STATUS_FAILED + ) { modalContent = ( ) } else if (currentStep.section === SECTIONS.BEFORE_BEGINNING) { @@ -395,7 +399,10 @@ export const PipetteWizardFlows = ( let exitWizardButton = onExit if (isCommandMutationLoading || isDeleteLoading) { exitWizardButton = undefined - } else if (errorMessage != null && isExiting) { + } else if ( + (errorMessage != null && isExiting) || + maintenanceRunData?.data.status === RUN_STATUS_FAILED + ) { exitWizardButton = handleClose } else if (showConfirmExit) { exitWizardButton = handleCleanUpAndClose diff --git a/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py b/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py index 76c355af72a..6c4aa4db205 100644 --- a/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py +++ b/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py @@ -115,7 +115,9 @@ async def create( state_summary=state_summary, ) - await self._maintenance_runs_publisher.publish_current_maintenance_run_async() + await self._maintenance_runs_publisher.start_publishing_for_maintenance_run( + run_id=run_id, get_state_summary=self._get_state_summary + ) return maintenance_run_data diff --git a/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py b/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py index 80285cb6452..6be16d5c390 100644 --- a/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py +++ b/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py @@ -1,20 +1,69 @@ +from dataclasses import dataclass +from typing import Callable, Optional from fastapi import Depends +from opentrons.protocol_engine.state.state_summary import StateSummary +from opentrons.protocol_engine.types import EngineStatus from server_utils.fastapi_utils.app_state import ( AppState, AppStateAccessor, get_app_state, ) from ..notification_client import NotificationClient, get_notification_client +from ..publisher_notifier import PublisherNotifier, get_pe_publisher_notifier from .. import topics +@dataclass +class _RunHooks: + """Generated during a protocol run. Utilized by MaintenanceRunsPublisher.""" + + run_id: str + get_state_summary: Callable[[str], Optional[StateSummary]] + + +@dataclass +class _EngineStateSlice: + """Protocol Engine state relevant to MaintenanceRunsPublisher.""" + + state_summary_status: Optional[EngineStatus] = None + + class MaintenanceRunsPublisher: """Publishes maintenance run topics.""" - def __init__(self, client: NotificationClient) -> None: + def __init__( + self, client: NotificationClient, publisher_notifier: PublisherNotifier + ) -> None: """Returns a configured Maintenance Runs Publisher.""" self._client = client + self._run_hooks: Optional[_RunHooks] = None + self._engine_state_slice: Optional[_EngineStateSlice] = None + + publisher_notifier.register_publish_callbacks( + [ + self._handle_engine_status_change, + ] + ) + + async def start_publishing_for_maintenance_run( + self, + run_id: str, + get_state_summary: Callable[[str], Optional[StateSummary]], + ) -> None: + """Initialize RunsPublisher with necessary information derived from the current run. + + Args: + run_id: ID of the current run. + get_state_summary: Callback to get the current run's state summary, if any. + """ + self._run_hooks = _RunHooks( + run_id=run_id, + get_state_summary=get_state_summary, + ) + self._engine_state_slice = _EngineStateSlice() + + await self.publish_current_maintenance_run_async() async def publish_current_maintenance_run_async( self, @@ -30,6 +79,21 @@ def publish_current_maintenance_run( """Publishes the equivalent of GET /maintenance_run/current_run""" self._client.publish_advise_refetch(topic=topics.MAINTENANCE_RUNS_CURRENT_RUN) + async def _handle_engine_status_change(self) -> None: + """Publish a refetch flag if the engine status has changed.""" + if self._run_hooks is not None and self._engine_state_slice is not None: + new_state_summary = self._run_hooks.get_state_summary( + self._run_hooks.run_id + ) + + if ( + new_state_summary is not None + and self._engine_state_slice.state_summary_status + != new_state_summary.status + ): + await self.publish_current_maintenance_run_async() + self._engine_state_slice.state_summary_status = new_state_summary.status + _maintenance_runs_publisher_accessor: AppStateAccessor[ MaintenanceRunsPublisher @@ -39,6 +103,7 @@ def publish_current_maintenance_run( async def get_maintenance_runs_publisher( app_state: AppState = Depends(get_app_state), notification_client: NotificationClient = Depends(get_notification_client), + publisher_notifier: PublisherNotifier = Depends(get_pe_publisher_notifier), ) -> MaintenanceRunsPublisher: """Get a singleton MaintenanceRunsPublisher to publish maintenance run topics.""" maintenance_runs_publisher = _maintenance_runs_publisher_accessor.get_from( @@ -47,7 +112,7 @@ async def get_maintenance_runs_publisher( if maintenance_runs_publisher is None: maintenance_runs_publisher = MaintenanceRunsPublisher( - client=notification_client + client=notification_client, publisher_notifier=publisher_notifier ) _maintenance_runs_publisher_accessor.set_on( app_state, maintenance_runs_publisher diff --git a/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py b/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py index bfdbbd26312..ea4a02a1e5f 100644 --- a/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py +++ b/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py @@ -1,22 +1,30 @@ """Tests for the maintenance runs publisher.""" import pytest -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, Mock from robot_server.service.notifications import MaintenanceRunsPublisher, topics +from robot_server.service.notifications.notification_client import NotificationClient +from robot_server.service.notifications.publisher_notifier import PublisherNotifier @pytest.fixture -def notification_client() -> AsyncMock: +def notification_client() -> Mock: """Mocked notification client.""" - return AsyncMock() + return Mock(spec_set=NotificationClient) + + +@pytest.fixture +def publisher_notifier() -> Mock: + """Mocked publisher notifier.""" + return Mock(spec_set=PublisherNotifier) @pytest.fixture def maintenance_runs_publisher( - notification_client: AsyncMock, + notification_client: Mock, publisher_notifier: Mock ) -> MaintenanceRunsPublisher: """Instantiate MaintenanceRunsPublisher.""" - return MaintenanceRunsPublisher(notification_client) + return MaintenanceRunsPublisher(notification_client, publisher_notifier) @pytest.mark.asyncio