Skip to content

Commit

Permalink
refactor(discovery-client,app,app-shell): robust robot model detection (
Browse files Browse the repository at this point in the history
#11301)

* feat(discovery-client): robust robot models

* refactor(discovery-client): coalesce robot model data from health and mdns
As of #11295 , robots
advertise their robot model in
- a TXT record attached to their MDNS advertisements
- the health response presented by the update server
in addition to the presentation in the health response from the robot
server.

The discovery client now harvests (and presents proper types for) all
this data and sends it upstream to whatever is listening, with a new
advertisedModel field for the MDNS response, and a proper shape for the
health data from the update server.

* refactor(app,app-shell): robot robot models from discovery

We previously could figure out whether a robot was an OT-2 or OT-3 based
on an element of the robot server health response, sent to us by the
discovery client. However, this isn't robust enough for all the
different places we display information about robots - somtimes we can't
get the health response but still want to display information.

After the previous commit, we now also have robot model information in
the server health response, if present; and in a per-address data blob
that comes from the MDNS advertisements.

We need to coalesce these three data sources into a single robust
presentation of the robot model, which we can put in the redux store and
add a selector for.

We coalesce the data by taking the first valid element from the
prioritized list (/health, /server/update/health, mdns) where validity
means that the response is present; the key containing the model is
present; and the model is regex-valid to known models. If any element is
missing or not present, it is skipped. If no element is valid or
present, then for now we default to saying that the robot is an OT-2. In
the future, that might be a decision we want to make at a higher level.

refactor(app): display robot name in device pane

Based on the new robust robot model detection, properly display either 
"OT-2" or "OT-3" in the header bar of a device card and window.

Co-authored-by: Koji <[email protected]>
Co-authored-by: Mike Cousins <[email protected]>
  • Loading branch information
3 people authored Aug 15, 2022
1 parent 9d89b73 commit 8e6d032
Show file tree
Hide file tree
Showing 27 changed files with 776 additions and 168 deletions.
3 changes: 3 additions & 0 deletions app-shell/src/__tests__/discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ describe('app-shell/discovery', () => {
serverHealthStatus: null,
healthError: null,
serverHealthError: null,
advertisedModel: null,
},
{
ip: '169.254.92.130',
Expand All @@ -303,6 +304,7 @@ describe('app-shell/discovery', () => {
serverHealthStatus: null,
healthError: null,
serverHealthError: null,
advertisedModel: null,
},
{
ip: '169.254.239.127',
Expand All @@ -312,6 +314,7 @@ describe('app-shell/discovery', () => {
serverHealthStatus: null,
healthError: null,
serverHealthError: null,
advertisedModel: null,
},
],
},
Expand Down
1 change: 1 addition & 0 deletions app-shell/src/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const migrateLegacyServices = (
serverHealthStatus: null,
healthError: null,
serverHealthError: null,
advertisedModel: null,
},
]
: []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ describe('ChooseRobotSlideout', () => {
})
it('if selected robot is on a different version of the software than the app, disable CTA and show link to device details in options', () => {
when(mockGetBuildrootUpdateDisplayInfo)
.calledWith((undefined as any) as State, 'opentrons-robot-name')
.calledWith(({} as any) as State, 'opentrons-robot-name')
.mockReturnValue({
autoUpdateAction: 'upgrade',
autoUpdateDisabledReason: null,
Expand Down
14 changes: 11 additions & 3 deletions app/src/organisms/Devices/RobotCard.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react'
import { useSelector } from 'react-redux'
import { useTranslation } from 'react-i18next'
import { Link, useHistory } from 'react-router-dom'
import { css } from 'styled-components'
Expand All @@ -23,7 +24,11 @@ import { getModuleDisplayName } from '@opentrons/shared-data'
import OT2_PNG from '../../assets/images/OT2-R_HERO.png'
import { StyledText } from '../../atoms/text'
import { SecondaryTertiaryButton } from '../../atoms/buttons'
import { CONNECTABLE, UNREACHABLE } from '../../redux/discovery'
import {
CONNECTABLE,
UNREACHABLE,
getRobotModelByName,
} from '../../redux/discovery'
import { ModuleIcon } from '../../molecules/ModuleIcon'
import { useCurrentRunId } from '../../organisms/ProtocolUpload/hooks'
import { useCurrentRunStatus } from '../../organisms/RunTimeControl/hooks'
Expand All @@ -37,6 +42,7 @@ import { ReachableBanner } from './ReachableBanner'
import { RobotOverflowMenu } from './RobotOverflowMenu'

import type { DiscoveredRobot } from '../../redux/discovery/types'
import type { State } from '../../redux/types'

const ROBOT_CARD_STYLE = css`
border: 1px solid ${COLORS.medGreyEnabled};
Expand Down Expand Up @@ -77,6 +83,9 @@ export function RobotCard(props: RobotCardProps): JSX.Element | null {
const { robot } = props
const { name: robotName = null, local } = robot
const history = useHistory()
const robotModel = useSelector((state: State) =>
getRobotModelByName(state, robot?.name)
)

return robotName != null ? (
<Flex
Expand Down Expand Up @@ -107,8 +116,7 @@ export function RobotCard(props: RobotCardProps): JSX.Element | null {
id={`RobotStatusBanner_${robotName}_robotModel`}
color={COLORS.darkGreyEnabled}
>
{/* robot_model can be seen in the health response, but only for "connectable" robots. Probably best to leave as "OT-2" for now */}
OT-2
{robotModel}
</StyledText>
<Flex alignItems={ALIGN_CENTER} paddingBottom={SPACING.spacing4}>
<Flex alignItems={ALIGN_CENTER}>
Expand Down
10 changes: 7 additions & 3 deletions app/src/organisms/Devices/RobotStatusBanner.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react'
import { useSelector } from 'react-redux'
import { useTranslation } from 'react-i18next'
import { Link } from 'react-router-dom'

Expand All @@ -18,8 +19,10 @@ import { StyledText } from '../../atoms/text'
import { useCurrentRunId } from '../../organisms/ProtocolUpload/hooks'
import { useCurrentRunStatus } from '../../organisms/RunTimeControl/hooks'
import { useProtocolDetailsForRun } from './hooks'
import { getRobotModelByName } from '../../redux/discovery'

import type { DiscoveredRobot } from '../../redux/discovery/types'
import type { State } from '../../redux/types'

type RobotStatusBannerProps = Pick<DiscoveredRobot, 'name' | 'local'>

Expand All @@ -30,6 +33,9 @@ export function RobotStatusBanner(props: RobotStatusBannerProps): JSX.Element {
const currentRunId = useCurrentRunId()
const currentRunStatus = useCurrentRunStatus()
const { displayName } = useProtocolDetailsForRun(currentRunId)
const robotModel = useSelector((state: State) =>
getRobotModelByName(state, name)
)

const runningProtocolBanner: JSX.Element | null =
currentRunId != null ? (
Expand All @@ -54,16 +60,14 @@ export function RobotStatusBanner(props: RobotStatusBannerProps): JSX.Element {

return (
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN}>
{/* robot_model can be seen in the health response, but only for "connectable" robots.
Probably best to leave as "OT-2" for now */}
<Flex flexDirection={DIRECTION_COLUMN}>
<StyledText
as="h6"
color={COLORS.darkGreyEnabled}
paddingBottom={SPACING.spacing1}
id={`RobotStatusBanner_${name}_robotModel`}
>
OT-2
{robotModel}
</StyledText>
<Flex alignItems={ALIGN_CENTER} paddingBottom={SPACING.spacing4}>
<Flex alignItems={ALIGN_CENTER}>
Expand Down
95 changes: 85 additions & 10 deletions app/src/organisms/Devices/__tests__/RobotCard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import { when, resetAllWhenMocks } from 'jest-when'
import { renderWithProviders } from '@opentrons/components'
import { RUN_STATUS_RUNNING } from '@opentrons/api-client'
import _uncastedSimpleV6Protocol from '@opentrons/shared-data/protocol/fixtures/6/simpleV6.json'
import {
mockOT2HealthResponse,
mockOT2ServerHealthResponse,
mockOT3HealthResponse,
mockOT3ServerHealthResponse,
} from '@opentrons/discovery-client/src/__fixtures__'

import { i18n } from '../../../i18n'
import { mockFetchModulesSuccessActionPayloadModules } from '../../../redux/modules/__fixtures__'
Expand All @@ -14,6 +20,12 @@ import {
} from '../../../redux/pipettes/__fixtures__'
import { mockConnectableRobot } from '../../../redux/discovery/__fixtures__'
import { getBuildrootUpdateDisplayInfo } from '../../../redux/buildroot'
import { getRobotModelByName } from '../../../redux/discovery'
import {
HEALTH_STATUS_OK,
ROBOT_MODEL_OT2,
ROBOT_MODEL_OT3,
} from '../../../redux/discovery/constants'
import {
useAttachedModules,
useAttachedPipettes,
Expand All @@ -26,8 +38,10 @@ import { UpdateRobotBanner } from '../../UpdateRobotBanner'
import { RobotCard } from '../RobotCard'

import type { ProtocolAnalysisFile } from '@opentrons/shared-data'
import type { State } from '../../../redux/types'

jest.mock('../../../redux/buildroot/selectors')
jest.mock('../../../redux/discovery/selectors')
jest.mock('../../../organisms/ProtocolUpload/hooks')
jest.mock('../../../organisms/RunTimeControl/hooks')
jest.mock('../../ProtocolUpload/hooks')
Expand All @@ -36,6 +50,47 @@ jest.mock('../../UpdateRobotBanner')
jest.mock('../../ChooseProtocolSlideout')

const OT2_PNG_FILE_NAME = 'OT2-R_HERO.png'
const MOCK_STATE: State = {
discovery: {
robot: { connection: { connectedTo: null } },
robotsByName: {
'opentrons-robot-name': {
name: 'opentrons-robot-name',
health: mockOT2HealthResponse,
serverHealth: mockOT2ServerHealthResponse,
addresses: [
{
ip: '10.0.0.3',
port: 31950,
seen: true,
healthStatus: HEALTH_STATUS_OK,
serverHealthStatus: HEALTH_STATUS_OK,
healthError: null,
serverHealthError: null,
advertisedModel: ROBOT_MODEL_OT2,
},
],
},
buzz: {
name: 'buzz',
health: mockOT3HealthResponse,
serverHealth: mockOT3ServerHealthResponse,
addresses: [
{
ip: '10.0.0.4',
port: 31950,
seen: true,
healthStatus: HEALTH_STATUS_OK,
serverHealthStatus: HEALTH_STATUS_OK,
healthError: null,
serverHealthError: null,
advertisedModel: ROBOT_MODEL_OT3,
},
],
},
},
},
} as any

const mockUseCurrentRunId = useCurrentRunId as jest.MockedFunction<
typeof useCurrentRunId
Expand All @@ -61,6 +116,9 @@ const mockUpdateRobotBanner = UpdateRobotBanner as jest.MockedFunction<
const mockGetBuildrootUpdateDisplayInfo = getBuildrootUpdateDisplayInfo as jest.MockedFunction<
typeof getBuildrootUpdateDisplayInfo
>
const mockGetRobotModelByName = getRobotModelByName as jest.MockedFunction<
typeof getRobotModelByName
>

const simpleV6Protocol = (_uncastedSimpleV6Protocol as unknown) as ProtocolAnalysisFile<{}>
const PROTOCOL_DETAILS = {
Expand All @@ -69,19 +127,23 @@ const PROTOCOL_DETAILS = {
protocolKey: 'fakeProtocolKey',
}

const render = () => {
const render = (props: React.ComponentProps<typeof RobotCard>) => {
return renderWithProviders(
<MemoryRouter>
<RobotCard robot={mockConnectableRobot} />
<RobotCard {...props} />
</MemoryRouter>,
{
i18nInstance: i18n,
initialState: MOCK_STATE,
}
)
}

describe('RobotCard', () => {
let props: React.ComponentProps<typeof RobotCard>

beforeEach(() => {
props = { robot: mockConnectableRobot }
mockUseAttachedModules.mockReturnValue(
mockFetchModulesSuccessActionPayloadModules
)
Expand Down Expand Up @@ -109,26 +171,29 @@ describe('RobotCard', () => {
protocolData: {} as ProtocolAnalysisFile<{}>,
protocolKey: null,
})
when(mockGetRobotModelByName)
.calledWith(MOCK_STATE, mockConnectableRobot.name)
.mockReturnValue('OT-2')
})
afterEach(() => {
jest.resetAllMocks()
resetAllWhenMocks()
})

it('renders an OT image', () => {
const [{ getByRole }] = render()
const [{ getByRole }] = render(props)
const image = getByRole('img')

expect(image.getAttribute('src')).toEqual(OT2_PNG_FILE_NAME)
})

it('renders a UpdateRobotBanner component', () => {
const [{ getByText }] = render()
const [{ getByText }] = render(props)
getByText('Mock UpdateRobotBanner')
})

it('renders the type of pipettes attached to left and right mounts', () => {
const [{ getByText }] = render()
const [{ getByText }] = render(props)

getByText('Left Mount')
getByText('Left Pipette')
Expand All @@ -137,19 +202,29 @@ describe('RobotCard', () => {
})

it('renders a modules section', () => {
const [{ getByText }] = render()
const [{ getByText }] = render(props)

getByText('Modules')
})

it('renders the type of robot and robot name', () => {
const [{ getByText }] = render()
it('renders the model of robot and robot name - OT-2', () => {
const [{ getByText }] = render(props)
getByText('OT-2')
getByText(mockConnectableRobot.name)
})

it('renders the model of robot and robot name - OT-3', () => {
props = { robot: { ...mockConnectableRobot, name: 'buzz' } }
when(mockGetRobotModelByName)
.calledWith(MOCK_STATE, 'buzz')
.mockReturnValue('OT-3')
const [{ getByText }] = render(props)
getByText('OT-3')
getByText('buzz')
})

it('does not render a running protocol banner when a protocol is not running', () => {
const [{ queryByText }] = render()
const [{ queryByText }] = render(props)

expect(queryByText('Testosaur;')).toBeFalsy()
expect(queryByText('Go to Run')).toBeFalsy()
Expand All @@ -164,7 +239,7 @@ describe('RobotCard', () => {
.calledWith('1')
.mockReturnValue(PROTOCOL_DETAILS)

const [{ getByRole, getByText }] = render()
const [{ getByRole, getByText }] = render(props)

getByText('Testosaur; Running')

Expand Down
Loading

0 comments on commit 8e6d032

Please sign in to comment.