Skip to content

Commit

Permalink
fix(app): show robot network connection in app if able to communicate…
Browse files Browse the repository at this point in the history 
… via that connection (#13206)

adds a check to ensure a connected network status ip reported by a robot has a healthy response from
the app discovery-client. for example, robots connected to a different wifi network than the client
app should not show a wifi connection in the app, because the app cannot communicate with that robot
via wifi.

closes RQA-1150
  • Loading branch information
@brenthagen
brenthagen authored Aug 2, 2023
1 parent 1eb456a commit 008cae1
Show file tree
Hide file tree
Showing 4 changed files with 191 additions and 27 deletions.
33 changes: 22 additions & 11 deletions app/src/organisms/Devices/RobotSettings/RobotSettingsNetworking.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,31 @@ export function RobotSettingsNetworking({

const canDisconnect = useCanDisconnect(robotName)

const addresses = useSelector((state: State) =>
getRobotAddressesByName(state, robotName)
)
const usbAddress = addresses.find(addr => addr.ip === OPENTRONS_USB)
const isOT3ConnectedViaUSB =
usbAddress != null && usbAddress.healthStatus === HEALTH_STATUS_OK

const { wifi, ethernet } = useSelector((state: State) =>
getNetworkInterfaces(state, robotName)
)
const activeNetwork = wifiList?.find(network => network.active)

const ssid = activeNetwork?.ssid ?? null

const addresses = useSelector((state: State) =>
getRobotAddressesByName(state, robotName)
)

const wifiAddress = addresses.find(addr => addr.ip === wifi?.ipAddress)
const isOT3ConnectedViaWifi =
wifiAddress != null && wifiAddress.healthStatus === HEALTH_STATUS_OK

const ethernetAddress = addresses.find(
addr => addr.ip === ethernet?.ipAddress
)
const isOT3ConnectedViaEthernet =
ethernetAddress != null && ethernetAddress.healthStatus === HEALTH_STATUS_OK

const usbAddress = addresses.find(addr => addr.ip === OPENTRONS_USB)
const isOT3ConnectedViaUSB =
usbAddress != null && usbAddress.healthStatus === HEALTH_STATUS_OK

useInterval(() => dispatch(fetchStatus(robotName)), STATUS_REFRESH_MS, true)

React.useEffect(() => {
Expand All @@ -95,7 +106,7 @@ export function RobotSettingsNetworking({
</Portal>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing16}>
<Flex alignItems={ALIGN_CENTER}>
{wifi?.ipAddress != null ? (
{isOT3ConnectedViaWifi ? (
<Icon
size="1.25rem"
name="ot-check"
Expand All @@ -120,7 +131,7 @@ export function RobotSettingsNetworking({
</StyledText>
</Flex>
<Box paddingLeft="3.75rem">
{wifi?.ipAddress != null ? (
{isOT3ConnectedViaWifi ? (
<>
<Flex marginBottom={SPACING.spacing24}>
<Flex marginRight={SPACING.spacing8}>
Expand Down Expand Up @@ -180,7 +191,7 @@ export function RobotSettingsNetworking({
</Box>
<Divider />
<Flex alignItems={ALIGN_CENTER}>
{ethernet?.ipAddress != null ? (
{isOT3ConnectedViaEthernet ? (
<Icon
size="1.25rem"
name="ot-check"
Expand All @@ -203,7 +214,7 @@ export function RobotSettingsNetworking({
</Flex>
<Box paddingLeft="3.75rem">
<Flex gridGap={SPACING.spacing16}>
{ethernet?.ipAddress != null ? (
{isOT3ConnectedViaEthernet ? (
<>
<Flex
flexDirection={DIRECTION_COLUMN}
Expand Down
68 changes: 66 additions & 2 deletions app/src/organisms/Devices/RobotSettings/__tests__/RobotSettingsNetworking.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { i18n } from '../../../../i18n'
import {
getRobotAddressesByName,
HEALTH_STATUS_OK,
HEALTH_STATUS_NOT_OK,
OPENTRONS_USB,
} from '../../../../redux/discovery'
import * as Networking from '../../../../redux/networking'
Expand Down Expand Up @@ -92,7 +93,16 @@ describe('RobotSettingsNetworking', () => {
beforeEach(() => {
when(mockGetRobotAddressesByName)
.calledWith({} as State, ROBOT_NAME)
.mockReturnValue([])
.mockReturnValue([
{
ip: initialMockWifi.ipAddress,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
{
ip: initialMockEthernet.ipAddress,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
])
when(mockGetNetworkInterfaces)
.calledWith({} as State, ROBOT_NAME)
.mockReturnValue({
Expand Down Expand Up @@ -197,7 +207,14 @@ describe('RobotSettingsNetworking', () => {
when(mockGetNetworkInterfaces)
.calledWith({} as State, ROBOT_NAME)
.mockReturnValue({ wifi: mockWiFi, ethernet: null })

when(mockGetRobotAddressesByName)
.calledWith({} as State, ROBOT_NAME)
.mockReturnValue([
{
ip: mockWiFi.ipAddress,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
])
const [{ getByText, getByTestId, queryByText, queryAllByTestId }] = render()
getByText('Wi-Fi - foo')
getByText('Wireless IP')
Expand Down Expand Up @@ -231,6 +248,14 @@ describe('RobotSettingsNetworking', () => {
wifi: null,
ethernet: mockWiredUSB,
})
when(mockGetRobotAddressesByName)
.calledWith({} as State, ROBOT_NAME)
.mockReturnValue([
{
ip: mockWiredUSB.ipAddress,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
])
when(mockUseWifiList).calledWith(ROBOT_NAME).mockReturnValue([])
const [{ getByText, getByTestId, queryAllByTestId }] = render()

Expand Down Expand Up @@ -305,4 +330,43 @@ describe('RobotSettingsNetworking', () => {

expect(queryByRole('button', { name: 'Disconnect from Wi-Fi' })).toBeNull()
})

it('should not render Wi-Fi mock data and ethernet mock data when discovery client cannot find a healthy robot at its network connection ip addresses', () => {
when(mockUseWifiList).calledWith(ROBOT_NAME).mockReturnValue(mockWifiList)
when(mockGetRobotAddressesByName)
.calledWith({} as State, ROBOT_NAME)
.mockReturnValue([
{
ip: 'some-other-ip',
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
{
ip: initialMockEthernet.ipAddress,
healthStatus: HEALTH_STATUS_NOT_OK,
} as DiscoveryClientRobotAddress,
])
const [{ getByText, getByTestId, queryByText, queryAllByTestId }] = render()
getByText('Wi-Fi - foo')
getByText('Wired USB')
expect(queryByText('Wireless IP')).toBeNull()
expect(queryByText('Wireless Subnet Mask')).toBeNull()
expect(queryByText('Wireless MAC Address')).toBeNull()
expect(queryByText('127.0.0.100')).toBeNull()
expect(queryByText('255.255.255.230')).toBeNull()
expect(queryByText('WI:FI:00:00:00:00')).toBeNull()
expect(queryByText('Wired IP')).toBeNull()
expect(queryByText('Wired Subnet Mask')).toBeNull()
expect(queryByText('Wired MAC Address')).toBeNull()
expect(queryByText('127.0.0.101')).toBeNull()
expect(queryByText('255.255.255.231')).toBeNull()
expect(queryByText('US:B0:00:00:00:00')).toBeNull()
expect(queryByText('Wi-Fi - bar')).not.toBeInTheDocument()
expect(
getByTestId('RobotSettings_Networking_wifi_icon')
).toBeInTheDocument()
expect(getByTestId('RobotSettings_Networking_usb_icon')).toBeInTheDocument()
expect(
queryAllByTestId('RobotSettings_Networking_check_circle')
).toHaveLength(0)
})
})
29 changes: 26 additions & 3 deletions app/src/organisms/Devices/RobotStatusHeader.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import { StyledText } from '../../atoms/text'
import { Tooltip } from '../../atoms/Tooltip'
import { useCurrentRunId } from '../../organisms/ProtocolUpload/hooks'
import { useCurrentRunStatus } from '../../organisms/RunTimeControl/hooks'
import {
getRobotAddressesByName,
HEALTH_STATUS_OK,
OPENTRONS_USB,
} from '../../redux/discovery'
import { getNetworkInterfaces, fetchStatus } from '../../redux/networking'

import type { IconName, StyleProps } from '@opentrons/components'
Expand Down Expand Up @@ -84,15 +89,33 @@ export function RobotStatusHeader(props: RobotStatusHeaderProps): JSX.Element {
getNetworkInterfaces(state, name)
)

const addresses = useSelector((state: State) =>
getRobotAddressesByName(state, name)
)

const wifiAddress = addresses.find(addr => addr.ip === wifi?.ipAddress)
const isOT3ConnectedViaWifi =
wifiAddress != null && wifiAddress.healthStatus === HEALTH_STATUS_OK

const ethernetAddress = addresses.find(
addr => addr.ip === ethernet?.ipAddress
)
const isOT3ConnectedViaEthernet =
ethernetAddress != null && ethernetAddress.healthStatus === HEALTH_STATUS_OK

const usbAddress = addresses.find(addr => addr.ip === OPENTRONS_USB)
const isOT3ConnectedViaUSB =
usbAddress != null && usbAddress.healthStatus === HEALTH_STATUS_OK

let iconName: IconName | null = null
let tooltipTranslationKey = null
if (wifi?.ipAddress != null) {
if (isOT3ConnectedViaWifi) {
iconName = 'wifi'
tooltipTranslationKey = 'device_settings:wifi'
} else if (ethernet?.ipAddress != null) {
} else if (isOT3ConnectedViaEthernet) {
iconName = 'ethernet'
tooltipTranslationKey = 'device_settings:ethernet'
} else if (local != null && local) {
} else if ((local != null && local) || isOT3ConnectedViaUSB) {
iconName = 'usb'
tooltipTranslationKey = 'device_settings:wired_usb'
}
Expand Down
88 changes: 77 additions & 11 deletions app/src/organisms/Devices/__tests__/RobotStatusHeader.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,23 @@ import { useProtocolQuery, useRunQuery } from '@opentrons/react-api-client'
import { i18n } from '../../../i18n'
import { useCurrentRunId } from '../../../organisms/ProtocolUpload/hooks'
import { useCurrentRunStatus } from '../../../organisms/RunTimeControl/hooks'
import {
getRobotAddressesByName,
HEALTH_STATUS_OK,
OPENTRONS_USB,
} from '../../../redux/discovery'
import { getNetworkInterfaces } from '../../../redux/networking'

import { RobotStatusHeader } from '../RobotStatusHeader'

import type { DiscoveryClientRobotAddress } from '../../../redux/discovery/types'
import type { SimpleInterfaceStatus } from '../../../redux/networking/types'
import type { State } from '../../../redux/types'

jest.mock('@opentrons/react-api-client')
jest.mock('../../../organisms/ProtocolUpload/hooks')
jest.mock('../../../organisms/RunTimeControl/hooks')
jest.mock('../../../redux/discovery')
jest.mock('../../../redux/networking')
jest.mock('../hooks')

Expand All @@ -35,6 +42,23 @@ const mockUseRunQuery = useRunQuery as jest.MockedFunction<typeof useRunQuery>
const mockGetNetworkInterfaces = getNetworkInterfaces as jest.MockedFunction<
typeof getNetworkInterfaces
>
const mockGetRobotAddressesByName = getRobotAddressesByName as jest.MockedFunction<
typeof getRobotAddressesByName
>

const MOCK_OTIE = {
name: 'otie',
local: true,
robotModel: 'OT-2',
}
const MOCK_BUZZ = {
name: 'buzz',
local: true,
robotModel: 'Opentrons Flex',
}

const WIFI_IP = 'wifi-ip'
const ETHERNET_IP = 'ethernet-ip'

const render = (props: React.ComponentProps<typeof RobotStatusHeader>) => {
return renderWithProviders(
Expand All @@ -50,11 +74,7 @@ describe('RobotStatusHeader', () => {
let props: React.ComponentProps<typeof RobotStatusHeader>

beforeEach(() => {
props = {
name: 'otie',
local: true,
robotModel: 'OT-2',
}
props = MOCK_OTIE
when(mockUseCurrentRunId).calledWith().mockReturnValue(null)
when(mockUseCurrentRunStatus).calledWith().mockReturnValue(null)
when(mockUseRunQuery)
Expand Down Expand Up @@ -82,6 +102,22 @@ describe('RobotStatusHeader', () => {
when(mockGetNetworkInterfaces)
.calledWith({} as State, 'otie')
.mockReturnValue({ wifi: null, ethernet: null })
when(mockGetRobotAddressesByName)
.calledWith({} as State, 'otie')
.mockReturnValue([
{
ip: WIFI_IP,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
{
ip: ETHERNET_IP,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
{
ip: OPENTRONS_USB,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
])
})
afterEach(() => {
resetAllWhenMocks()
Expand All @@ -94,12 +130,26 @@ describe('RobotStatusHeader', () => {
})

it('renders the model of robot and robot name - OT-3', () => {
props.name = 'buzz'
props.robotModel = 'Opentrons Flex'
when(mockGetNetworkInterfaces)
.calledWith({} as State, 'buzz')
.mockReturnValue({ wifi: null, ethernet: null })
const [{ getByText }] = render(props)
when(mockGetRobotAddressesByName)
.calledWith({} as State, 'buzz')
.mockReturnValue([
{
ip: WIFI_IP,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
{
ip: ETHERNET_IP,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
{
ip: OPENTRONS_USB,
healthStatus: HEALTH_STATUS_OK,
} as DiscoveryClientRobotAddress,
])
const [{ getByText }] = render(MOCK_BUZZ)
getByText('Opentrons Flex')
getByText('buzz')
})
Expand Down Expand Up @@ -131,8 +181,8 @@ describe('RobotStatusHeader', () => {
when(mockGetNetworkInterfaces)
.calledWith({} as State, 'otie')
.mockReturnValue({
wifi: { ipAddress: 'wifi-ip' } as SimpleInterfaceStatus,
ethernet: { ipAddress: 'ethernet-ip' } as SimpleInterfaceStatus,
wifi: { ipAddress: WIFI_IP } as SimpleInterfaceStatus,
ethernet: { ipAddress: ETHERNET_IP } as SimpleInterfaceStatus,
})

const [{ getByLabelText }] = render(props)
Expand All @@ -145,7 +195,7 @@ describe('RobotStatusHeader', () => {
.calledWith({} as State, 'otie')
.mockReturnValue({
wifi: null,
ethernet: { ipAddress: 'ethernet-ip' } as SimpleInterfaceStatus,
ethernet: { ipAddress: ETHERNET_IP } as SimpleInterfaceStatus,
})

const [{ getByLabelText }] = render(props)
Expand All @@ -158,4 +208,20 @@ describe('RobotStatusHeader', () => {

getByLabelText('usb')
})

it('does not render a wifi or ethernet icon when discovery client cannot find a healthy robot at its network connection ip addresses', () => {
when(mockGetNetworkInterfaces)
.calledWith({} as State, 'otie')
.mockReturnValue({
wifi: { ipAddress: WIFI_IP } as SimpleInterfaceStatus,
ethernet: { ipAddress: ETHERNET_IP } as SimpleInterfaceStatus,
})
when(mockGetRobotAddressesByName)
.calledWith({} as State, 'otie')
.mockReturnValue([])
const [{ queryByLabelText }] = render(props)

expect(queryByLabelText('wifi')).toBeNull()
expect(queryByLabelText('ethernet')).toBeNull()
})
})

0 comments on commit 008cae1

Please sign in to comment.