From f298226ae75375b8436425165539a5c2c4992b2c Mon Sep 17 00:00:00 2001
From: Mike Cousins <mike@opentrons.com>
Date: Thu, 4 Oct 2018 12:49:59 -0400
Subject: [PATCH 1/4] feat(discovery-client): Add mdns flag and health
 responses to services

---
 app-shell/Makefile                            |   5 +-
 app-shell/src/discovery.js                    |  10 +-
 app/Makefile                                  |  30 +-
 discovery-client/README.md                    |  52 +++-
 discovery-client/src/__fixtures__/service.js  |  10 +-
 discovery-client/src/__tests__/client.test.js |  59 ++--
 .../src/__tests__/service-list.test.js        |  50 +++-
 .../src/__tests__/service.test.js             | 263 ++++++++++++++----
 discovery-client/src/cli.js                   |   4 +-
 discovery-client/src/index.js                 |   9 +-
 discovery-client/src/poller.js                |   9 +-
 discovery-client/src/service-list.js          |  64 +++--
 discovery-client/src/service.js               |  50 ++--
 discovery-client/src/types.js                 |  28 +-
 14 files changed, 466 insertions(+), 177 deletions(-)

diff --git a/app-shell/Makefile b/app-shell/Makefile
index 65d48777649..0b14b61fba7 100644
--- a/app-shell/Makefile
+++ b/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
diff --git a/app-shell/src/discovery.js b/app-shell/src/discovery.js
index 51ba56012e8..4badb99e343 100644
--- a/app-shell/src/discovery.js
+++ b/app-shell/src/discovery.js
@@ -4,6 +4,7 @@ import assert from 'assert'
 import Store from 'electron-store'
 import groupBy from 'lodash/groupBy'
 import map from 'lodash/map'
+import omit from 'lodash/omit'
 import throttle from 'lodash/throttle'
 import uniqBy from 'lodash/uniqBy'
 
@@ -106,13 +107,12 @@ function servicesToConnections (services: Array<Service>): Array<Connection> {
 }
 
 function serviceToConnection (service: Service): ?Connection {
-  if (!service.ip) return null
+  const {ip} = service
+  if (!ip) return null
 
   return {
-    ip: service.ip,
-    ok: service.ok,
-    port: service.port,
-    local: isLocal(service.ip),
+    ...omit(service, ['name']),
+    local: isLocal(ip),
   }
 }
 
diff --git a/app/Makefile b/app/Makefile
index b442faf51e2..ea8465260bb 100644
--- a/app/Makefile
+++ b/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
diff --git a/discovery-client/README.md b/discovery-client/README.md
index f2dae9224c6..5a1b20aa53a 100644
--- a/discovery-client/README.md
+++ b/discovery-client/README.md
@@ -24,7 +24,7 @@ const options = {
   nameFilter: ['opentrons'],
   portFilter: [31950],
   pollInterval: 5000,
-  candidates: [{ ip: '[fd00:0:cafe:fefe::1]', port: 31950 }, 'localhost']
+  candidates: [{ip: '[fd00:0:cafe:fefe::1]', port: 31950}, 'localhost'],
 }
 
 const client = DiscoveryClientFactory(options)
@@ -94,8 +94,32 @@ type Service = {
   /** possible service port (31950 if unspecified) */
   port: ?number,
 
-  /** possible health status (null if not yet determined) */
-  ok: ?boolean
+  /** health status of the API server (null if not yet determined) */
+  ok: ?boolean,
+
+  /** health status of the update server (null if not yet determined) */
+  serverOk: ?boolean,
+
+  /** whether robot is advertising over MDNS (null if not yet determined) */
+  advertising: ?boolean,
+
+  /** last good health response */
+  health: ?{
+    name: string,
+    api_version: string,
+    fw_version: string,
+    system_version?: string,
+    logs?: Array<string>,
+  },
+
+  /** last good update server health response */
+  serverHealth: ?{
+    name: string,
+    apiServerVersion: string,
+    updateServerVersion: string,
+    smoothieVersion: string,
+    systemVersion: string,
+  },
 }
 ```
 
@@ -105,14 +129,14 @@ type Candidate = {
   ip: string,
 
   /** service port (31950 if unspecified) */
-  port: ?number
+  port: ?number,
 }
 ```
 
 `discovery.client` takes an optional `options` parameter:
 
 ```js
-type Option = {
+type Options = {
   /**
    * interval (ms) at which to poll an IP to find a robot
    * default: 5000
@@ -152,15 +176,15 @@ type Option = {
 
   /** optional logger */
   logger?: {
-    ['error' | 'warn' | 'info' | 'debug']: (message: string, meta: {}) => void
-  }
+    ['error' | 'warn' | 'info' | 'debug']: (message: string, meta: {}) => void,
+  },
 }
 ```
 
 If you need access to the `DiscoveryClient` class itself for some reason:
 
 ```js
-import { DiscoveryClient } from '@opentrons/discovery-client'
+import {DiscoveryClient} from '@opentrons/discovery-client'
 
 const client = new DiscoveryClient({})
 ```
@@ -177,11 +201,11 @@ client.on(SERVICE_REMOVED_EVENT, (data) => console.log('service removed', data))
 client.on('error', (error) => console.error(error)
 ```
 
-| event name       | data      | description                                   |
-| ---------------- | --------- | --------------------------------------------- |
-| `service`        | `Service` | Service added or updated in the services list |
-| `serviceRemoved` | `Service` | Service removed from the services list        |
-| `error`          | `Error`   | MDNS encountered an error                     |
+| event name       | data             | description                                    |
+| ---------------- | ---------------- | ---------------------------------------------- |
+| `service`        | `Array<Service>` | Services added or updated in the services list |
+| `serviceRemoved` | `Array<Service>` | Services removed from the services list        |
+| `error`          | `Error`          | MDNS encountered an error                      |
 
 ## cli
 
@@ -211,7 +235,7 @@ The CLI's global options are almost completely the same as the API's options, wi
 | `-c, --candidates`   | see `candidates` option   | `[]`     | `-c localhost`   |
 | `-n, --nameFilter`   | see `nameFilter` option   | `[]`     | `-n opentrons`   |
 | `-i, --ipFilter`     | see `ipFilter` option     | `[]`     | `-i 169.254`     |
-| `-a, --portFilter` | see `portFilter` option | `[]`     | `-a 31951 31952` |
+| `-a, --portFilter`   | see `portFilter` option   | `[]`     | `-a 31951 31952` |
 | `-l, --logLevel`     | log level for printout    | `'info'` | `-l debug`       |
 
 ### `discovery (browse) [options]`
diff --git a/discovery-client/src/__fixtures__/service.js b/discovery-client/src/__fixtures__/service.js
index 73000139b50..8248bf4776b 100644
--- a/discovery-client/src/__fixtures__/service.js
+++ b/discovery-client/src/__fixtures__/service.js
@@ -1,7 +1,15 @@
-export default {
+// @flow
+import type {Service} from '../types'
+
+const MOCK_SERVICE: Service = {
   name: 'opentrons-dev',
   ip: '192.168.1.42',
   port: 31950,
   ok: null,
   serverOk: null,
+  advertising: null,
+  health: null,
+  serverHealth: null,
 }
+
+export default MOCK_SERVICE
diff --git a/discovery-client/src/__tests__/client.test.js b/discovery-client/src/__tests__/client.test.js
index 12c6089bb04..2e496ea5927 100644
--- a/discovery-client/src/__tests__/client.test.js
+++ b/discovery-client/src/__tests__/client.test.js
@@ -66,11 +66,12 @@ describe('discovery client', () => {
       const client = DiscoveryClient()
 
       client.start()
-      client.once('service', robot => {
+      client.once('service', results => {
         expect(service.fromMdnsBrowser).toHaveBeenCalledWith(
           MOCK_BROWSER_SERVICE
         )
-        expect(robot).toEqual(MOCK_SERVICE)
+        expect(results).toHaveLength(1)
+        expect(results[0]).toEqual({...MOCK_SERVICE, advertising: true})
         done()
       })
 
@@ -79,22 +80,14 @@ describe('discovery client', () => {
     10
   )
 
-  test(
-    'adds robot to client.services if browser finds a service',
-    done => {
-      const client = DiscoveryClient()
-
-      client.start()
-      client.once('service', service => {
-        expect(client.services).toEqual([service])
-        expect(client.candidates).toEqual([])
-        done()
-      })
+  test('adds robot to client.services if browser finds a service', () => {
+    const client = DiscoveryClient()
 
-      mdns.__mockBrowser.emit('update', MOCK_BROWSER_SERVICE)
-    },
-    10
-  )
+    client.start()
+    mdns.__mockBrowser.emit('update', MOCK_BROWSER_SERVICE)
+    expect(client.services).toEqual([{...MOCK_SERVICE, advertising: true}])
+    expect(client.candidates).toEqual([])
+  })
 
   test('new mdns service does not null out ok of existing service', () => {
     const client = DiscoveryClient()
@@ -104,6 +97,7 @@ describe('discovery client', () => {
         ...MOCK_SERVICE,
         ok: true,
         serverOk: true,
+        advertising: true,
       },
     ]
 
@@ -115,6 +109,7 @@ describe('discovery client', () => {
         ...MOCK_SERVICE,
         ok: true,
         serverOk: true,
+        advertising: true,
       },
     ])
   })
@@ -326,6 +321,8 @@ describe('discovery client', () => {
         ...MOCK_SERVICE,
         ok: true,
         serverOk: true,
+        health: {name: 'opentrons-dev'},
+        serverHealth: {name: 'opentrons-dev'},
       },
     ])
   })
@@ -334,28 +331,28 @@ describe('discovery client', () => {
     const client = DiscoveryClient()
 
     client.services = [
-      {name: 'bar', ip: 'foo', port: 31950},
-      {name: 'baz', ip: 'foo', port: 31950},
+      {...MOCK_SERVICE, name: 'bar', ok: true, serverOk: true},
+      {...MOCK_SERVICE, name: 'baz', ok: true, serverOk: true},
     ]
 
     client.start()
     const onHealth = poller.poll.mock.calls[0][2]
     onHealth(
-      {ip: 'foo', port: 31950},
+      {ip: MOCK_SERVICE.ip, port: 31950},
       {name: 'opentrons-dev'},
       {name: 'opentrons-dev'}
     )
 
     expect(client.services).toEqual([
       {
-        name: 'opentrons-dev',
-        ip: 'foo',
-        port: 31950,
+        ...MOCK_SERVICE,
         ok: true,
         serverOk: true,
+        health: {name: 'opentrons-dev'},
+        serverHealth: {name: 'opentrons-dev'},
       },
-      {name: 'bar', ip: null, port: 31950, ok: null, serverOk: null},
-      {name: 'baz', ip: null, port: 31950, ok: null, serverOk: null},
+      {...MOCK_SERVICE, name: 'bar', ip: null},
+      {...MOCK_SERVICE, name: 'baz', ip: null},
     ])
   })
 
@@ -428,7 +425,7 @@ describe('discovery client', () => {
   test(
     'candidate removal emits removal events',
     done => {
-      let services = [
+      const services = [
         MOCK_SERVICE,
         {
           ...MOCK_SERVICE,
@@ -439,13 +436,9 @@ describe('discovery client', () => {
 
       const client = DiscoveryClient({services})
 
-      client.on('serviceRemoved', service => {
-        expect(services).toContainEqual(service)
-        services = services.filter(
-          s => s.name !== service.name || s.ip !== service.ip
-        )
-
-        if (services.length === 0) done()
+      client.on('serviceRemoved', results => {
+        expect(results).toEqual(services)
+        done()
       })
 
       client.start()
diff --git a/discovery-client/src/__tests__/service-list.test.js b/discovery-client/src/__tests__/service-list.test.js
index b33777d8c3b..ede417849d2 100644
--- a/discovery-client/src/__tests__/service-list.test.js
+++ b/discovery-client/src/__tests__/service-list.test.js
@@ -12,14 +12,52 @@ describe('serviceList', () => {
       {
         name: 'cleans up input list',
         input: [
-          {...MOCK_SERVICE, name: 'foo', ok: true, serverOk: false},
-          {...MOCK_SERVICE, name: 'bar', ok: false, serverOk: true},
-          {...MOCK_SERVICE, name: 'baz', ok: true, serverOk: true},
+          {
+            ...MOCK_SERVICE,
+            name: 'foo',
+            ok: true,
+            serverOk: false,
+            health: {api_version: '1.0.0'},
+            serverHealth: null,
+          },
+          {
+            ...MOCK_SERVICE,
+            name: 'bar',
+            ok: false,
+            serverOk: true,
+            health: null,
+            serverHealth: {apiServerVersion: '1.0.0'},
+          },
+          {
+            ...MOCK_SERVICE,
+            name: 'baz',
+            ok: true,
+            serverOk: true,
+            health: {api_version: '1.0.0'},
+            serverHealth: {apiServerVersion: '1.0.0'},
+          },
         ],
         expected: [
-          {...MOCK_SERVICE, name: 'foo'},
-          {...MOCK_SERVICE, name: 'bar', ip: null},
-          {...MOCK_SERVICE, name: 'baz', ip: null},
+          {
+            ...MOCK_SERVICE,
+            name: 'foo',
+            health: {api_version: '1.0.0'},
+            serverHealth: null,
+          },
+          {
+            ...MOCK_SERVICE,
+            name: 'bar',
+            ip: null,
+            health: null,
+            serverHealth: {apiServerVersion: '1.0.0'},
+          },
+          {
+            ...MOCK_SERVICE,
+            name: 'baz',
+            ip: null,
+            health: {api_version: '1.0.0'},
+            serverHealth: {apiServerVersion: '1.0.0'},
+          },
         ],
       },
       {
diff --git a/discovery-client/src/__tests__/service.test.js b/discovery-client/src/__tests__/service.test.js
index 3a23d3fb86e..e7457f89430 100644
--- a/discovery-client/src/__tests__/service.test.js
+++ b/discovery-client/src/__tests__/service.test.js
@@ -1,87 +1,236 @@
 // tests for service and candidate creation utils
-import cloneDeep from 'lodash/cloneDeep'
+import {setIn} from '@thi.ng/paths'
 
 import * as service from '../service'
 import MOCK_BROWSER_SERVICE from '../__fixtures__/mdns-browser-service'
 
 describe('service utils', () => {
   describe('makeService', () => {
-    test('all info specified', () => {
-      expect(service.makeService('name', 'ip', 1234, false, false)).toEqual({
-        name: 'name',
-        ip: 'ip',
-        port: 1234,
-        ok: false,
-        serverOk: false,
-      })
-    })
-
-    test('defaults', () => {
-      expect(service.makeService('name')).toEqual(
-        expect.objectContaining({
+    const SPECS = [
+      {
+        name: 'all info specified',
+        args: [
+          'name',
+          'ip',
+          1234,
+          false,
+          false,
+          false,
+          {name: 'name'},
+          {name: 'name'},
+        ],
+        expected: {
+          name: 'name',
+          ip: 'ip',
+          port: 1234,
+          ok: false,
+          serverOk: false,
+          advertising: false,
+          health: {name: 'name'},
+          serverHealth: {name: 'name'},
+        },
+      },
+      {
+        name: 'defaults',
+        args: ['name'],
+        expected: {
+          name: 'name',
           ip: null,
           port: 31950,
           ok: null,
           serverOk: null,
-        })
+          advertising: null,
+          health: null,
+          serverHealth: null,
+        },
+      },
+    ]
+
+    SPECS.forEach(spec => {
+      const {name, args, expected} = spec
+      test(name, () =>
+        expect(service.makeService.apply(null, args)).toEqual(expected)
       )
     })
   })
 
   describe('makeCandidate', () => {
-    test('all info specified', () => {
-      expect(service.makeCandidate('ip', 1234)).toEqual({ip: 'ip', port: 1234})
-    })
+    const SPECS = [
+      {
+        name: 'all info specified',
+        args: ['ip', 1234],
+        expected: {ip: 'ip', port: 1234},
+      },
+      {
+        name: 'defaults',
+        args: ['ip'],
+        expected: {ip: 'ip', port: 31950},
+      },
+    ]
 
-    test('defaults', () => {
-      expect(service.makeCandidate('ip')).toEqual({ip: 'ip', port: 31950})
+    SPECS.forEach(spec => {
+      const {name, args, expected} = spec
+      test(name, () =>
+        expect(service.makeCandidate.apply(null, args)).toEqual(expected)
+      )
     })
   })
 
   describe('fromMdnsBrowser', () => {
-    let mdnsService
-
-    beforeEach(() => {
-      mdnsService = cloneDeep(MOCK_BROWSER_SERVICE)
-    })
-
-    test('gets name from fqdn', () => {
-      expect(service.fromMdnsBrowser(mdnsService)).toEqual(
-        service.makeService(
-          'opentrons-dev',
-          expect.anything(),
-          expect.anything()
-        )
-      )
-    })
-
-    test('with IPv4 service', () => {
-      mdnsService.addresses = [
-        'fe80::caf4:6db4:4652:e975',
-        ...mdnsService.addresses,
-      ]
+    const SPECS = [
+      {
+        name: 'returns null if no IP',
+        args: [setIn(MOCK_BROWSER_SERVICE, 'addresses', [])],
+        expected: null,
+      },
+      {
+        name: 'gets name from fqdn and populates IP, port, and advertising',
+        args: [MOCK_BROWSER_SERVICE],
+        expected: {
+          name: 'opentrons-dev',
+          ip: MOCK_BROWSER_SERVICE.addresses[0],
+          port: MOCK_BROWSER_SERVICE.port,
+          ok: null,
+          serverOk: null,
+          advertising: true,
+          health: null,
+          serverHealth: null,
+        },
+      },
+      {
+        name: 'prefers IPv4',
+        args: [
+          setIn(MOCK_BROWSER_SERVICE, 'addresses', [
+            'fe80::caf4:6db4:4652:e975',
+            ...MOCK_BROWSER_SERVICE.addresses,
+          ]),
+        ],
+        expected: {
+          name: 'opentrons-dev',
+          ip: '192.168.1.42',
+          port: 31950,
+          ok: null,
+          serverOk: null,
+          advertising: true,
+          health: null,
+          serverHealth: null,
+        },
+      },
+      {
+        name: 'adds brackets to IPv6',
+        args: [
+          setIn(MOCK_BROWSER_SERVICE, 'addresses', [
+            'fe80::caf4:6db4:4652:e975',
+          ]),
+        ],
+        expected: {
+          name: 'opentrons-dev',
+          ip: '[fe80::caf4:6db4:4652:e975]',
+          port: 31950,
+          ok: null,
+          serverOk: null,
+          advertising: true,
+          health: null,
+          serverHealth: null,
+        },
+      },
+    ]
 
-      expect(service.fromMdnsBrowser(mdnsService)).toEqual(
-        service.makeService('opentrons-dev', '192.168.1.42', 31950)
+    SPECS.forEach(spec => {
+      const {name, args, expected} = spec
+      test(name, () =>
+        expect(service.fromMdnsBrowser.apply(null, args)).toEqual(expected)
       )
     })
+  })
 
-    test('with IPv6 service', () => {
-      mdnsService.addresses = ['fe80::caf4:6db4:4652:e975']
+  describe('fromResponse', () => {
+    const MOCK_CANDIDATE = {ip: '192.168.1.42', port: 31950}
+    const SPECS = [
+      {
+        name: 'returns null if no responses',
+        args: [MOCK_CANDIDATE, null, null],
+        expected: null,
+      },
+      {
+        name: 'gets ip from candidate and name from responses',
+        args: [
+          MOCK_CANDIDATE,
+          {name: 'opentrons-dev', api_version: '1.0.0'},
+          {name: 'opentrons-dev', apiServerVersion: '1.0.0'},
+        ],
+        expected: {
+          name: 'opentrons-dev',
+          ip: '192.168.1.42',
+          port: 31950,
+          ok: true,
+          serverOk: true,
+          advertising: null,
+          health: {name: 'opentrons-dev', api_version: '1.0.0'},
+          serverHealth: {name: 'opentrons-dev', apiServerVersion: '1.0.0'},
+        },
+      },
+      {
+        name: 'flags ok false if no /health response',
+        args: [
+          MOCK_CANDIDATE,
+          null,
+          {name: 'opentrons-dev', apiServerVersion: '1.0.0'},
+        ],
+        expected: {
+          name: 'opentrons-dev',
+          ip: '192.168.1.42',
+          port: 31950,
+          ok: false,
+          serverOk: true,
+          advertising: null,
+          health: null,
+          serverHealth: {name: 'opentrons-dev', apiServerVersion: '1.0.0'},
+        },
+      },
+      {
+        name: 'flags serverOk false if no /server/update/health response',
+        args: [
+          MOCK_CANDIDATE,
+          {name: 'opentrons-dev', api_version: '1.0.0'},
+          null,
+        ],
+        expected: {
+          name: 'opentrons-dev',
+          ip: '192.168.1.42',
+          port: 31950,
+          ok: true,
+          serverOk: false,
+          advertising: null,
+          health: {name: 'opentrons-dev', api_version: '1.0.0'},
+          serverHealth: null,
+        },
+      },
+      {
+        name: 'flags ok false if name mismatch in responses',
+        args: [
+          MOCK_CANDIDATE,
+          {name: 'opentrons-wrong', api_version: '1.0.0'},
+          {name: 'opentrons-dev', apiServerVersion: '1.0.0'},
+        ],
+        expected: {
+          name: 'opentrons-dev',
+          ip: '192.168.1.42',
+          port: 31950,
+          ok: false,
+          serverOk: true,
+          advertising: null,
+          health: {name: 'opentrons-wrong', api_version: '1.0.0'},
+          serverHealth: {name: 'opentrons-dev', apiServerVersion: '1.0.0'},
+        },
+      },
+    ]
 
-      expect(service.fromMdnsBrowser(mdnsService)).toEqual(
-        service.makeService(
-          'opentrons-dev',
-          '[fe80::caf4:6db4:4652:e975]',
-          31950
-        )
+    SPECS.forEach(spec => {
+      const {name, args, expected} = spec
+      test(name, () =>
+        expect(service.fromResponse.apply(null, args)).toEqual(expected)
       )
     })
-
-    test('return null if no IP found', () => {
-      mdnsService.addresses = []
-
-      expect(service.fromMdnsBrowser(mdnsService)).toEqual(null)
-    })
   })
 })
diff --git a/discovery-client/src/cli.js b/discovery-client/src/cli.js
index 3909d58a135..a28cd00e829 100755
--- a/discovery-client/src/cli.js
+++ b/discovery-client/src/cli.js
@@ -71,8 +71,8 @@ require('yargs')
 
 function browse (argv) {
   DiscoveryClient(argv)
-    .on('service', s => argv.logger.info('service added or updated:', s))
-    .on('serviceRemoved', s => argv.logger.info('service removed:', s))
+    .on('service', s => argv.logger.info('services added or updated:', s))
+    .on('serviceRemoved', s => argv.logger.info('services removed:', s))
     .once('error', argv.handleError)
     .start()
 
diff --git a/discovery-client/src/index.js b/discovery-client/src/index.js
index 5690a14d147..f81e96f9510 100644
--- a/discovery-client/src/index.js
+++ b/discovery-client/src/index.js
@@ -25,11 +25,13 @@ import {
 } from './service'
 
 import type {Browser, BrowserService} from 'mdns-js'
+
 import type {
   Candidate,
   Service,
   ServiceList,
   HealthResponse,
+  ServerHealthResponse,
   LogLevel,
   Logger,
 } from './types'
@@ -137,7 +139,7 @@ export class DiscoveryClient extends EventEmitter {
 
     log(this._logger, 'debug', 'removed services from discovery', {removals})
     this._poll()
-    removals.forEach(s => this.emit(SERVICE_REMOVED_EVENT, s))
+    this.emit(SERVICE_REMOVED_EVENT, removals)
 
     return this
   }
@@ -209,7 +211,7 @@ export class DiscoveryClient extends EventEmitter {
   _handleHealth (
     candidate: Candidate,
     apiResponse: ?HealthResponse,
-    serverResponse: ?HealthResponse
+    serverResponse: ?ServerHealthResponse
   ): mixed {
     const service = fromResponse(candidate, apiResponse, serverResponse)
 
@@ -240,14 +242,15 @@ export class DiscoveryClient extends EventEmitter {
   // update this.services, emit if necessary, re-poll if necessary
   _updateLists (nextServices: ServiceList): void {
     const updated = differenceBy(nextServices, this.services)
+
     if (updated.length) {
       // $FlowFixMe: flow doesn't type differenceBy properly, but this works
       this.candidates = differenceBy(this.candidates, nextServices, 'ip')
       this.services = nextServices
       this._poll()
 
-      updated.forEach(s => this.emit(SERVICE_EVENT, s))
       log(this._logger, 'debug', 'updated services', {updated})
+      this.emit(SERVICE_EVENT, updated)
     }
   }
 }
diff --git a/discovery-client/src/poller.js b/discovery-client/src/poller.js
index c0f1080ea6a..1803dd48abb 100644
--- a/discovery-client/src/poller.js
+++ b/discovery-client/src/poller.js
@@ -3,12 +3,17 @@
 
 import fetch from 'node-fetch'
 
-import type { Candidate, Logger, HealthResponse } from './types'
+import type {
+  Candidate,
+  Logger,
+  HealthResponse,
+  ServerHealthResponse,
+} from './types'
 
 type HealthHandler = (
   candidate: Candidate,
   apiResponse: ?HealthResponse,
-  serverResponse: ?HealthResponse
+  serverResponse: ?ServerHealthResponse
 ) => mixed
 
 export type PollRequest = {
diff --git a/discovery-client/src/service-list.js b/discovery-client/src/service-list.js
index 6d7987d9619..f4364e76d9b 100644
--- a/discovery-client/src/service-list.js
+++ b/discovery-client/src/service-list.js
@@ -1,4 +1,8 @@
 // @flow
+import differenceBy from 'lodash/differenceBy'
+import partition from 'lodash/partition'
+import uniqBy from 'lodash/uniqBy'
+
 import {
   makeService,
   updateService,
@@ -10,7 +14,18 @@ import type {Service, ServiceList, ServiceUpdate} from './types'
 
 export function createServiceList (list: ServiceList = []): ServiceList {
   // strip health flags from input list
-  const nextList = list.map(s => makeService(s.name, s.ip, s.port))
+  const nextList = list.map(s =>
+    makeService(
+      s.name,
+      s.ip,
+      s.port,
+      null,
+      null,
+      null,
+      s.health,
+      s.serverHealth
+    )
+  )
 
   return dedupeServices(nextList)
 }
@@ -50,30 +65,41 @@ export function updateServiceListByIp (
 // ensure there aren't multiple entries with the same IP and there aren't
 // multiple entries with the same name and ip: null
 function dedupeServices (list: ServiceList) {
-  const dedupeResult = list
-    .slice()
-    .sort(compareByIpExists)
-    .reduce(
-      (result, service) => {
-        const {unique, seenIps, seenNames} = result
-        const {name} = service
-        const ip = service.ip && seenIps[service.ip] ? null : service.ip
-        const cleanedService = service.ip === ip ? service : {...service, ip}
+  const [listWithIp, listWithoutIp] = partition(list, 'ip')
+  const sanitizedWithIp = listWithIp.reduce(
+    (result, service) => {
+      // we know IP exists here thanks to our partition above
+      const ip: string = (service.ip: any)
+      const cleanedService = result.seenIps[ip]
+        ? clearServiceIfConflict(service, {ip})
+        : service
 
-        if (ip || !seenNames[name]) unique.push(cleanedService)
-        if (ip && !seenIps[ip]) seenIps[ip] = true
-        if (!seenNames[name]) seenNames[name] = true
+      result.seenIps[ip] = true
+      result.unique.push(cleanedService)
 
-        return {unique, seenIps, seenNames}
-      },
-      {unique: [], seenIps: {}, seenNames: {}}
-    )
+      return result
+    },
+    {unique: [], seenIps: {}}
+  ).unique
+
+  const dedupedWithoutIp = differenceBy(
+    uniqBy(listWithoutIp, 'name'),
+    sanitizedWithIp,
+    'name'
+  )
 
-  return dedupeResult.unique
+  return sanitizedWithIp.concat(dedupedWithoutIp).sort(compareServices)
 }
 
-function compareByIpExists (a: Service, b: Service) {
+// sort service list by: ip exists, update server healthy, API healthy, advertising
+function compareServices (a: Service, b: Service) {
   if (a.ip && !b.ip) return -1
   if (!a.ip && b.ip) return 1
+  if (a.serverOk && !b.serverOk) return -1
+  if (!a.serverOk && b.serverOk) return 1
+  if (a.ok && !b.ok) return -1
+  if (!a.ok && b.ok) return 1
+  if (a.advertising && !b.advertising) return -1
+  if (!a.advertising && b.advertising) return 1
   return 0
 }
diff --git a/discovery-client/src/service.js b/discovery-client/src/service.js
index c29469ca997..88f37ac240e 100644
--- a/discovery-client/src/service.js
+++ b/discovery-client/src/service.js
@@ -2,9 +2,16 @@
 // create Services from different sources
 import net from 'net'
 import defaultTo from 'lodash/defaultTo'
+import isEqual from 'lodash/isEqual'
 
 import type {BrowserService, ServiceType} from 'mdns-js'
-import type {Service, ServiceUpdate, Candidate, HealthResponse} from './types'
+import type {
+  Service,
+  ServiceUpdate,
+  Candidate,
+  HealthResponse,
+  ServerHealthResponse,
+} from './types'
 
 const nameExtractor = (st: ServiceType) =>
   new RegExp(`^(.+)\\._${st.name}\\._${st.protocol}`)
@@ -16,14 +23,20 @@ export function makeService (
   ip: ?string,
   port: ?number,
   ok: ?boolean,
-  serverOk: ?boolean
+  serverOk: ?boolean,
+  advertising: ?boolean,
+  health: ?HealthResponse,
+  serverHealth: ?ServerHealthResponse
 ): Service {
   return {
     name,
-    ip: ip || null,
-    port: port || DEFAULT_PORT,
-    ok: ok != null ? ok : null,
-    serverOk: serverOk != null ? serverOk : null,
+    ip: defaultTo(ip, null),
+    port: defaultTo(port, DEFAULT_PORT),
+    ok: defaultTo(ok, null),
+    serverOk: defaultTo(serverOk, null),
+    advertising: defaultTo(advertising, null),
+    health: defaultTo(health, null),
+    serverHealth: defaultTo(serverHealth, null),
   }
 }
 
@@ -38,15 +51,16 @@ export function updateService (
   return Object.keys(update).reduce((result, key) => {
     const prevVal = result[key]
     const nextVal = defaultTo(next[key], prevVal)
+    // use isEqual to deep compare response objects
     // $FlowFixMe: flow can't type [key]: nextVal but we know this is correct
-    return nextVal !== prevVal ? {...result, [key]: nextVal} : result
+    return isEqual(nextVal, prevVal) ? result : {...result, [key]: nextVal}
   }, service)
 }
 
 // null out conflicting fields
 export function clearServiceIfConflict (
   service: Service,
-  update: ?Service
+  update: ?ServiceUpdate
 ): Service {
   return update && service.ip === update.ip
     ? {...service, ip: null, ok: null, serverOk: null}
@@ -71,24 +85,23 @@ export function fromMdnsBrowser (browserService: BrowserService): ?Service {
   const nameMatch = type[0] && fullname.match(nameExtractor(type[0]))
   const name = (nameMatch && nameMatch[1]) || fullname
 
-  return makeService(name, ip, port)
+  return makeService(name, ip, port, null, null, true)
 }
 
 export function fromResponse (
   candidate: Candidate,
-  response: ?HealthResponse,
-  serverResponse: ?HealthResponse
+  healthResponse: ?HealthResponse,
+  serverHealthResponse: ?ServerHealthResponse
 ): ?Service {
-  const apiName = response && response.name
-  const serverName = serverResponse && serverResponse.name
-  let apiOk = apiName != null
-  let name = apiName || serverName
+  const apiName = healthResponse && healthResponse.name
+  const serverName = serverHealthResponse && serverHealthResponse.name
+  const name = defaultTo(serverName, apiName)
+  let apiOk = !!healthResponse
 
   if (!name) return null
 
   // in case of name mismatch, prefer /server/health name and flag not ok
   if (apiName != null && serverName != null && apiName !== serverName) {
-    name = serverName
     apiOk = false
   }
 
@@ -97,7 +110,10 @@ export function fromResponse (
     candidate.ip,
     candidate.port,
     apiOk,
-    serverName != null
+    !!serverHealthResponse,
+    null,
+    healthResponse,
+    serverHealthResponse
   )
 }
 
diff --git a/discovery-client/src/types.js b/discovery-client/src/types.js
index 42795010023..d78e7139c79 100644
--- a/discovery-client/src/types.js
+++ b/discovery-client/src/types.js
@@ -1,5 +1,22 @@
 // @flow
 
+// TODO(mc, 2018-10-03): figure out what to do with duplicate type in app
+export type HealthResponse = {
+  name: string,
+  api_version: string,
+  fw_version: string,
+  system_version?: string,
+  logs?: Array<string>,
+}
+
+export type ServerHealthResponse = {
+  name: string,
+  apiServerVersion: string,
+  updateServerVersion: string,
+  smoothieVersion: string,
+  systemVersion: string,
+}
+
 export type Candidate = {
   ip: string,
   port: number,
@@ -13,6 +30,12 @@ export type Service = {
   ok: ?boolean,
   // GET /server/health response.ok === true
   serverOk: ?boolean,
+  // is advertising on MDNS
+  advertising: ?boolean,
+  // last good /health response
+  health: ?HealthResponse,
+  // last good /server/health response
+  serverHealth: ?ServerHealthResponse,
 }
 
 export type ServiceUpdate = $Shape<Service>
@@ -30,8 +53,3 @@ export type LogLevel =
   | 'silly'
 
 export type Logger = {[level: LogLevel]: (message: string, meta?: {}) => void}
-
-// note: the discovery module only cares about name
-export type HealthResponse = {
-  name: string,
-}

From ec1ca4ee52aafc76a36d01a2620d0f9f28500b82 Mon Sep 17 00:00:00 2001
From: Mike Cousins <mike@opentrons.com>
Date: Thu, 4 Oct 2018 16:11:40 -0400
Subject: [PATCH 2/4] fixup: streamline discovery-client > app state

---
 app-shell/src/__tests__/discovery.test.js     | 59 +------------------
 app-shell/src/discovery.js                    | 49 +--------------
 .../analytics/__tests__/make-event.test.js    | 10 +---
 app/src/discovery/__tests__/reducer.test.js   | 16 ++---
 app/src/discovery/index.js                    | 16 ++---
 app/src/discovery/types.js                    |  9 +--
 .../__tests__/health-check.test.js            |  2 +-
 app/src/robot/selectors.js                    | 10 ++--
 app/src/robot/test/api-client.test.js         |  5 +-
 app/src/robot/test/selectors.test.js          | 40 +++++--------
 app/src/robot/types.js                        |  2 +-
 app/src/shell/index.js                        |  4 +-
 discovery-client/README.md                    |  7 ++-
 discovery-client/src/__fixtures__/service.js  |  1 +
 discovery-client/src/__tests__/client.test.js |  1 +
 .../src/__tests__/service-list.test.js        | 11 ++--
 .../src/__tests__/service.test.js             | 14 +++++
 discovery-client/src/service.js               | 10 ++++
 discovery-client/src/types.js                 |  2 +
 19 files changed, 88 insertions(+), 180 deletions(-)

diff --git a/app-shell/src/__tests__/discovery.test.js b/app-shell/src/__tests__/discovery.test.js
index 807a8471b54..e48dff68c47 100644
--- a/app-shell/src/__tests__/discovery.test.js
+++ b/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},
         })
       })
     )
diff --git a/app-shell/src/discovery.js b/app-shell/src/discovery.js
index 4badb99e343..977cc623b1f 100644
--- a/app-shell/src/discovery.js
+++ b/app-shell/src/discovery.js
@@ -1,12 +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 omit from 'lodash/omit'
 import throttle from 'lodash/throttle'
-import uniqBy from 'lodash/uniqBy'
 
 import DiscoveryClient, {
   SERVICE_EVENT,
@@ -20,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)
 
@@ -71,7 +62,7 @@ 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},
     })
   }
 }
@@ -79,7 +70,7 @@ export function registerDiscovery (dispatch: Action => void) {
 export function getRobots () {
   if (!client) return []
 
-  return servicesToRobots(client.services)
+  return client.services
 }
 
 function filterServicesToPersist (services: Array<Service>) {
@@ -89,39 +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 {
-  const {ip} = service
-  if (!ip) return null
-
-  return {
-    ...omit(service, ['name']),
-    local: isLocal(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'
-  )
-}
diff --git a/app/src/analytics/__tests__/make-event.test.js b/app/src/analytics/__tests__/make-event.test.js
index 3a1114d909c..84a3d68d414 100644
--- a/app/src/analytics/__tests__/make-event.test.js
+++ b/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}],
         },
       },
     })
diff --git a/app/src/discovery/__tests__/reducer.test.js b/app/src/discovery/__tests__/reducer.test.js
index e8d170b979d..ee7f145430f 100644
--- a/app/src/discovery/__tests__/reducer.test.js
+++ b/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}],
         },
       },
     },
diff --git a/app/src/discovery/index.js b/app/src/discovery/index.js
index e4c3d73f84e..17d3b56ff30 100644
--- a/app/src/discovery/index.js
+++ b/app/src/discovery/index.js
@@ -1,11 +1,13 @@
 // @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,7 +26,7 @@ type FinishAction = {|
 
 type UpdateListAction = {|
   type: 'discovery:UPDATE_LIST',
-  payload: {|robots: Array<DiscoveredRobot>|},
+  payload: {|robots: Array<Service>|},
 |}
 
 export * from './types'
@@ -91,12 +93,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')
 }
diff --git a/app/src/discovery/types.js b/app/src/discovery/types.js
index 5289ce3c881..24db3e19ffc 100644
--- a/app/src/discovery/types.js
+++ b/app/src/discovery/types.js
@@ -1,10 +1,7 @@
 // @flow
-export type Connection = {
-  ip: string,
-  port: number,
-  local: boolean,
-  ok: ?boolean,
-}
+import type {Service} from '@opentrons/discovery-client'
+
+export type Connection = Service & {local: boolean}
 
 export type DiscoveredRobot = {
   name: string,
diff --git a/app/src/health-check/__tests__/health-check.test.js b/app/src/health-check/__tests__/health-check.test.js
index 903b34f8a8a..3c7126350ed 100644
--- a/app/src/health-check/__tests__/health-check.test.js
+++ b/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: {
diff --git a/app/src/robot/selectors.js b/app/src/robot/selectors.js
index 50d36554dcd..9d0d997e8be 100644
--- a/app/src/robot/selectors.js
+++ b/app/src/robot/selectors.js
@@ -54,18 +54,18 @@ export const getDiscovered: Selector<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,
diff --git a/app/src/robot/test/api-client.test.js b/app/src/robot/test/api-client.test.js
index 327df440f58..0c89bc95775 100755
--- a/app/src/robot/test/api-client.test.js
+++ b/app/src/robot/test/api-client.test.js
@@ -81,10 +81,7 @@ describe('api client', () => {
     },
     discovery: {
       robotsByName: {
-        [ROBOT_NAME]: {
-          name: ROBOT_NAME,
-          connections: [{ip: ROBOT_IP, port: 31950, ok: true}],
-        },
+        [ROBOT_NAME]: [{ip: ROBOT_IP, port: 31950, ok: true}],
       },
     },
   }
diff --git a/app/src/robot/test/selectors.test.js b/app/src/robot/test/selectors.test.js
index 238bb5d1707..60ca9478f4a 100644
--- a/app/src/robot/test/selectors.test.js
+++ b/app/src/robot/test/selectors.test.js
@@ -41,32 +41,20 @@ describe('robot selectors', () => {
         robot: {connection: {connectedTo: 'bar'}},
         discovery: {
           robotsByName: {
-            foo: {
-              name: 'foo',
-              connections: [
-                {ip: '10.10.1.2', port: 31950, ok: true, local: false},
-              ],
-            },
-            bar: {
-              name: 'bar',
-              connections: [
-                {ip: '10.10.3.4', port: 31950, ok: true, local: false},
-                {ip: '169.254.3.4', port: 31950, ok: true, local: true},
-              ],
-            },
-            baz: {
-              name: 'baz',
-              connections: [
-                {ip: '10.10.5.6', port: 31950, ok: true, local: false},
-                {ip: '169.254.5.6', port: 31950, ok: false, local: true},
-              ],
-            },
-            qux: {
-              name: 'qux',
-              connections: [
-                {ip: '169.254.7.8', port: 31950, ok: true, local: true},
-              ],
-            },
+            foo: [
+              {ip: '10.10.1.2', port: 31950, ok: true, local: false},
+            ],
+            bar: [
+              {ip: '10.10.3.4', port: 31950, ok: true, local: false},
+              {ip: '169.254.3.4', port: 31950, ok: true, local: true},
+            ],
+            baz: [
+              {ip: '10.10.5.6', port: 31950, ok: true, local: false},
+              {ip: '169.254.5.6', port: 31950, ok: false, local: true},
+            ],
+            qux: [
+              {ip: '169.254.7.8', port: 31950, ok: true, local: true},
+            ],
           },
         },
       }
diff --git a/app/src/robot/types.js b/app/src/robot/types.js
index 1fd742e714d..a0c6919bd6b 100644
--- a/app/src/robot/types.js
+++ b/app/src/robot/types.js
@@ -38,7 +38,7 @@ export type BaseRobot = {
 export type RobotService = BaseRobot & {
   ip: string,
   port: number,
-  wired?: boolean,
+  wired: ?boolean,
 }
 
 // robot from getDiscovered selector
diff --git a/app/src/shell/index.js b/app/src/shell/index.js
index 4b4eb045e4b..a32092eb75a 100644
--- a/app/src/shell/index.js
+++ b/app/src/shell/index.js
@@ -7,10 +7,10 @@ import {makeGetRobotHealth} from '../http-api-client'
 import {updateReducer} from './update'
 import {apiUpdateReducer} from './api-update'
 
+import type {Service} from '@opentrons/discovery-client'
 import type {Middleware, ThunkAction} from '../types'
 import type {RobotService} from '../robot'
 import type {Config} from '../config'
-import type {DiscoveredRobot} from '../discovery'
 import type {ShellUpdateAction} from './update'
 
 export type ShellAction = ShellUpdateAction
@@ -55,7 +55,7 @@ export function getShellConfig (): Config {
 }
 
 // getShellRobots makes a sync RPC call, so use sparingly
-export function getShellRobots (): Array<DiscoveredRobot> {
+export function getShellRobots (): Array<Service> {
   return getRobots()
 }
 
diff --git a/discovery-client/README.md b/discovery-client/README.md
index 5a1b20aa53a..0c91e3fd230 100644
--- a/discovery-client/README.md
+++ b/discovery-client/README.md
@@ -91,8 +91,11 @@ type Service = {
   /** possible ip address (null if an IP conflict occurred) */
   ip: ?string,
 
-  /** possible service port (31950 if unspecified) */
-  port: ?number,
+  /** service port (deafult 31950) */
+  port: number,
+
+  /** IP address (if known) is a link-local address */
+  local: ?boolean,
 
   /** health status of the API server (null if not yet determined) */
   ok: ?boolean,
diff --git a/discovery-client/src/__fixtures__/service.js b/discovery-client/src/__fixtures__/service.js
index 8248bf4776b..285da083c42 100644
--- a/discovery-client/src/__fixtures__/service.js
+++ b/discovery-client/src/__fixtures__/service.js
@@ -5,6 +5,7 @@ const MOCK_SERVICE: Service = {
   name: 'opentrons-dev',
   ip: '192.168.1.42',
   port: 31950,
+  local: false,
   ok: null,
   serverOk: null,
   advertising: null,
diff --git a/discovery-client/src/__tests__/client.test.js b/discovery-client/src/__tests__/client.test.js
index 2e496ea5927..7f7c1dc22f0 100644
--- a/discovery-client/src/__tests__/client.test.js
+++ b/discovery-client/src/__tests__/client.test.js
@@ -431,6 +431,7 @@ describe('discovery client', () => {
           ...MOCK_SERVICE,
           ip: '[fd00:0:cafe:fefe::1]',
           port: 31950,
+          local: true,
         },
       ]
 
diff --git a/discovery-client/src/__tests__/service-list.test.js b/discovery-client/src/__tests__/service-list.test.js
index ede417849d2..fb3acdf43ff 100644
--- a/discovery-client/src/__tests__/service-list.test.js
+++ b/discovery-client/src/__tests__/service-list.test.js
@@ -68,12 +68,15 @@ describe('serviceList', () => {
       {
         name: 'collapses multiple IP unknown duplicates into one',
         input: [
-          {...MOCK_SERVICE, ip: null},
+          {...MOCK_SERVICE, ip: null, local: null},
           MOCK_SERVICE,
-          {...MOCK_SERVICE, name: 'bar', ip: null},
-          {...MOCK_SERVICE, name: 'bar', ip: null},
+          {...MOCK_SERVICE, name: 'bar', ip: null, local: null},
+          {...MOCK_SERVICE, name: 'bar', ip: null, local: null},
+        ],
+        expected: [
+          MOCK_SERVICE,
+          {...MOCK_SERVICE, name: 'bar', ip: null, local: null},
         ],
-        expected: [MOCK_SERVICE, {...MOCK_SERVICE, name: 'bar', ip: null}],
       },
     ]
 
diff --git a/discovery-client/src/__tests__/service.test.js b/discovery-client/src/__tests__/service.test.js
index e7457f89430..2c6bebb1246 100644
--- a/discovery-client/src/__tests__/service.test.js
+++ b/discovery-client/src/__tests__/service.test.js
@@ -23,6 +23,7 @@ describe('service utils', () => {
           name: 'name',
           ip: 'ip',
           port: 1234,
+          local: false,
           ok: false,
           serverOk: false,
           advertising: false,
@@ -37,6 +38,7 @@ describe('service utils', () => {
           name: 'name',
           ip: null,
           port: 31950,
+          local: null,
           ok: null,
           serverOk: null,
           advertising: null,
@@ -44,6 +46,11 @@ describe('service utils', () => {
           serverHealth: null,
         },
       },
+      {
+        name: 'link-local IPv4',
+        args: ['name', '169.254.1.2'],
+        expected: expect.objectContaining({local: true}),
+      },
     ]
 
     SPECS.forEach(spec => {
@@ -90,6 +97,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: MOCK_BROWSER_SERVICE.addresses[0],
           port: MOCK_BROWSER_SERVICE.port,
+          local: false,
           ok: null,
           serverOk: null,
           advertising: true,
@@ -109,6 +117,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: '192.168.1.42',
           port: 31950,
+          local: false,
           ok: null,
           serverOk: null,
           advertising: true,
@@ -127,6 +136,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: '[fe80::caf4:6db4:4652:e975]',
           port: 31950,
+          local: true,
           ok: null,
           serverOk: null,
           advertising: true,
@@ -163,6 +173,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: '192.168.1.42',
           port: 31950,
+          local: false,
           ok: true,
           serverOk: true,
           advertising: null,
@@ -181,6 +192,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: '192.168.1.42',
           port: 31950,
+          local: false,
           ok: false,
           serverOk: true,
           advertising: null,
@@ -199,6 +211,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: '192.168.1.42',
           port: 31950,
+          local: false,
           ok: true,
           serverOk: false,
           advertising: null,
@@ -217,6 +230,7 @@ describe('service utils', () => {
           name: 'opentrons-dev',
           ip: '192.168.1.42',
           port: 31950,
+          local: false,
           ok: false,
           serverOk: true,
           advertising: null,
diff --git a/discovery-client/src/service.js b/discovery-client/src/service.js
index 88f37ac240e..510b1085907 100644
--- a/discovery-client/src/service.js
+++ b/discovery-client/src/service.js
@@ -16,6 +16,15 @@ import type {
 const nameExtractor = (st: ServiceType) =>
   new RegExp(`^(.+)\\._${st.name}\\._${st.protocol}`)
 
+const isLocal = (ip: ?string) => {
+  if (ip == null) return null
+  return (
+    ip.startsWith('169.254') ||
+    ip.startsWith('[fe80') ||
+    ip.startsWith('[fd00') ||
+    ip === 'localhost'
+  )
+}
 export const DEFAULT_PORT = 31950
 
 export function makeService (
@@ -32,6 +41,7 @@ export function makeService (
     name,
     ip: defaultTo(ip, null),
     port: defaultTo(port, DEFAULT_PORT),
+    local: isLocal(ip),
     ok: defaultTo(ok, null),
     serverOk: defaultTo(serverOk, null),
     advertising: defaultTo(advertising, null),
diff --git a/discovery-client/src/types.js b/discovery-client/src/types.js
index d78e7139c79..4b5f2ffd9a0 100644
--- a/discovery-client/src/types.js
+++ b/discovery-client/src/types.js
@@ -26,6 +26,8 @@ export type Service = {
   name: string,
   ip: ?string,
   port: number,
+  // IP address (if known) is a link-local address
+  local: ?boolean,
   // GET /health response.ok === true
   ok: ?boolean,
   // GET /server/health response.ok === true

From ab2298dfbb915af9cde33abdef3a163bb85324a6 Mon Sep 17 00:00:00 2001
From: Mike Cousins <mike@opentrons.com>
Date: Thu, 4 Oct 2018 16:17:21 -0400
Subject: [PATCH 3/4] fixup: remove unused types

---
 app/src/discovery/index.js | 5 +----
 app/src/discovery/types.js | 9 ---------
 2 files changed, 1 insertion(+), 13 deletions(-)
 delete mode 100644 app/src/discovery/types.js

diff --git a/app/src/discovery/index.js b/app/src/discovery/index.js
index 17d3b56ff30..3128ea94776 100644
--- a/app/src/discovery/index.js
+++ b/app/src/discovery/index.js
@@ -5,7 +5,6 @@ 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]: Array<Service>}
 
@@ -29,8 +28,6 @@ type UpdateListAction = {|
   payload: {|robots: Array<Service>|},
 |}
 
-export * from './types'
-
 export type DiscoveryAction = StartAction | FinishAction | UpdateListAction
 
 const DISCOVERY_TIMEOUT = 20000
@@ -48,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}}
 // }
diff --git a/app/src/discovery/types.js b/app/src/discovery/types.js
deleted file mode 100644
index 24db3e19ffc..00000000000
--- a/app/src/discovery/types.js
+++ /dev/null
@@ -1,9 +0,0 @@
-// @flow
-import type {Service} from '@opentrons/discovery-client'
-
-export type Connection = Service & {local: boolean}
-
-export type DiscoveredRobot = {
-  name: string,
-  connections: Array<Connection>,
-}

From 0fb255a4f506f57d47780c8cf19f708ed624c282 Mon Sep 17 00:00:00 2001
From: Mike Cousins <mike@opentrons.com>
Date: Fri, 5 Oct 2018 11:34:15 -0400
Subject: [PATCH 4/4] fixup: null out local when ip is nulled out

---
 discovery-client/src/__tests__/client.test.js |  4 ++--
 .../src/__tests__/service-list.test.js        | 20 +++++++++++++++++--
 discovery-client/src/service.js               |  2 +-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/discovery-client/src/__tests__/client.test.js b/discovery-client/src/__tests__/client.test.js
index 7f7c1dc22f0..1c7f77cb8c7 100644
--- a/discovery-client/src/__tests__/client.test.js
+++ b/discovery-client/src/__tests__/client.test.js
@@ -351,8 +351,8 @@ describe('discovery client', () => {
         health: {name: 'opentrons-dev'},
         serverHealth: {name: 'opentrons-dev'},
       },
-      {...MOCK_SERVICE, name: 'bar', ip: null},
-      {...MOCK_SERVICE, name: 'baz', ip: null},
+      {...MOCK_SERVICE, name: 'bar', ip: null, local: null},
+      {...MOCK_SERVICE, name: 'baz', ip: null, local: null},
     ])
   })
 
diff --git a/discovery-client/src/__tests__/service-list.test.js b/discovery-client/src/__tests__/service-list.test.js
index fb3acdf43ff..bf4d750e0e8 100644
--- a/discovery-client/src/__tests__/service-list.test.js
+++ b/discovery-client/src/__tests__/service-list.test.js
@@ -48,6 +48,7 @@ describe('serviceList', () => {
             ...MOCK_SERVICE,
             name: 'bar',
             ip: null,
+            local: null,
             health: null,
             serverHealth: {apiServerVersion: '1.0.0'},
           },
@@ -55,6 +56,7 @@ describe('serviceList', () => {
             ...MOCK_SERVICE,
             name: 'baz',
             ip: null,
+            local: null,
             health: {api_version: '1.0.0'},
             serverHealth: {apiServerVersion: '1.0.0'},
           },
@@ -122,8 +124,22 @@ describe('serviceList', () => {
         upsert: MOCK_SERVICE,
         expected: [
           MOCK_SERVICE,
-          {...MOCK_SERVICE, name: 'bar', ip: null, ok: null, serverOk: null},
-          {...MOCK_SERVICE, name: 'baz', ip: null, ok: null, serverOk: null},
+          {
+            ...MOCK_SERVICE,
+            name: 'bar',
+            ip: null,
+            local: null,
+            ok: null,
+            serverOk: null,
+          },
+          {
+            ...MOCK_SERVICE,
+            name: 'baz',
+            ip: null,
+            local: null,
+            ok: null,
+            serverOk: null,
+          },
         ],
       },
     ]
diff --git a/discovery-client/src/service.js b/discovery-client/src/service.js
index 510b1085907..5eab4c86d30 100644
--- a/discovery-client/src/service.js
+++ b/discovery-client/src/service.js
@@ -73,7 +73,7 @@ export function clearServiceIfConflict (
   update: ?ServiceUpdate
 ): Service {
   return update && service.ip === update.ip
-    ? {...service, ip: null, ok: null, serverOk: null}
+    ? {...service, ip: null, local: null, ok: null, serverOk: null}
     : service
 }