From 79fe2362b65409c68ae5644737dd3b086b6bffd8 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Fri, 6 Jul 2018 11:19:29 -0400 Subject: [PATCH] refactor(app): Use generic actions in /calibration API module (#1819) Removed the module specific api:CAL_REQUEST, etc actions in favor of api:REQUEST, etc. Making the calibration module more generic also meant moving the jog step size out of redux and into the state of the JogControls component, as had been discussed with @b-cooper --- .../CalibrateDeck/ConfirmPosition.js | 6 +- app/src/components/CalibrateDeck/index.js | 13 +- app/src/components/CalibrateDeck/types.js | 6 +- .../ConfirmPositionContents.js | 35 +-- app/src/components/JogControls/index.js | 44 ++-- .../__tests__/calibration.test.js | 142 +++++------ app/src/http-api-client/calibration.js | 223 ++++++------------ app/src/http-api-client/index.js | 4 +- app/src/http-api-client/robot.js | 10 +- app/src/http-api-client/settings.js | 6 +- app/src/robot/actions.js | 8 +- app/src/robot/api-client/client.js | 4 +- app/src/robot/reducer/calibration.js | 12 - app/src/robot/selectors.js | 4 - app/src/robot/test/actions.test.js | 9 +- app/src/robot/test/api-client.test.js | 9 +- .../robot/test/calibration-reducer.test.js | 48 ++-- app/src/robot/test/selectors.test.js | 9 - app/src/robot/test/session-reducer.test.js | 24 -- 19 files changed, 225 insertions(+), 391 deletions(-) diff --git a/app/src/components/CalibrateDeck/ConfirmPosition.js b/app/src/components/CalibrateDeck/ConfirmPosition.js index e627acd4d85..4e2b9a3e17b 100644 --- a/app/src/components/CalibrateDeck/ConfirmPosition.js +++ b/app/src/components/CalibrateDeck/ConfirmPosition.js @@ -13,11 +13,7 @@ export default function ConfirmPosition (props: Props) { return (
- + Save Calibration and Continue diff --git a/app/src/components/CalibrateDeck/index.js b/app/src/components/CalibrateDeck/index.js index 978daed6726..d3782f5009e 100644 --- a/app/src/components/CalibrateDeck/index.js +++ b/app/src/components/CalibrateDeck/index.js @@ -15,8 +15,6 @@ import { home, startDeckCalibration, deckCalibrationCommand, - setCalibrationJogStep, - getCalibrationJogStep, makeGetDeckCalibrationCommandState, makeGetDeckCalibrationStartState } from '../../http-api-client' @@ -127,11 +125,11 @@ function CalibrateDeck (props: CalibrateDeckProps) { ) } -function makeMapStateToProps () { +function makeMapStateToProps (): (state: State, ownProps: OP) => SP { const getDeckCalCommand = makeGetDeckCalibrationCommandState() const getDeckCalStartState = makeGetDeckCalibrationStartState() - return (state: State, ownProps: OP): SP => { + return (state, ownProps) => { const {robot} = ownProps const startRequest = getDeckCalStartState(state, robot) const pipetteInfo = startRequest.response && startRequest.response.pipette @@ -146,8 +144,7 @@ function makeMapStateToProps () { return { startRequest, pipetteProps, - commandRequest: getDeckCalCommand(state, robot), - jogStep: getCalibrationJogStep(state) + commandRequest: getDeckCalCommand(state, robot) } } } @@ -159,10 +156,6 @@ function mapDispatchToProps (dispatch: Dispatch, ownProps: OP): DP { jog: (axis, direction, step) => dispatch( deckCalibrationCommand(robot, {command: 'jog', axis, direction, step}) ), - onJogStepSelect: (event) => { - const step = Number(event.target.value) - dispatch(setCalibrationJogStep(step)) - }, forceStart: () => dispatch(startDeckCalibration(robot, true)), // exit button click in title bar, opens exit alert modal, confirm exit click exit: () => dispatch(chainActions( diff --git a/app/src/components/CalibrateDeck/types.js b/app/src/components/CalibrateDeck/types.js index d519cd635ee..4a54b6cc0ef 100644 --- a/app/src/components/CalibrateDeck/types.js +++ b/app/src/components/CalibrateDeck/types.js @@ -3,7 +3,7 @@ import type {Match} from 'react-router' import type {PipetteConfig} from '@opentrons/shared-data' import type {RobotService, Mount} from '../../robot' import type {DeckCalCommandState, DeckCalStartState} from '../../http-api-client' -import type {JogControlsProps} from '../JogControls' +import type {Jog} from '../JogControls' export type CalibrationStep = '1' | '2' | '3' | '4' | '5' | '6' @@ -16,7 +16,6 @@ export type OP = { export type SP = { startRequest: DeckCalStartState, commandRequest: DeckCalCommandState, - jogStep: $PropertyType, pipetteProps: ?{ mount: Mount, pipette: ?PipetteConfig, @@ -25,8 +24,7 @@ export type SP = { export type DP = { forceStart: () => mixed, - jog: $PropertyType, - onJogStepSelect: $PropertyType, + jog: Jog, exit: () => mixed, back: () => mixed } diff --git a/app/src/components/CalibrateLabware/ConfirmPositionContents.js b/app/src/components/CalibrateLabware/ConfirmPositionContents.js index 70618d1d416..c4a537bc2a3 100644 --- a/app/src/components/CalibrateLabware/ConfirmPositionContents.js +++ b/app/src/components/CalibrateLabware/ConfirmPositionContents.js @@ -3,31 +3,26 @@ import * as React from 'react' import {connect} from 'react-redux' -import type {State, Dispatch} from '../../types' +import type {Dispatch} from '../../types' import type {Instrument, Labware} from '../../robot' -import {selectors as robotSelectors, actions as robotActions} from '../../robot' +import {actions as robotActions} from '../../robot' import {PrimaryButton} from '@opentrons/components' import ConfirmPositionDiagram from './ConfirmPositionDiagram' -import JogControls, {type JogControlsProps} from '../JogControls' - -type SP = { - step: $PropertyType, -} +import JogControls, {type Jog} from '../JogControls' type DP = { - onConfirmClick: () => void, - jog: $PropertyType, - onStepSelect: $PropertyType, + onConfirmClick: () => mixed, + jog: Jog, } type OP = Labware & { calibrator: Instrument } -type Props = SP & DP & OP +type Props = DP & OP -export default connect(mapStateToProps, mapDispatchToProps)(ConfirmPositionContents) +export default connect(null, mapDispatchToProps)(ConfirmPositionContents) function ConfirmPositionContents (props: Props) { const {isTiprack, onConfirmClick, calibrator: {channels}} = props @@ -46,12 +41,6 @@ function ConfirmPositionContents (props: Props) { ) } -function mapStateToProps (state: State): SP { - return { - step: robotSelectors.getJogDistance(state) - } -} - function mapDispatchToProps (dispatch: Dispatch, ownProps: OP): DP { const {slot, isTiprack, calibrator: {mount}} = ownProps const onConfirmAction = isTiprack @@ -59,13 +48,9 @@ function mapDispatchToProps (dispatch: Dispatch, ownProps: OP): DP { : robotActions.updateOffset(mount, slot) return { - jog: (axis, direction) => { - dispatch(robotActions.jog(mount, axis, direction)) - }, - onStepSelect: (event) => { - const step = Number(event.target.value) - dispatch(robotActions.setJogDistance(step)) + jog: (axis, direction, step) => { + dispatch(robotActions.jog(mount, axis, direction, step)) }, - onConfirmClick: () => { dispatch(onConfirmAction) } + onConfirmClick: () => dispatch(onConfirmAction) } } diff --git a/app/src/components/JogControls/index.js b/app/src/components/JogControls/index.js index 63b48e6de07..5d1388262ff 100644 --- a/app/src/components/JogControls/index.js +++ b/app/src/components/JogControls/index.js @@ -15,7 +15,7 @@ import { import styles from './styles.css' -type Jog = (axis: JogAxis, direction: JogDirection, step: JogStep) => mixed +export type Jog = (axis: JogAxis, direction: JogDirection, step: JogStep) => mixed type JogButtonProps = { name: string, @@ -23,11 +23,9 @@ type JogButtonProps = { onClick: () => mixed, } -export type JogControlsProps = { - jog: Jog, - step: JogStep, - onStepSelect: (event: SyntheticInputEvent<*>) => mixed, -} +type Props = {jog: Jog} + +type State = {step: JogStep} const JOG_BUTTON_NAMES = ['left', 'right', 'back', 'forward', 'up', 'down'] @@ -49,28 +47,32 @@ const JOG_PARAMS_BY_NAME = { down: ['z', -1] } -const STEPS = [0.1, 1, 10] +const STEPS: Array = [0.1, 1, 10] const STEP_OPTIONS = STEPS.map(s => ({name: `${s} mm`, value: `${s}`})) -export default class JogControls extends React.Component { +export default class JogControls extends React.Component { + constructor (props: Props) { + super(props) + this.state = {step: STEPS[0]} + } + increaseStepSize = () => { - const current = STEPS.indexOf(this.props.step) - if (current < STEPS.length - 1) { - // $FlowFixMe: (mc, 2018-06-26) refactor so event trickery isn't needed - this.props.onStepSelect({target: {value: `${STEPS[current + 1]}`}}) - } + const i = STEPS.indexOf(this.state.step) + if (i < STEPS.length - 1) this.setState({step: STEPS[i + 1]}) } decreaseStepSize = () => { - const current = STEPS.indexOf(this.props.step) - if (current > 0) { - // $FlowFixMe: (mc, 2018-06-26) refactor so event trickery isn't needed - this.props.onStepSelect({target: {value: `${STEPS[current - 1]}`}}) - } + const i = STEPS.indexOf(this.state.step) + if (i > 0) this.setState({step: STEPS[i - 1]}) + } + + handleStepSelect = (event: SyntheticInputEvent<*>) => { + this.setState({step: Number(event.target.value)}) } getJogHandlers () { - const {jog, step} = this.props + const {jog} = this.props + const {step} = this.state return JOG_BUTTON_NAMES.reduce((result, name) => ({ ...result, @@ -79,8 +81,8 @@ export default class JogControls extends React.Component { } renderJogControls () { + const {step} = this.state const jogHandlers = this.getJogHandlers() - const {step, onStepSelect} = this.props return ( { className={styles.increment_item} value={`${step}`} options={STEP_OPTIONS} - onChange={onStepSelect} + onChange={this.handleStepSelect} disableKeypress /> diff --git a/app/src/http-api-client/__tests__/calibration.test.js b/app/src/http-api-client/__tests__/calibration.test.js index 042dda7fb90..d3960dd4be8 100644 --- a/app/src/http-api-client/__tests__/calibration.test.js +++ b/app/src/http-api-client/__tests__/calibration.test.js @@ -7,10 +7,8 @@ import { reducer, startDeckCalibration, deckCalibrationCommand, - setCalibrationJogStep, makeGetDeckCalibrationStartState, - makeGetDeckCalibrationCommandState, - getCalibrationJogStep + makeGetDeckCalibrationCommandState } from '..' jest.mock('../client') @@ -34,7 +32,7 @@ describe('/calibration/**', () => { }) describe('startDeckCalibration action creator', () => { - const path = 'deck/start' + const path = 'calibration/deck/start' const response = { token: 'token', pipette: {mount: 'left', model: 'p300_single_v1'} @@ -68,11 +66,11 @@ describe('/calibration/**', () => { )) }) - test('dispatches CAL_REQUEST and CAL_SUCCESS', () => { + test('dispatches api:REQUEST and api:SUCCESS', () => { const request = {force: false} const expectedActions = [ - {type: 'api:CAL_REQUEST', payload: {robot, request, path}}, - {type: 'api:CAL_SUCCESS', payload: {robot, response, path}} + {type: 'api:REQUEST', payload: {robot, request, path}}, + {type: 'api:SUCCESS', payload: {robot, response, path}} ] client.__setMockResponse(response) @@ -81,12 +79,12 @@ describe('/calibration/**', () => { .then(() => expect(store.getActions()).toEqual(expectedActions)) }) - test('dispatches CAL_REQUEST and CAL_FAILURE', () => { + test('dispatches api:REQUEST and api:FAILURE', () => { const request = {force: false} const error = {name: 'ResponseError', status: 409, message: ''} const expectedActions = [ - {type: 'api:CAL_REQUEST', payload: {robot, request, path}}, - {type: 'api:CAL_FAILURE', payload: {robot, error, path}} + {type: 'api:REQUEST', payload: {robot, request, path}}, + {type: 'api:FAILURE', payload: {robot, error, path}} ] client.__setMockError(error) @@ -97,13 +95,15 @@ describe('/calibration/**', () => { }) describe('deckCalibrationCommand action creator', () => { - const path = 'deck' + const path = 'calibration/deck' const token = 'mock-token' - const request = {command: 'release'} + const request = {command: 'save z'} const response = {message: 'mock-response'} beforeEach(() => { - state.api.calibration[NAME] = {'deck/start': {response: {token}}} + state.api.calibration[NAME] = { + 'calibration/deck/start': {response: {token}} + } }) test('calls POST /calibration/deck and adds token to request', () => { @@ -118,10 +118,10 @@ describe('/calibration/**', () => { )) }) - test('dispatches CAL_REQUEST and CAL_SUCCESS', () => { + test('dispatches api:REQUEST and api:SUCCESS', () => { const expectedActions = [ - {type: 'api:CAL_REQUEST', payload: {robot, request, path}}, - {type: 'api:CAL_SUCCESS', payload: {robot, response, path}} + {type: 'api:REQUEST', payload: {robot, request, path}}, + {type: 'api:SUCCESS', payload: {robot, response, path}} ] client.__setMockResponse(response) @@ -130,11 +130,11 @@ describe('/calibration/**', () => { .then(() => expect(store.getActions()).toEqual(expectedActions)) }) - test('dispatches CAL_REQUEST and CAL_FAILURE', () => { + test('dispatches api:REQUEST and api:FAILURE', () => { const error = {name: 'ResponseError', message: 'AH'} const expectedActions = [ - {type: 'api:CAL_REQUEST', payload: {robot, request, path}}, - {type: 'api:CAL_FAILURE', payload: {robot, error, path}} + {type: 'api:REQUEST', payload: {robot, request, path}}, + {type: 'api:FAILURE', payload: {robot, error, path}} ] client.__setMockError(error) @@ -142,14 +142,22 @@ describe('/calibration/**', () => { return store.dispatch(deckCalibrationCommand(robot, request)) .then(() => expect(store.getActions()).toEqual(expectedActions)) }) - }) - describe('non-API-call action creators', () => { - test('setCalibrationJogStep', () => { - const step = 4 - const expected = {type: 'api:SET_CAL_JOG_STEP', payload: {step}} + test('dispatchs api:CLEAR_RESPONSE if command is "release"', () => { + const request = {command: 'release'} + const expectedActions = [ + { + type: 'api:CLEAR_RESPONSE', + payload: {robot, path: 'calibration/deck/start'} + }, + {type: 'api:REQUEST', payload: {robot, request, path}}, + {type: 'api:SUCCESS', payload: {robot, response, path}} + ] + + client.__setMockResponse(response) - expect(setCalibrationJogStep(step)).toEqual(expected) + return store.dispatch(deckCalibrationCommand(robot, request)) + .then(() => expect(store.getActions()).toEqual(expectedActions)) }) }) @@ -160,7 +168,7 @@ describe('/calibration/**', () => { const REDUCER_REQUEST_RESPONSE_TESTS = [ { - path: 'deck/start', + path: 'calibration/deck/start', request: {}, response: { token: 'token', @@ -168,7 +176,7 @@ describe('/calibration/**', () => { } }, { - path: 'deck', + path: 'calibration/deck', request: {command: 'save transform'}, response: {message: 'saved'} } @@ -178,9 +186,9 @@ describe('/calibration/**', () => { const {path, request, response} = spec describe(`reducer with /calibration/${path}`, () => { - test('handles CAL_REQUEST', () => { + test('handles api:REQUEST', () => { const action = { - type: 'api:CAL_REQUEST', + type: 'api:REQUEST', payload: {path, robot, request} } @@ -196,9 +204,9 @@ describe('/calibration/**', () => { }) }) - test('handles CAL_SUCCESS', () => { + test('handles api:SUCCESS', () => { const action = { - type: 'api:CAL_SUCCESS', + type: 'api:SUCCESS', payload: {path, robot, response} } @@ -223,10 +231,10 @@ describe('/calibration/**', () => { }) }) - test('handles CAL_FAILURE', () => { + test('handles api:FAILURE', () => { const error = {message: 'we did not do it!'} const action = { - type: 'api:CAL_FAILURE', + type: 'api:FAILURE', payload: {path, robot, error} } @@ -252,45 +260,36 @@ describe('/calibration/**', () => { }) }) }) - - // TODO(mc, 2018-05-07): refactor this test according to the TODO in the - // CAL_SUCCESS handler in the reducer - test('CAL_RESPONSE with command: "release" clears token', () => { - state.calibration[NAME] = { - deck: {request: {command: 'release'}}, - 'deck/start': {response: {token: 'token'}} - } - - const action = { - type: 'api:CAL_SUCCESS', - payload: {robot, path: 'deck', response: {message: 'released'}} - } - - expect(reducer(state, action).calibration[NAME]['deck/start'].response) - .toBe(null) - }) }) - describe('reducer with non-API-call actions', () => { - beforeEach(() => { - state = state.api - }) + test('reducer with api:CLEAR_RESPONSE', () => { + state = state.api - test('handles SET_CAL_JOG_STEP', () => { - const action = setCalibrationJogStep(5) + const path = 'calibration/deck/start' + const action = {type: 'api:CLEAR_RESPONSE', payload: {robot, path}} - expect(reducer(state, action).calibration).toEqual({ - ...state.calibration, - jogStep: 5 - }) + state.calibration[NAME] = { + [path]: { + inProgress: false, + request: {}, + response: 'foo', + error: 'bar' + } + } + + expect(reducer(state, action).calibration[NAME][path]).toEqual({ + inProgress: false, + request: {}, + response: null, + error: null }) }) describe('selectors', () => { beforeEach(() => { state.api.calibration[NAME] = { - deck: {inProgress: true}, - 'deck/start': {inProgress: true} + 'calibration/deck': {inProgress: true}, + 'calibration/deck/start': {inProgress: true} } }) @@ -298,13 +297,10 @@ describe('/calibration/**', () => { const getStartState = makeGetDeckCalibrationStartState() expect(getStartState(state, robot)) - .toEqual(state.api.calibration[NAME]['deck/start']) + .toEqual(state.api.calibration[NAME]['calibration/deck/start']) expect(getStartState(state, {name: 'foo'})).toEqual({ - inProgress: false, - error: null, - request: null, - response: null + inProgress: false }) }) @@ -312,19 +308,11 @@ describe('/calibration/**', () => { const getCommandState = makeGetDeckCalibrationCommandState() expect(getCommandState(state, robot)) - .toEqual(state.api.calibration[NAME].deck) + .toEqual(state.api.calibration[NAME]['calibration/deck']) expect(getCommandState(state, {name: 'foo'})).toEqual({ - inProgress: false, - error: null, - request: null, - response: null + inProgress: false }) }) - - test('getCalibrationJogStep', () => { - state.api.calibration.jogStep = 42 - expect(getCalibrationJogStep(state)).toBe(42) - }) }) }) diff --git a/app/src/http-api-client/calibration.js b/app/src/http-api-client/calibration.js index 4a2106b4708..275229eb213 100644 --- a/app/src/http-api-client/calibration.js +++ b/app/src/http-api-client/calibration.js @@ -4,7 +4,9 @@ import {createSelector, type Selector} from 'reselect' import type {State, Action, ThunkPromiseAction} from '../types' import type {BaseRobot, RobotService, Mount} from '../robot' import type {ApiCall, ApiRequestError} from './types' +import type {ApiRequestAction, ApiSuccessAction, ApiFailureAction} from './actions' +import {apiRequest, apiSuccess, apiFailure, clearApiResponse} from './actions' import client from './client' export type JogAxis = 'x' | 'y' | 'z' @@ -17,9 +19,9 @@ export type DeckCalPoint = '1' | '2' | '3' export type DeckCalMovePoint = 'attachTip' | 'safeZ' | DeckCalPoint -type DeckStartRequest = { +type DeckStartRequest = {| force?: boolean -} +|} type DeckStartResponse = { token: string, @@ -43,72 +45,43 @@ type DeckCalResponse = { message: string, } +type CalPath = + | 'calibration/deck/start' + | 'calibration/deck' + type CalRequest = DeckStartRequest | DeckCalRequest type CalResponse = DeckStartResponse | DeckCalResponse -type RequestPath = - | 'deck/start' - | 'deck' - -type CalRequestAction = {| - type: 'api:CAL_REQUEST', - payload: {| - robot: RobotService, - path: RequestPath, - request: CalRequest, - |}, -|} - -type CalSuccessAction = {| - type: 'api:CAL_SUCCESS', - payload: {| - robot: RobotService, - path: RequestPath, - response: CalResponse, - |}, -|} - -type CalFailureAction = {| - type: 'api:CAL_FAILURE', - payload: {| - robot: RobotService, - path: RequestPath, - error: ApiRequestError, - |}, -|} - -type SetCalJogStepAction = {| - type: 'api:SET_CAL_JOG_STEP', - payload: {| - step: JogStep, - |}, -|} - export type CalibrationAction = - | CalRequestAction - | CalSuccessAction - | CalFailureAction - | SetCalJogStepAction + | ApiRequestAction + | ApiSuccessAction + | ApiFailureAction export type DeckCalStartState = ApiCall export type DeckCalCommandState = ApiCall type RobotCalState = { - 'deck/start'?: DeckCalStartState, - deck?: DeckCalCommandState, + 'calibration/deck/start'?: DeckCalStartState, + 'calibration/deck'?: DeckCalCommandState, } type CalState = { - jogStep: JogStep, [robotName: string]: ?RobotCalState, } -const DECK: 'deck' = 'deck' -const DECK_START: 'deck/start' = 'deck/start' +const DECK: 'calibration/deck' = 'calibration/deck' +const DECK_START: 'calibration/deck/start' = 'calibration/deck/start' -const DEFAULT_JOG_STEP = 0.1 +// TODO(mc, 2018-07-05): flow helper until we have one reducer, since +// p === 'constant' checks but p === CONSTANT does not, even if +// CONSTANT is defined as `const CONSTANT: 'constant' = 'constant'` +function getCalPath (p: string): ?CalPath { + if (p === 'calibration/deck/start' || p === 'calibration/deck') return p + + return null +} export function startDeckCalibration ( robot: RobotService, @@ -117,11 +90,13 @@ export function startDeckCalibration ( const request = {force} return (dispatch) => { - dispatch(calRequest(robot, DECK_START, request)) + dispatch(apiRequest(robot, DECK_START, request)) - return client(robot, 'POST', `calibration/${DECK_START}`, request) - .then((res: DeckStartResponse) => calSuccess(robot, DECK_START, res)) - .catch((err: ApiRequestError) => calFailure(robot, DECK_START, err)) + return client(robot, 'POST', DECK_START, request) + .then( + (res: DeckStartResponse) => apiSuccess(robot, DECK_START, res), + (err: ApiRequestError) => apiFailure(robot, DECK_START, err) + ) .then(dispatch) } } @@ -135,36 +110,33 @@ export function deckCalibrationCommand ( const startState = getStartStateFromCalState(state) const token = startState.response && startState.response.token - dispatch(calRequest(robot, DECK, request)) + if (request.command === 'release') { + dispatch(clearApiResponse(robot, DECK_START)) + } + + dispatch(apiRequest(robot, DECK, request)) - return client(robot, 'POST', `calibration/${DECK}`, {...request, token}) - .then((res: DeckCalResponse) => calSuccess(robot, DECK, res)) - .catch((err: ApiRequestError) => calFailure(robot, DECK, err)) + return client(robot, 'POST', DECK, {...request, token}) + .then( + (res: DeckCalResponse) => apiSuccess(robot, DECK, res), + (err: ApiRequestError) => apiFailure(robot, DECK, err) + ) .then(dispatch) } } -export function setCalibrationJogStep (step: JogStep): SetCalJogStepAction { - return {type: 'api:SET_CAL_JOG_STEP', payload: {step}} -} - export function calibrationReducer ( state: ?CalState, action: Action ): CalState { - if (!state) return {jogStep: DEFAULT_JOG_STEP} - - let name - let path - let request - let response - let error - let stateByName + if (!state) return {} switch (action.type) { - case 'api:CAL_REQUEST': - ({path, request, robot: {name}} = action.payload) - stateByName = state[name] || {} + case 'api:REQUEST': { + const path = getCalPath(action.payload.path) + if (!path) return state + const {payload: {request, robot: {name}}} = action + const stateByName = state[name] || {} return { ...state, @@ -173,58 +145,55 @@ export function calibrationReducer ( [path]: {request, inProgress: true, response: null, error: null} } } + } + + case 'api:SUCCESS': { + const path = getCalPath(action.payload.path) + if (!path) return state + const {payload: {response, robot: {name}}} = action + const stateByName = state[name] || {} + const stateByPath = stateByName[path] || {} - case 'api:CAL_SUCCESS': - ({path, response, robot: {name}} = action.payload) - stateByName = state[name] || {} - - // TODO(mc, 2018-05-07): this has a race condition if a new /deck request - // is fired w/ one already in flight; disallow this - if ( - path === DECK && stateByName[DECK] && stateByName[DECK].request && - stateByName[DECK].request.command === 'release' - ) { - stateByName = { + return { + ...state, + [name]: { ...stateByName, - [DECK_START]: { - ...stateByName[DECK_START], - error: null, - response: null - } + [path]: {...stateByPath, response, inProgress: false, error: null} } } + } + + case 'api:FAILURE': { + const path = getCalPath(action.payload.path) + if (!path) return state + const {payload: {error, robot: {name}}} = action + const stateByName = state[name] || {} + const stateByPath = stateByName[path] || {} return { ...state, [name]: { ...stateByName, - [path]: { - ...stateByName[path], - response, - inProgress: false, - error: null - } + [path]: {...stateByPath, error, inProgress: false} } } + } - case 'api:CAL_FAILURE': - ({path, error, robot: {name}} = action.payload) - stateByName = state[name] || {} + case 'api:CLEAR_RESPONSE': { + const path = getCalPath(action.payload.path) + if (!path) return state + const {payload: {robot: {name}}} = action + const stateByName = state[name] || {} + const stateByPath = stateByName[path] || {} return { ...state, [name]: { ...stateByName, - [path]: { - ...stateByName[path], - error, - inProgress: false - } + [path]: {...stateByPath, error: null, response: null} } } - - case 'api:SET_CAL_JOG_STEP': - return {...state, jogStep: action.payload.step} + } } return state @@ -248,52 +217,14 @@ export function makeGetDeckCalibrationCommandState () { return sel } -export function getCalibrationJogStep (state: State): JogStep { - return state.api.calibration.jogStep -} - function getRobotCalState (state: State, props: BaseRobot): RobotCalState { return state.api.calibration[props.name] || {} } function getStartStateFromCalState (state: RobotCalState): DeckCalStartState { - return state[DECK_START] || { - inProgress: false, - error: null, - request: null, - response: null - } + return state[DECK_START] || {inProgress: false} } function getDeckStateFromCalState (state: RobotCalState): DeckCalCommandState { - return state[DECK] || { - inProgress: false, - error: null, - request: null, - response: null - } -} - -function calRequest ( - robot: RobotService, - path: RequestPath, - request: CalRequest -): CalRequestAction { - return {type: 'api:CAL_REQUEST', payload: {robot, path, request}} -} - -function calSuccess ( - robot: RobotService, - path: RequestPath, - response: CalResponse -): CalSuccessAction { - return {type: 'api:CAL_SUCCESS', payload: {robot, path, response}} -} - -function calFailure ( - robot: RobotService, - path: RequestPath, - error: ApiRequestError -): CalFailureAction { - return {type: 'api:CAL_FAILURE', payload: {robot, path, error}} + return state[DECK] || {inProgress: false} } diff --git a/app/src/http-api-client/index.js b/app/src/http-api-client/index.js index 98e6dafd317..d9b54278c3f 100644 --- a/app/src/http-api-client/index.js +++ b/app/src/http-api-client/index.js @@ -87,10 +87,8 @@ export type Action = export { startDeckCalibration, deckCalibrationCommand, - setCalibrationJogStep, makeGetDeckCalibrationStartState, - makeGetDeckCalibrationCommandState, - getCalibrationJogStep + makeGetDeckCalibrationCommandState } from './calibration' export { diff --git a/app/src/http-api-client/robot.js b/app/src/http-api-client/robot.js index 537b919d170..044bae993b4 100644 --- a/app/src/http-api-client/robot.js +++ b/app/src/http-api-client/robot.js @@ -104,11 +104,13 @@ type RobotState = { // note: POSITIONS only used inside `moveRobotTo` const POSITIONS = 'robot/positions' -const MOVE: RobotPath = 'robot/move' -const HOME: RobotPath = 'robot/home' -const LIGHTS: RobotPath = 'robot/lights' +const MOVE: 'robot/move' = 'robot/move' +const HOME: 'robot/home' = 'robot/home' +const LIGHTS: 'robot/lights' = 'robot/lights' -// TODO(mc, 2018-07-03): flow helper until we have one reducer +// TODO(mc, 2018-07-03): flow helper until we have one reducer, since +// p === 'constant' checks but p === CONSTANT does not, even if +// CONSTANT is defined as `const CONSTANT: 'constant' = 'constant'` function getRobotPath (p: string): ?RobotPath { if (p === 'robot/move' || p === 'robot/home' || p === 'robot/lights') { return p diff --git a/app/src/http-api-client/settings.js b/app/src/http-api-client/settings.js index aaf7c8abc50..361ed236cbc 100644 --- a/app/src/http-api-client/settings.js +++ b/app/src/http-api-client/settings.js @@ -45,9 +45,9 @@ export type SettingsState = { const SETTINGS: 'settings' = 'settings' -// TODO(mc, 2018-07-03): flow helper until we have one reducer -// note: p === 'settings' works but p === SETTINGS does not, even if -// SETTINGS is defined as `const SETTINGS: 'settings' = 'settings'` +// TODO(mc, 2018-07-03): flow helper until we have one reducer, since +// p === 'constant' checks but p === CONSTANT does not, even if +// CONSTANT is defined as `const CONSTANT: 'constant' = 'constant'` function getSettingsPath (p: string): ?SettingsPath { if (p === 'settings') return p diff --git a/app/src/robot/actions.js b/app/src/robot/actions.js index b7424d00a99..55dfd7222a2 100755 --- a/app/src/robot/actions.js +++ b/app/src/robot/actions.js @@ -91,7 +91,8 @@ export type PipetteCalibrationAction = {| payload: {| mount: Mount, axis?: Axis, - direction?: Direction + direction?: Direction, + step?: number |}, meta: {| robotCommand: true @@ -451,11 +452,12 @@ export const actions = { jog ( mount: Mount, axis: Axis, - direction: Direction + direction: Direction, + step: number ): PipetteCalibrationAction { return { type: 'robot:JOG', - payload: {mount, axis, direction}, + payload: {mount, axis, direction, step}, meta: {robotCommand: true} } }, diff --git a/app/src/robot/api-client/client.js b/app/src/robot/api-client/client.js index 9372b6ca512..a9d3af7f2da 100755 --- a/app/src/robot/api-client/client.js +++ b/app/src/robot/api-client/client.js @@ -222,9 +222,9 @@ export default function client (dispatch) { } function jog (state, action) { - const {payload: {mount, axis, direction}} = action + const {payload: {mount, axis, direction, step}} = action const instrument = selectors.getInstrumentsByMount(state)[mount] - const distance = selectors.getJogDistance(state) * direction + const distance = step * direction // FIXME(mc, 2017-10-06): DEBUG CODE // return setTimeout(() => dispatch(actions.jogResponse()), 1000) diff --git a/app/src/robot/reducer/calibration.js b/app/src/robot/reducer/calibration.js index 4c3768bb507..eed7242a5a4 100755 --- a/app/src/robot/reducer/calibration.js +++ b/app/src/robot/reducer/calibration.js @@ -39,7 +39,6 @@ type CalibrationRequest = { export type State = { +deckPopulated: ?boolean, - +jogDistance: number, +probedByMount: {[Mount]: boolean}, +tipOnByMount: {[Mount]: boolean}, @@ -62,7 +61,6 @@ const { const INITIAL_STATE: State = { deckPopulated: null, - jogDistance: 0.1, // TODO(mc, 2018-01-22): combine these into subreducer probedByMount: {}, @@ -138,9 +136,6 @@ export default function calibrationReducer ( case 'robot:DISCONNECT_RESPONSE': return handleDisconnectResponse(state, action) - case 'robot:SET_JOG_DISTANCE': - return handleSetJogDistance(state, action) - case 'robot:REFRESH_SESSION': return handleSession(state, action) @@ -485,13 +480,6 @@ function handleConfirmTiprackFailure ( } } -function handleSetJogDistance (state: State, action: any) { - return { - ...state, - jogDistance: Number(action.payload) - } -} - function handleJog (state: State, action: PipetteCalibrationAction): State { const {payload: {mount}} = action diff --git a/app/src/robot/selectors.js b/app/src/robot/selectors.js index 0f95c1cddfe..211f76b6054 100644 --- a/app/src/robot/selectors.js +++ b/app/src/robot/selectors.js @@ -432,10 +432,6 @@ export function getOffsetUpdateInProgress (state: State): boolean { return request.type === 'UPDATE_OFFSET' && request.inProgress } -export function getJogDistance (state: State): number { - return calibration(state).jogDistance -} - // get current instrument selector factory // to be used by a react-router Route component export const makeGetCurrentInstrument = () => createSelector( diff --git a/app/src/robot/test/actions.test.js b/app/src/robot/test/actions.test.js index a4a40fce5fc..6d6c890b6af 100644 --- a/app/src/robot/test/actions.test.js +++ b/app/src/robot/test/actions.test.js @@ -298,19 +298,14 @@ describe('robot actions', () => { expect(actions.moveToResponse(new Error('AH'))).toEqual(failure) }) - test('set jog distance action', () => { - const expected = {type: 'robot:SET_JOG_DISTANCE', payload: 10} - expect(actions.setJogDistance(10)).toEqual(expected) - }) - test('JOG action', () => { const expected = { type: 'robot:JOG', - payload: {mount: 'left', axis: 'x', direction: -1}, + payload: {mount: 'left', axis: 'x', direction: -1, step: 10}, meta: {robotCommand: true} } - expect(actions.jog('left', 'x', -1)).toEqual(expected) + expect(actions.jog('left', 'x', -1, 10)).toEqual(expected) }) test('jog response action', () => { diff --git a/app/src/robot/test/api-client.test.js b/app/src/robot/test/api-client.test.js index 782b05f037e..f8d59ec618b 100755 --- a/app/src/robot/test/api-client.test.js +++ b/app/src/robot/test/api-client.test.js @@ -422,8 +422,7 @@ describe('api client', () => { calibration: { calibrationRequest: {}, confirmedBySlot: {}, - labwareBySlot: {}, - jogDistance: constants.JOG_DISTANCE_FAST_MM + labwareBySlot: {} }, session: { instrumentsByMount: { @@ -670,7 +669,7 @@ describe('api client', () => { }) test('handles JOG success', () => { - const action = actions.jog('left', 'y', -1) + const action = actions.jog('left', 'y', -1, 10) const expectedResponse = actions.jogResponse() mockResolvedValue(calibrationManager.jog) @@ -680,7 +679,7 @@ describe('api client', () => { .then(() => { expect(calibrationManager.jog).toHaveBeenCalledWith( {_id: 'inst-2'}, - -constants.JOG_DISTANCE_FAST_MM, + -10, 'y' ) expect(dispatch).toHaveBeenCalledWith(expectedResponse) @@ -688,7 +687,7 @@ describe('api client', () => { }) test('handles JOG failure', () => { - const action = actions.jog('left', 'x', 1) + const action = actions.jog('left', 'x', 1, 10) const expectedResponse = actions.jogResponse(new Error('AH')) mockRejectedValue(calibrationManager.jog, new Error('AH')) diff --git a/app/src/robot/test/calibration-reducer.test.js b/app/src/robot/test/calibration-reducer.test.js index 395e42d852f..90756191f29 100644 --- a/app/src/robot/test/calibration-reducer.test.js +++ b/app/src/robot/test/calibration-reducer.test.js @@ -1,25 +1,32 @@ // calibration reducer tests import {reducer, actionTypes} from '../' +const EXPECTED_INITIAL_STATE = { + deckPopulated: null, + + // intrument probed + basic tip-tracking flags + // TODO(mc, 2018-01-22): combine these into subreducer + probedByMount: {}, + tipOnByMount: {}, + + // labware confirmed flags + confirmedBySlot: {}, + + // TODO(mc, 2018-01-22): make this state a sub-reducer + calibrationRequest: {type: '', inProgress: false, error: null} +} + describe('robot reducer - calibration', () => { test('initial state', () => { - const state = reducer(undefined, {}).calibration + const state = reducer(undefined, {}) - expect(state).toEqual({ - deckPopulated: null, - jogDistance: 0.1, - - // intrument probed + basic tip-tracking flags - // TODO(mc, 2018-01-22): combine these into subreducer - probedByMount: {}, - tipOnByMount: {}, + expect(state.calibration).toEqual(EXPECTED_INITIAL_STATE) + }) - // labware confirmed flags - confirmedBySlot: {}, + test('handles robot:REFRESH_SESSION', () => { + const state = reducer({calibration: {}}, {type: 'robot:REFRESH_SESSION'}) - // TODO(mc, 2018-01-22): make this state a sub-reducer - calibrationRequest: {type: '', inProgress: false, error: null} - }) + expect(state.calibration).toEqual(EXPECTED_INITIAL_STATE) }) test('handles DISCONNECT_RESPONSE success', () => { @@ -511,19 +518,6 @@ describe('robot reducer - calibration', () => { }) }) - test('handles SET_JOG_DISTANCE action', () => { - const state = { - calibration: { - jogDistance: 10 - } - } - - const action = {type: 'robot:SET_JOG_DISTANCE', payload: 1} - expect(reducer(state, action).calibration).toEqual({ - jogDistance: 1 - }) - }) - test('handles JOG action', () => { const state = { calibration: { diff --git a/app/src/robot/test/selectors.test.js b/app/src/robot/test/selectors.test.js index efdf4d17f9f..26a70c99d14 100644 --- a/app/src/robot/test/selectors.test.js +++ b/app/src/robot/test/selectors.test.js @@ -26,7 +26,6 @@ const { getUnconfirmedTipracks, getUnconfirmedLabware, getNextLabware, - getJogDistance, makeGetCurrentInstrument } = selectors @@ -427,14 +426,6 @@ describe('robot selectors', () => { }) }) - test('get jog distance', () => { - const state = makeState({ - calibration: {jogDistance: constants.JOG_DISTANCE_SLOW_MM} - }) - - expect(getJogDistance(state)).toBe(constants.JOG_DISTANCE_SLOW_MM) - }) - test('get calibrator mount', () => { const leftState = makeState({ session: { diff --git a/app/src/robot/test/session-reducer.test.js b/app/src/robot/test/session-reducer.test.js index 77dcbdf3cc4..7ac96c2f89f 100644 --- a/app/src/robot/test/session-reducer.test.js +++ b/app/src/robot/test/session-reducer.test.js @@ -77,34 +77,11 @@ describe('robot reducer - session', () => { }) test('handles robot:REFRESH_SESSION action', () => { - const INITIAL_CALIBRATION_STATE = { - deckPopulated: null, - jogDistance: 0.1, - - // TODO(mc, 2018-01-22): combine these into subreducer - probedByMount: {}, - tipOnByMount: {}, - - confirmedBySlot: {}, - - calibrationRequest: {type: '', inProgress: false, error: null} - } - const state = { session: { sessionRequest: {inProgress: false, error: null}, startTime: 40, runTime: 42 - }, - calibration: { - deckPopulated: true, - jogDistance: 1, - probedByMount: { - left: true - }, - confirmedBySlot: { - 9: true - } } } const action = {type: 'robot:REFRESH_SESSION'} @@ -114,7 +91,6 @@ describe('robot reducer - session', () => { startTime: null, runTime: 0 }) - expect(reducer(state, action).calibration).toEqual(INITIAL_CALIBRATION_STATE) }) test('handles SESSION_RESPONSE success', () => {