Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 5, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app-shell/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
59 changes: 3 additions & 56 deletions app-shell/src/__tests__/discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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},
],
},
],
},
]

Expand All @@ -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},
})
})
)
Expand Down
49 changes: 2 additions & 47 deletions app-shell/src/discovery.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)

Expand Down Expand Up @@ -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>) {
Expand All @@ -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'
)
}
30 changes: 18 additions & 12 deletions app/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
#####################################################################
Expand All @@ -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
10 changes: 2 additions & 8 deletions app/src/analytics/__tests__/make-event.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}],
},
},
})
Expand Down
16 changes: 8 additions & 8 deletions app/src/discovery/__tests__/reducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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},
]),
}))

Expand All @@ -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}],
},
},
},
Expand All @@ -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}],
},
},
},
Expand Down
21 changes: 7 additions & 14 deletions app/src/discovery/index.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand All @@ -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}}
// }
Expand Down Expand Up @@ -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')
}
12 changes: 0 additions & 12 deletions app/src/discovery/types.js

This file was deleted.

2 changes: 1 addition & 1 deletion app/src/health-check/__tests__/health-check.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('health check', () => {
},
discovery: {
robotsByName: {
[name]: {name, connections: [{ip, port, ok: true}]},
[name]: [{ip, port, ok: true}],
},
},
healthCheck: {
Expand Down
10 changes: 5 additions & 5 deletions app/src/robot/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading