feat(discovery-client): Add mdns flag and health responses to services (

#2420)

app-shell/Makefile

@@ -5,6 +5,9 @@ SHELL := /bin/bash
 # add node_modules/.bin to PATH
 PATH := $(shell cd .. && yarn bin):$(PATH)
 
+# dev server port
+PORT ?= 8090
+
 # source and output directories for main process code
 src_dir := src
 lib_dir := lib
@@ -49,7 +52,7 @@ electron := electron . \
 	--log.level.console="debug" \
 	--disable_ui.webPreferences.webSecurity \
 	--ui.url.protocol="http:" \
-	--ui.url.path="localhost:$(port)" \
+	--ui.url.path="localhost:$(PORT)" \
 	--discovery.candidates=localhost
 
 # standard targets

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

@@ -83,19 +83,11 @@ describe('app-shell/discovery', () => {
   })
 
   test('always sends "discovery:UPDATE_LIST" on "discovery:START"', () => {
-    mockClient.services = [
-      {name: 'opentrons-dev', ip: '192.168.1.42', port: 31950, ok: true},
-    ]
-
     const expected = [
-      {
-        name: 'opentrons-dev',
-        connections: [
-          {ip: '192.168.1.42', port: 31950, ok: true, local: false},
-        ],
-      },
+      {name: 'opentrons-dev', ip: '192.168.1.42', port: 31950, ok: true},
     ]
 
+    mockClient.services = expected
     registerDiscovery(dispatch)({type: 'discovery:START'})
     expect(dispatch).toHaveBeenCalledWith({
       type: 'discovery:UPDATE_LIST',
@@ -112,51 +104,6 @@ describe('app-shell/discovery', () => {
         services: [
           {name: 'opentrons-dev', ip: '192.168.1.42', port: 31950, ok: true},
         ],
-        expected: [
-          {
-            name: 'opentrons-dev',
-            connections: [
-              {ip: '192.168.1.42', port: 31950, ok: true, local: false},
-            ],
-          },
-        ],
-      },
-      {
-        name: 'handles multiple services',
-        services: [
-          {name: 'opentrons-1', ip: '192.168.1.42', port: 31950, ok: false},
-          {name: 'opentrons-2', ip: '169.254.9.8', port: 31950, ok: true},
-        ],
-        expected: [
-          {
-            name: 'opentrons-1',
-            connections: [
-              {ip: '192.168.1.42', port: 31950, ok: false, local: false},
-            ],
-          },
-          {
-            name: 'opentrons-2',
-            connections: [
-              {ip: '169.254.9.8', port: 31950, ok: true, local: true},
-            ],
-          },
-        ],
-      },
-      {
-        name: 'combines multiple services into one robot',
-        services: [
-          {name: 'opentrons-dev', ip: '192.168.1.42', port: 31950, ok: true},
-          {name: 'opentrons-dev', ip: '169.254.9.8', port: 31950, ok: true},
-        ],
-        expected: [
-          {
-            name: 'opentrons-dev',
-            connections: [
-              {ip: '192.168.1.42', port: 31950, ok: true, local: false},
-              {ip: '169.254.9.8', port: 31950, ok: true, local: true},
-            ],
-          },
-        ],
       },
     ]
 
@@ -167,7 +114,7 @@ describe('app-shell/discovery', () => {
         mockClient.emit('service')
         expect(dispatch).toHaveBeenCalledWith({
           type: 'discovery:UPDATE_LIST',
-          payload: {robots: spec.expected},
+          payload: {robots: spec.services},
         })
       })
     )

app-shell/src/discovery.js

@@ -1,11 +1,7 @@
 // @flow
 // app shell discovery module
-import assert from 'assert'
 import Store from 'electron-store'
-import groupBy from 'lodash/groupBy'
-import map from 'lodash/map'
 import throttle from 'lodash/throttle'
-import uniqBy from 'lodash/uniqBy'
 
 import DiscoveryClient, {
   SERVICE_EVENT,
@@ -19,10 +15,6 @@ import type {Service} from '@opentrons/discovery-client'
 
 // TODO(mc, 2018-08-08): figure out type exports from app
 import type {Action} from '@opentrons/app/src/types'
-import type {
-  DiscoveredRobot,
-  Connection,
-} from '@opentrons/app/src/discovery/types'
 
 const log = createLogger(__filename)
 
@@ -70,15 +62,15 @@ export function registerDiscovery (dispatch: Action => void) {
     store.set('services', filterServicesToPersist(client.services))
     dispatch({
       type: 'discovery:UPDATE_LIST',
-      payload: {robots: servicesToRobots(client.services)},
+      payload: {robots: client.services},
     })
   }
 }
 
 export function getRobots () {
   if (!client) return []
 
-  return servicesToRobots(client.services)
+  return client.services
 }
 
 function filterServicesToPersist (services: Array<Service>) {
@@ -88,40 +80,3 @@ function filterServicesToPersist (services: Array<Service>) {
   const blacklist = [].concat(candidateOverrides)
   return client.services.filter(s => blacklist.every(ip => ip !== s.ip))
 }
-
-// TODO(mc, 2018-08-09): exploring moving this to DiscoveryClient
-function servicesToRobots (services: Array<Service>): Array<DiscoveredRobot> {
-  const servicesByName = groupBy(services, 'name')
-
-  return map(servicesByName, (services: Array<Service>, name) => ({
-    name,
-    connections: servicesToConnections(services),
-  }))
-}
-
-function servicesToConnections (services: Array<Service>): Array<Connection> {
-  assert(uniqBy(services, 'name').length <= 1, 'services should have same name')
-
-  return services.map(serviceToConnection).filter(Boolean)
-}
-
-function serviceToConnection (service: Service): ?Connection {
-  if (!service.ip) return null
-
-  return {
-    ip: service.ip,
-    ok: service.ok,
-    port: service.port,
-    local: isLocal(service.ip),
-  }
-}
-
-function isLocal (ip: string): boolean {
-  // TODO(mc, 2018-08-09): remove `fd00` check for legacy IPv6 robots
-  return (
-    ip.startsWith('169.254') ||
-    ip.startsWith('[fe80') ||
-    ip.startsWith('[fd00') ||
-    ip === 'localhost'
-  )
-}

app/Makefile

@@ -5,14 +5,12 @@ SHELL := /bin/bash
 # add node_modules/.bin to PATH
 PATH := $(shell cd .. && yarn bin):$(PATH)
 
-# desktop shell directory for dev
-shell_dir := ../app-shell
-
-# set NODE_ENV for a command with $(env)=environment
-env := cross-env NODE_ENV
-
 # dev server port
-port ?= 8090
+PORT ?= 8090
+
+# dependency directories for dev
+shell_dir := ../app-shell
+discovery_client_dir = ../discovery-client
 
 # standard targets
 #####################################################################
@@ -32,23 +30,31 @@ clean:
 #####################################################################
 
 .PHONY: dist
+dist: export NODE_ENV := production
 dist:
-	$(env)=production webpack --profile
+	webpack --profile
 
 # development
 #####################################################################
 
 .PHONY: dev
-dev:
+dev: export NODE_ENV := development
+dev: export PORT := $(PORT)
+dev: dev-deps
 	concurrently --no-color --kill-others --names "server,shell" \
 		"$(MAKE) dev-server" \
 		"$(MAKE) dev-shell"
 
 .PHONY: dev-server
 dev-server:
-	$(env)=development PORT=$(port) webpack-dev-server --hot
+	webpack-dev-server --hot
 
 .PHONY: dev-shell
 dev-shell:
-	wait-on http-get://localhost:$(port) && \
-	$(MAKE) -C $(shell_dir) lib dev port=$(port)
+	wait-on http-get://localhost:$(PORT)
+	$(MAKE) -C $(shell_dir) dev
+
+.PHONY: dev-deps
+dev-deps:
+	$(MAKE) -C $(discovery_client_dir)
+	$(MAKE) -C $(shell_dir) lib

app/src/analytics/__tests__/make-event.test.js

@@ -25,14 +25,8 @@ describe('analytics events map', () => {
       },
       discovery: {
         robotsByName: {
-          wired: {
-            name: 'wired',
-            connections: [{ip: 'foo', port: 123, ok: true, local: true}],
-          },
-          wireless: {
-            name: 'wireless',
-            connections: [{ip: 'bar', port: 456, ok: true, local: false}],
-          },
+          wired: [{ip: 'foo', port: 123, ok: true, local: true}],
+          wireless: [{ip: 'bar', port: 456, ok: true, local: false}],
         },
       },
     })

app/src/discovery/__tests__/reducer.test.js

@@ -3,8 +3,8 @@ import {discoveryReducer} from '..'
 
 jest.mock('../../shell', () => ({
   getShellRobots: () => ([
-    {name: 'foo', connections: []},
-    {name: 'bar', connections: []},
+    {name: 'foo', ip: '192.168.1.1', port: 31950},
+    {name: 'bar', ip: '192.168.1.2', port: 31950},
   ]),
 }))
 
@@ -21,8 +21,8 @@ describe('discoveryReducer', () => {
       expectedState: {
         scanning: false,
         robotsByName: {
-          foo: {name: 'foo', connections: []},
-          bar: {name: 'bar', connections: []},
+          foo: [{name: 'foo', ip: '192.168.1.1', port: 31950}],
+          bar: [{name: 'bar', ip: '192.168.1.2', port: 31950}],
         },
       },
     },
@@ -44,16 +44,16 @@ describe('discoveryReducer', () => {
         type: 'discovery:UPDATE_LIST',
         payload: {
           robots: [
-            {name: 'foo', connections: []},
-            {name: 'bar', connections: []},
+            {name: 'foo', ip: '192.168.1.1', port: 31950},
+            {name: 'bar', ip: '192.168.1.2', port: 31950},
           ],
         },
       },
       initialState: {robotsByName: {}},
       expectedState: {
         robotsByName: {
-          foo: {name: 'foo', connections: []},
-          bar: {name: 'bar', connections: []},
+          foo: [{name: 'foo', ip: '192.168.1.1', port: 31950}],
+          bar: [{name: 'bar', ip: '192.168.1.2', port: 31950}],
         },
       },
     },

app/src/discovery/index.js

@@ -1,11 +1,12 @@
 // @flow
 // robot discovery state
+import groupBy from 'lodash/groupBy'
 import {getShellRobots} from '../shell'
 
+import type {Service} from '@opentrons/discovery-client'
 import type {State, Action, ThunkAction} from '../types'
-import type {DiscoveredRobot} from './types'
 
-type RobotsMap = {[name: string]: DiscoveredRobot}
+type RobotsMap = {[name: string]: Array<Service>}
 
 type DiscoveryState = {
   scanning: boolean,
@@ -24,11 +25,9 @@ type FinishAction = {|
 
 type UpdateListAction = {|
   type: 'discovery:UPDATE_LIST',
-  payload: {|robots: Array<DiscoveredRobot>|},
+  payload: {|robots: Array<Service>|},
 |}
 
-export * from './types'
-
 export type DiscoveryAction = StartAction | FinishAction | UpdateListAction
 
 const DISCOVERY_TIMEOUT = 20000
@@ -46,7 +45,7 @@ export function startDiscovery (): ThunkAction {
 // TODO(mc, 2018-08-09): uncomment when we figure out how to get this
 // to the app shell
 // export function updateDiscoveryList (
-//   robots: Array<DiscoveredRobot>
+//   robots: Array<Service>
 // ): UpdateListAction {
 //   return {type: 'discovery:UPDATE_LIST', payload: {robots}}
 // }
@@ -91,12 +90,6 @@ export function discoveryReducer (
   return state
 }
 
-function normalizeRobots (robots: Array<DiscoveredRobot> = []): RobotsMap {
-  return robots.reduce(
-    (robotsMap: RobotsMap, robot: DiscoveredRobot) => ({
-      ...robotsMap,
-      [robot.name]: robot,
-    }),
-    {}
-  )
+function normalizeRobots (robots: Array<Service> = []): RobotsMap {
+  return groupBy(robots, 'name')
 }

app/src/health-check/__tests__/health-check.test.js

@@ -33,7 +33,7 @@ describe('health check', () => {
       },
       discovery: {
         robotsByName: {
-          [name]: {name, connections: [{ip, port, ok: true}]},
+          [name]: [{ip, port, ok: true}],
         },
       },
       healthCheck: {

app/src/robot/selectors.js

@@ -55,18 +55,18 @@ export const getDiscovered: OutputSelector<State, void, Array<Robot>> =
     (discoveredByName, connectedTo, unexpectedDisconnect) => {
       const robots = Object.keys(discoveredByName)
         .map(name => {
-          const robot = discoveredByName[name]
           const connection = orderBy(
-            robot.connections,
+            discoveredByName[name],
             ['ok', 'local'],
             ['desc', 'desc']
-          ).find(c => c.ok)
+          ).find(c => c.ip && c.ok)
 
           if (!connection) return null
 
           return {
-            name: robot.name,
-            ip: connection.ip,
+            name,
+            // $FlowFixMe: to be fixed by the removal of this selector
+            ip: (connection.ip: string),
             port: connection.port,
             wired: connection.local,
             isConnected: connectedTo === name,