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

refactor(app): add move, positions, and disengage calls to robot-controls #4651

Merged
merged 1 commit into from
Dec 23, 2019

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Dec 19, 2019

overview

This PR follows up on the refactor work completed in #4440, #4477, #4544, #4599, #4615, #4631, #4646. The endpoints being recreated today are:

  • GET /robot/positions
  • POST /robot/move
  • POST /motors/disengage

This PR does not wire up these actions because it will involve a fairly heavy rewrite of Change Pipette. That will be its own PR. As of this PR, all necessary endpoints for the wizard are in the new epic-based robot-controls module!

changelog

  • Added movement controls to app/src/robot-controls
    • Added 3 actions (move + success / failure)
    • Added move and move error state to state.robotControls reducer
    • Added 1 epic (moveEpic) that:
      1. Calls GET /robot/positions
      2. Then calls POST /robot/move with result of GET /robot/positions
      3. Then calls POST /motors/disengage depending on the original action
    • Added tests for all of the above

review requests

Since this PR doesn't wire anything up to the UI, we can save functional testing for that upcoming PR. For this one, please make sure the code and added unit tests look reasonable!

If you have a robot handy, though, you can try this out in the redux devtools. Just make sure that the robot's attached pipettes are already in state

// move to change pipette position
{
  type: 'robotControls:MOVE',
  payload: {
    robotName: 'your-robot-name',
    position: 'changePipette',
    mount: 'left',
    disengageMotors: true
  },
  meta: {},
}

// move to attach tip pipette position
// requires attached pipettes in state to make the correct POST /robot/move request
{
  type: 'robotControls:MOVE',
  payload: {
    robotName: 'your-robot-name',
    position: 'attachTip',
    mount: 'left',
    disengageMotors: false
  },
  meta: {},
}

You should see the API calls in the network monitor. If disengageMotors is true, you should be able to move the pipette by hand once the calls complete. Afterwards, you should see a robotControls:MOVE_SUCCESS in the devtools.

@mcous mcous added app Affects the `app` project ready for review refactor labels Dec 19, 2019
@mcous mcous requested a review from a team December 19, 2019 22:53
@codecov
Copy link

codecov bot commented Dec 19, 2019

Codecov Report

Merging #4651 into edge will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #4651      +/-   ##
==========================================
+ Coverage   58.09%   58.12%   +0.03%     
==========================================
  Files         948      949       +1     
  Lines       26167    26216      +49     
==========================================
+ Hits        15201    15239      +38     
- Misses      10966    10977      +11
Impacted Files Coverage Δ
app/src/robot-controls/epic/index.js 100% <ø> (ø) ⬆️
app/src/robot-controls/reducer.js 93.33% <ø> (ø) ⬆️
app/src/robot-controls/actions.js 100% <100%> (ø) ⬆️
app/src/robot-controls/epic/moveEpic.js 100% <100%> (ø)
app/src/robot-controls/constants.js 100% <100%> (ø) ⬆️
protocol-designer/src/ui/modules/selectors.js 50% <0%> (ø) ⬆️
protocol-designer/src/ui/steps/actions/thunks.js 14.7% <0%> (+1.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae35d5d...d58c329. Read the comment docs.

Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants