Skip to content

Commit

Permalink
fix(app, robot-server): notify on maintenance run status change (#16150)
Browse files Browse the repository at this point in the history
# 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 <[email protected]>
  • Loading branch information
TamarZanzouri and sfoster1 authored Aug 29, 2024
1 parent f208e93 commit f789332
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 15 deletions.
16 changes: 13 additions & 3 deletions app/src/organisms/GripperWizardFlows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -108,6 +110,7 @@ export function GripperWizardFlows(
}
}, [
maintenanceRunData?.data.id,
maintenanceRunData?.data.status,
createdMaintenanceRunId,
monitorMaintenanceRunForDeletion,
closeFlow,
Expand Down Expand Up @@ -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}
Expand All @@ -183,6 +187,7 @@ export function GripperWizardFlows(
interface GripperWizardProps {
flowType: GripperWizardFlowType
maintenanceRunId?: string
maintenanceRunStatus?: RunStatus
createdMaintenanceRunId: string | null
attachedGripper: InstrumentData | null
createMaintenanceRun: UseMutateFunction<
Expand Down Expand Up @@ -212,6 +217,7 @@ export const GripperWizard = (
const {
flowType,
maintenanceRunId,
maintenanceRunStatus,
createMaintenanceRun,
handleCleanUpAndClose,
handleClose,
Expand Down Expand Up @@ -266,6 +272,7 @@ export const GripperWizard = (
}

const sharedProps = {
maintenanceRunStatus,
flowType,
maintenanceRunId:
maintenanceRunId != null && createdMaintenanceRunId === maintenanceRunId
Expand All @@ -283,7 +290,7 @@ export const GripperWizard = (
let onExit
if (currentStep == null) return null
let modalContent: JSX.Element = <div>UNASSIGNED STEP</div>
if (showConfirmExit) {
if (showConfirmExit && maintenanceRunId !== null) {
modalContent = (
<ExitConfirmation
handleGoBack={cancelExit}
Expand All @@ -292,14 +299,17 @@ export const GripperWizard = (
isRobotMoving={isRobotMoving}
/>
)
} else if (isExiting && errorMessage != null) {
} else if (
(isExiting && errorMessage != null) ||
maintenanceRunStatus === RUN_STATUS_FAILED
) {
onExit = handleClose
modalContent = (
<SimpleWizardBody
isSuccess={false}
iconColor={COLORS.red50}
header={t('shared:error_encountered')}
subHeader={errorMessage}
subHeader={errorMessage ?? undefined}
/>
)
} else if (currentStep.section === SECTIONS.BEFORE_BEGINNING) {
Expand Down
7 changes: 6 additions & 1 deletion app/src/organisms/ModuleWizardFlows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 = (
<SimpleWizardBody
isSuccess={false}
Expand Down
13 changes: 10 additions & 3 deletions app/src/organisms/PipetteWizardFlows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import type {
PipetteMount,
} from '@opentrons/shared-data'
import type { CommandData, HostConfig } from '@opentrons/api-client'
import { RUN_STATUS_FAILED } from '@opentrons/api-client'
import type { PipetteWizardFlow, SelectablePipettes } from './types'

const RUN_REFETCH_INTERVAL = 5000
Expand Down Expand Up @@ -280,13 +281,16 @@ export const PipetteWizardFlows = (
let onExit
if (currentStep == null) return null
let modalContent: JSX.Element = <div>UNASSIGNED STEP</div>
if (isExiting && errorMessage != null) {
if (
(isExiting && errorMessage != null) ||
maintenanceRunData?.data.status === RUN_STATUS_FAILED
) {
modalContent = (
<SimpleWizardBody
isSuccess={false}
iconColor={COLORS.red50}
header={t('shared:error_encountered')}
subHeader={errorMessage}
subHeader={errorMessage ?? undefined}
/>
)
} else if (currentStep.section === SECTIONS.BEFORE_BEGINNING) {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down

0 comments on commit f789332

Please sign in to comment.