refactor(app): Consolidate /motor calls into common API #2301
Conversation
app/src/http-api-client/index.js
Outdated
| @@ -6,7 +6,7 @@ import {calibrationReducer, type CalibrationAction} from './calibration' | |||
| import type {HealthAction} from './health' | |||
| import type {PipettesAction} from './pipettes' | |||
| import type {ModulesAction} from './modules' | |||
| import {motorsReducer, type MotorsAction} from './motors' | |||
| import {type MotorsAction} from './motors' | |||
There was a problem hiding this comment.
Style nitpick, but can we move type outside the braces to match the other ones?
app/src/http-api-client/motors.js
Outdated
| | MotorsRequestAction | ||
| | MotorsSuccessAction | ||
| | MotorsFailureAction | ||
| | ApiAction<'disengage', null, DisengageResponse> |
There was a problem hiding this comment.
This path should be motors/disengage and the request type should be DisengageRequest, not null
app/src/http-api-client/motors.js
Outdated
| error: ApiRequestError, | ||
| |}, | ||
| |} | ||
| type MotorsCall = ApiCall<DisengageRequest, DisengageResponse> |
There was a problem hiding this comment.
I would call this DisengageCall, not MotorsCall
app/src/http-api-client/motors.js
Outdated
| | ApiAction<'disengage', null, DisengageResponse> | ||
|
|
||
| export type MotorsState = { | ||
| motors?: MotorsCall, |
There was a problem hiding this comment.
Path should be motors/disengage, not motors
Codecov Report
@@ Coverage Diff @@
## edge #2301 +/- ##
==========================================
- Coverage 29.92% 29.89% -0.04%
==========================================
Files 500 500
Lines 8049 8045 -4
==========================================
- Hits 2409 2405 -4
Misses 5640 5640
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## edge #2301 +/- ##
==========================================
+ Coverage 29.92% 30.37% +0.44%
==========================================
Files 500 502 +2
Lines 8049 8326 +277
==========================================
+ Hits 2409 2529 +120
- Misses 5640 5797 +157
Continue to review full report at Codecov.
|
4 similar comments
Codecov Report
@@ Coverage Diff @@
## edge #2301 +/- ##
==========================================
+ Coverage 29.92% 30.37% +0.44%
==========================================
Files 500 502 +2
Lines 8049 8326 +277
==========================================
+ Hits 2409 2529 +120
- Misses 5640 5797 +157
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## edge #2301 +/- ##
==========================================
+ Coverage 29.92% 30.37% +0.44%
==========================================
Files 500 502 +2
Lines 8049 8326 +277
==========================================
+ Hits 2409 2529 +120
- Misses 5640 5797 +157
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## edge #2301 +/- ##
==========================================
+ Coverage 29.92% 30.37% +0.44%
==========================================
Files 500 502 +2
Lines 8049 8326 +277
==========================================
+ Hits 2409 2529 +120
- Misses 5640 5797 +157
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## edge #2301 +/- ##
==========================================
+ Coverage 29.92% 30.37% +0.44%
==========================================
Files 500 502 +2
Lines 8049 8326 +277
==========================================
+ Hits 2409 2529 +120
- Misses 5640 5797 +157
Continue to review full report at Codecov.
|
overview
Small refactor PR in to make future API work around connectivity state easier with regards to #2100.
Since
motors/disengagecalls get pipettes before disengage it does not implement buildRequestMaker.Motor disengage request/response is now stored in api state. Previously there was an empty motors reducer that is now removed.
changelog
review requests
Please test change pipette and make sure pipette motors disengage works as expected.