fix(discovery-client): Ensure IPs are actually de-duped (#2404)

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

@@ -182,14 +182,20 @@ describe('app-shell/discovery', () => {
 
     mockClient.services = [{name: 'foo'}, {name: 'bar'}]
     mockClient.emit('service')
-    expect(Store.__store.set).toHaveBeenCalledWith('services', [
+    expect(Store.__store.set).toHaveBeenLastCalledWith('services', [
       {name: 'foo'},
       {name: 'bar'},
     ])
+  })
+
+  test('stores services to file on serviceRemoved events', () => {
+    registerDiscovery(dispatch)
 
     mockClient.services = [{name: 'foo'}]
     mockClient.emit('serviceRemoved')
-    expect(Store.__store.set).toHaveBeenCalledWith('services', [{name: 'foo'}])
+    expect(Store.__store.set).toHaveBeenLastCalledWith('services', [
+      {name: 'foo'},
+    ])
   })
 
   test('loads services from file on client initialization', () => {

app-shell/src/discovery.js

@@ -1,10 +1,11 @@
 // @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 Store from 'electron-store'
 
 import DiscoveryClient, {
   SERVICE_EVENT,
@@ -18,32 +19,38 @@ 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'
+import type {
+  DiscoveredRobot,
+  Connection,
+} from '@opentrons/app/src/discovery/types'
 
 const log = createLogger(__filename)
 
 // TODO(mc, 2018-08-09): values picked arbitrarily and should be researched
-const FAST_POLL_INTERVAL = 3000
-const SLOW_POLL_INTERVAL = 15000
+const FAST_POLL_INTERVAL_MS = 3000
+const SLOW_POLL_INTERVAL_MS = 15000
+const UPDATE_THROTTLE_MS = 500
 
 let config
 let store
 let client
 
 export function registerDiscovery (dispatch: Action => void) {
+  const onServiceUpdate = throttle(handleServices, UPDATE_THROTTLE_MS)
+
   config = getConfig('discovery')
   store = new Store({name: 'discovery', defaults: {services: []}})
 
   client = DiscoveryClient({
-    pollInterval: SLOW_POLL_INTERVAL,
+    pollInterval: SLOW_POLL_INTERVAL_MS,
     logger: log,
     candidates: ['[fd00:0:cafe:fefe::1]'].concat(config.candidates),
     services: store.get('services'),
   })
 
   client
-    .on(SERVICE_EVENT, handleServices)
-    .on(SERVICE_REMOVED_EVENT, handleServices)
+    .on(SERVICE_EVENT, onServiceUpdate)
+    .on(SERVICE_REMOVED_EVENT, onServiceUpdate)
     .on('error', error => log.error('discovery error', {error}))
     .start()
 
@@ -53,9 +60,9 @@ export function registerDiscovery (dispatch: Action => void) {
     switch (action.type) {
       case 'discovery:START':
         handleServices()
-        return client.setPollInterval(FAST_POLL_INTERVAL).start()
+        return client.setPollInterval(FAST_POLL_INTERVAL_MS).start()
       case 'discovery:FINISH':
-        return client.setPollInterval(SLOW_POLL_INTERVAL)
+        return client.setPollInterval(SLOW_POLL_INTERVAL_MS)
     }
   }
 

app/src/discovery/index.js

@@ -31,7 +31,7 @@ export * from './types'
 
 export type DiscoveryAction = StartAction | FinishAction | UpdateListAction
 
-const DISCOVERY_TIMEOUT = 15000
+const DISCOVERY_TIMEOUT = 20000
 
 export function startDiscovery (): ThunkAction {
   const start: StartAction = {type: 'discovery:START', meta: {shell: true}}

discovery-client/Makefile

@@ -8,13 +8,10 @@ PATH := $(shell cd .. && yarn bin):$(PATH)
 # source and output directories for main process code
 src_dir := src
 lib_dir := lib
-src_ignore := "**/__@(tests|mocks)__/**"
+src_ignore := "**/__@(tests|mocks|fixtures)__/**"
 babel := babel $(src_dir) --ignore $(src_ignore) --out-dir $(lib_dir)
 flow_copy := flow-copy-source --ignore $(src_ignore) $(src_dir) $(lib_dir)
 
-# set NODE_ENV for a command with $(env)=environment
-env := cross-env NODE_ENV
-
 # standard targets
 #####################################################################
 
@@ -33,8 +30,9 @@ clean:
 #####################################################################
 
 .PHONY: lib
+lib: export NODE_ENV := production
 lib:
-	$(env)=production $(babel)
+	$(babel)
 	$(flow_copy)
 
 .PHONY: dist

discovery-client/package.json

@@ -23,6 +23,7 @@
   },
   "dependencies": {
     "escape-string-regexp": "^1.0.5",
+    "lodash": "^4.17.4",
     "mdns-js": "^1.0.1",
     "node-fetch": "^2.1.2",
     "to-regex": "^3.0.2",

discovery-client/src/__fixtures__/mdns-browser-service.js

@@ -0,0 +1,18 @@
+export default {
+  addresses: ['192.168.1.42'],
+  query: ['_http._tcp.local'],
+  type: [
+    {
+      name: 'http',
+      protocol: 'tcp',
+      subtypes: [],
+      description: 'Web Site',
+    },
+  ],
+  txt: [''],
+  port: 31950,
+  fullname: 'opentrons-dev._http._tcp.local',
+  host: 'opentrons-dev.local',
+  interfaceIndex: 0,
+  networkInterface: 'en0',
+}

discovery-client/src/__fixtures__/service.js

@@ -0,0 +1,7 @@
+export default {
+  name: 'opentrons-dev',
+  ip: '192.168.1.42',
+  port: 31950,
+  ok: null,
+  serverOk: null,
+}