refactor(app-shell): Migrate desktop app notify connectivity to disco…

…very-client (#14648)

Closes EXEC-304

app-shell/src/__fixtures__/index.ts

@@ -1 +1,2 @@
 export * from './config'
+export * from './robots'

app-shell/src/__fixtures__/robots.ts

@@ -0,0 +1,123 @@
+import { HEALTH_STATUS_NOT_OK, HEALTH_STATUS_OK } from '../constants'
+
+export const mockLegacyHealthResponse = {
+  name: 'opentrons-dev',
+  api_version: '1.2.3',
+  fw_version: '4.5.6',
+  system_version: '7.8.9',
+  robot_model: 'OT-2 Standard',
+}
+
+export const mockLegacyServerHealthResponse = {
+  name: 'opentrons-dev',
+  apiServerVersion: '1.2.3',
+  serialNumber: '12345',
+  updateServerVersion: '1.2.3',
+  smoothieVersion: '4.5.6',
+  systemVersion: '7.8.9',
+}
+
+export const MOCK_DISCOVERY_ROBOTS = [
+  {
+    name: 'opentrons-dev',
+    health: mockLegacyHealthResponse,
+    serverHealth: mockLegacyServerHealthResponse,
+    addresses: [
+      {
+        ip: '10.14.19.50',
+        port: 31950,
+        seen: true,
+        healthStatus: HEALTH_STATUS_OK,
+        serverHealthStatus: HEALTH_STATUS_OK,
+        healthError: null,
+        serverHealthError: null,
+        advertisedModel: null,
+      },
+    ],
+  },
+  {
+    name: 'opentrons-dev2',
+    health: mockLegacyHealthResponse,
+    serverHealth: mockLegacyServerHealthResponse,
+    addresses: [
+      {
+        ip: '10.14.19.51',
+        port: 31950,
+        seen: true,
+        healthStatus: HEALTH_STATUS_OK,
+        serverHealthStatus: HEALTH_STATUS_OK,
+        healthError: null,
+        serverHealthError: null,
+        advertisedModel: null,
+      },
+    ],
+  },
+  {
+    name: 'opentrons-dev3',
+    health: mockLegacyHealthResponse,
+    serverHealth: mockLegacyServerHealthResponse,
+    addresses: [
+      {
+        ip: '10.14.19.52',
+        port: 31950,
+        seen: true,
+        healthStatus: HEALTH_STATUS_NOT_OK,
+        serverHealthStatus: HEALTH_STATUS_NOT_OK,
+        healthError: null,
+        serverHealthError: null,
+        advertisedModel: null,
+      },
+    ],
+  },
+  {
+    name: 'opentrons-dev4',
+    health: mockLegacyHealthResponse,
+    serverHealth: mockLegacyServerHealthResponse,
+    addresses: [
+      {
+        ip: '10.14.19.53',
+        port: 31950,
+        seen: true,
+        healthStatus: HEALTH_STATUS_OK,
+        serverHealthStatus: HEALTH_STATUS_OK,
+        healthError: null,
+        serverHealthError: null,
+        advertisedModel: null,
+      },
+    ],
+  },
+]
+
+export const MOCK_STORE_ROBOTS = [
+  {
+    robotName: 'opentrons-dev',
+    ip: '10.14.19.50',
+  },
+  {
+    robotName: 'opentrons-dev2',
+    ip: '10.14.19.51',
+  },
+  {
+    robotName: 'opentrons-dev3',
+    ip: '10.14.19.52',
+  },
+  {
+    robotName: 'opentrons-dev4',
+    ip: '10.14.19.53',
+  },
+]
+
+export const MOCK_HEALTHY_ROBOTS = [
+  {
+    robotName: 'opentrons-dev',
+    ip: '10.14.19.50',
+  },
+  {
+    robotName: 'opentrons-dev2',
+    ip: '10.14.19.51',
+  },
+  {
+    robotName: 'opentrons-dev4',
+    ip: '10.14.19.53',
+  },
+]

app-shell/src/__tests__/discovery.test.ts

@@ -27,6 +27,7 @@ vi.mock('../log', () => {
     },
   }
 })
+vi.mock('../notifications')
 
 let mockGet = vi.fn(property => {
   return []

app-shell/src/config/actions.ts

@@ -76,7 +76,6 @@ import {
   VALUE_UPDATED,
   VIEW_PROTOCOL_SOURCE_FOLDER,
   NOTIFY_SUBSCRIBE,
-  NOTIFY_UNSUBSCRIBE,
   ROBOT_MASS_STORAGE_DEVICE_ADDED,
   ROBOT_MASS_STORAGE_DEVICE_ENUMERATED,
   ROBOT_MASS_STORAGE_DEVICE_REMOVED,
@@ -99,7 +98,6 @@ import type {
   AppRestartAction,
   NotifySubscribeAction,
   NotifyTopic,
-  NotifyUnsubscribeAction,
   ReloadUiAction,
   RobotMassStorageDeviceAdded,
   RobotMassStorageDeviceEnumerated,
@@ -421,15 +419,3 @@ export const notifySubscribeAction = (
   },
   meta: { shell: true },
 })
-
-export const notifyUnsubscribeAction = (
-  hostname: string,
-  topic: NotifyTopic
-): NotifyUnsubscribeAction => ({
-  type: NOTIFY_UNSUBSCRIBE,
-  payload: {
-    hostname,
-    topic,
-  },
-  meta: { shell: true },
-})

app-shell/src/constants.ts

@@ -225,8 +225,6 @@ export const ROBOT_MASS_STORAGE_DEVICE_ENUMERATED: 'shell:ROBOT_MASS_STORAGE_DEV
   'shell:ROBOT_MASS_STORAGE_DEVICE_ENUMERATED'
 export const NOTIFY_SUBSCRIBE: 'shell:NOTIFY_SUBSCRIBE' =
   'shell:NOTIFY_SUBSCRIBE'
-export const NOTIFY_UNSUBSCRIBE: 'shell:NOTIFY_UNSUBSCRIBE' =
-  'shell:NOTIFY_UNSUBSCRIBE'
 
 // copy
 // TODO(mc, 2020-05-11): i18n
@@ -247,3 +245,9 @@ export const DISCOVERY_UPDATE_LIST: DISCOVERY_UPDATE_LIST_TYPE =
 export const DISCOVERY_REMOVE: DISCOVERY_REMOVE_TYPE = 'discovery:REMOVE'
 
 export const CLEAR_CACHE: CLEAR_CACHE_TYPE = 'discovery:CLEAR_CACHE'
+export const HEALTH_STATUS_OK: 'ok' = 'ok'
+export const HEALTH_STATUS_NOT_OK: 'notOk' = 'notOk'
+export const FAILURE_STATUSES = {
+  ECONNREFUSED: 'ECONNREFUSED',
+  ECONNFAILED: 'ECONNFAILED',
+} as const

app-shell/src/discovery.ts

@@ -9,27 +9,29 @@ import {
   DEFAULT_PORT,
 } from '@opentrons/discovery-client'
 import {
-  CLEAR_CACHE,
-  DISCOVERY_FINISH,
-  DISCOVERY_REMOVE,
-  DISCOVERY_START,
-  OPENTRONS_USB,
   UI_INITIALIZED,
   USB_HTTP_REQUESTS_START,
   USB_HTTP_REQUESTS_STOP,
-} from './constants'
+} from '@opentrons/app/src/redux/shell/actions'
+import {
+  DISCOVERY_START,
+  DISCOVERY_FINISH,
+  DISCOVERY_REMOVE,
+  CLEAR_CACHE,
+} from '@opentrons/app/src/redux/discovery/actions'
+import { OPENTRONS_USB } from '@opentrons/app/src/redux/discovery/constants'
 
 import { getFullConfig, handleConfigChange } from './config'
 import { createLogger } from './log'
 import { getSerialPortHttpAgent } from './usb'
+import { handleNotificationConnectionsFor } from './notifications'
 
 import type {
   Address,
   DiscoveryClientRobot,
   LegacyService,
   DiscoveryClient,
 } from '@opentrons/discovery-client'
-
 import type { Action, Dispatch } from './types'
 import type { ConfigV1 } from '@opentrons/app/src/redux/config/schema-types'
 
@@ -199,6 +201,7 @@ export function registerDiscovery(
 
   function handleRobots(): void {
     const robots = client.getRobots()
+    handleNotificationConnectionsFor(robots)
 
     if (!disableCache) store.set('robots', robots)
 

app-shell/src/main.ts

@@ -19,7 +19,7 @@ import { registerProtocolStorage } from './protocol-storage'
 import { getConfig, getStore, getOverrides, registerConfig } from './config'
 import { registerUsb } from './usb'
 import { createUsbDeviceMonitor } from './system-info/usb-devices'
-import { registerNotify, closeAllNotifyConnections } from './notify'
+import { registerNotify, closeAllNotifyConnections } from './notifications'
 
 import type { BrowserWindow } from 'electron'
 import type { Dispatch, Logger } from './types'

app-shell/src/notifications/__tests__/connect.test.ts

@@ -0,0 +1,115 @@
+import { vi, describe, expect, it } from 'vitest'
+
+import {
+  getHealthyRobotDataForNotifyConnections,
+  cleanUpUnreachableRobots,
+  establishConnections,
+  closeConnectionsForcefullyFor,
+} from '../connect'
+import { connectionStore } from '../store'
+import { FAILURE_STATUSES } from '../../constants'
+import {
+  MOCK_DISCOVERY_ROBOTS,
+  MOCK_HEALTHY_ROBOTS,
+  MOCK_STORE_ROBOTS,
+} from '../../__fixtures__'
+
+vi.mock('electron-store')
+vi.mock('../notifyLog', () => {
+  return {
+    createLogger: () => {
+      return { debug: () => null }
+    },
+    notifyLog: { debug: vi.fn(), warn: vi.fn() },
+  }
+})
+
+describe('getHealthyRobotDataForNotifyConnections', () => {
+  it('should filter a list of discovery robots, only returning robots that have a health status of ok', () => {
+    const healthyRobots = getHealthyRobotDataForNotifyConnections(
+      MOCK_DISCOVERY_ROBOTS
+    )
+    expect(healthyRobots).toEqual(MOCK_HEALTHY_ROBOTS)
+  })
+})
+
+describe('cleanUpUnreachableRobots', () => {
+  it('should close connections forcefully for unreachable robots and resolve them', async () => {
+    MOCK_STORE_ROBOTS.forEach(robot => {
+      void connectionStore
+        .setPendingConnection(robot.robotName)
+        .then(() =>
+          connectionStore.setConnected(robot.robotName, vi.fn() as any)
+        )
+    })
+    const unreachableRobots = await cleanUpUnreachableRobots(
+      MOCK_HEALTHY_ROBOTS
+    )
+    expect(unreachableRobots).toEqual(['opentrons-dev3'])
+  })
+})
+
+describe('establishConnections', () => {
+  it('should not resolve any new connections if all reported robots are already in the connection store and connected', async () => {
+    connectionStore.clearStore()
+    MOCK_STORE_ROBOTS.forEach(robot => {
+      void connectionStore
+        .setPendingConnection(robot.robotName)
+        .then(() =>
+          connectionStore.setConnected(robot.robotName, vi.fn() as any)
+        )
+    })
+
+    const newRobots = await establishConnections(MOCK_HEALTHY_ROBOTS)
+    expect(newRobots).toEqual([])
+  })
+
+  it('should not attempt to connect to a robot if it a known notification port blocked robot', async () => {
+    await connectionStore.setErrorStatus(
+      '10.14.19.51',
+      FAILURE_STATUSES.ECONNREFUSED
+    )
+    connectionStore.clearStore()
+
+    const newRobots = await establishConnections(MOCK_HEALTHY_ROBOTS)
+    expect(newRobots).toEqual([
+      { ip: '10.14.19.50', robotName: 'opentrons-dev' },
+      { ip: '10.14.19.53', robotName: 'opentrons-dev4' },
+    ])
+  })
+
+  it('should not report a robot as new if it is connecting', async () => {
+    connectionStore.clearStore()
+    MOCK_STORE_ROBOTS.forEach(robot => {
+      void connectionStore.setPendingConnection(robot.robotName)
+    })
+
+    const newRobots = await establishConnections(MOCK_HEALTHY_ROBOTS)
+    expect(newRobots).toEqual([])
+  })
+
+  it('should create a new entry in the connection store for a new robot', async () => {
+    connectionStore.clearStore()
+    await establishConnections(MOCK_HEALTHY_ROBOTS)
+    console.log(connectionStore)
+    expect(connectionStore.getRobotNameByIP('10.14.19.50')).not.toBeNull()
+  })
+})
+
+describe('closeConnectionsForcefullyFor', () => {
+  it('should return an array of promises for each closing connection and resolve after closing connections', async () => {
+    connectionStore.clearStore()
+    MOCK_STORE_ROBOTS.forEach(robot => {
+      void connectionStore
+        .setPendingConnection(robot.robotName)
+        .then(() =>
+          connectionStore.setConnected(robot.robotName, vi.fn() as any)
+        )
+    })
+    const closingRobots = closeConnectionsForcefullyFor([
+      'opentrons-dev',
+      'opentrons-dev2',
+    ])
+    closingRobots.forEach(robot => expect(robot).toBeInstanceOf(Promise))
+  })
+})

app-shell/src/notifications/__tests__/deserialize.test.ts

@@ -0,0 +1,33 @@
+import { describe, expect, it } from 'vitest'
+
+import { deserializeExpectedMessages } from '../deserialize'
+
+import type { NotifyResponseData } from '@opentrons/app/src/redux/shell/types'
+
+const MOCK_VALID_RESPONSE: NotifyResponseData = { refetchUsingHTTP: true }
+const MOCK_VALID_STRING_RESPONSE = JSON.stringify(MOCK_VALID_RESPONSE)
+const MOCK_INVALID_OBJECT = JSON.stringify({ test: 'MOCK_RESPONSE' })
+const MOCK_INVALID_STRING = 'MOCK_STRING'
+
+describe('closeConnectionsForcefullyFor', () => {
+  it('should resolve with the deserialized message if it is a valid notify response', async () => {
+    const response = await deserializeExpectedMessages(
+      MOCK_VALID_STRING_RESPONSE
+    )
+    expect(response).toEqual(MOCK_VALID_RESPONSE)
+  })
+
+  it('should reject with an error if the deserialized message is not a valid notify response', async () => {
+    const responsePromise = deserializeExpectedMessages(MOCK_INVALID_OBJECT)
+    await expect(responsePromise).rejects.toThrowError(
+      'Unexpected data received from notify broker: {"test":"MOCK_RESPONSE"}'
+    )
+  })
+
+  it('should reject with an error if the message cannot be deserialized', async () => {
+    const responsePromise = deserializeExpectedMessages(MOCK_INVALID_STRING)
+    await expect(responsePromise).rejects.toThrowError(
+      'Unexpected data received from notify broker: MOCK_STRING'
+    )
+  })
+})