Fix resume and reconnect #346
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## app-3-0-alpha #346 +/- ##
=================================================
+ Coverage 82.09% 82.42% +0.32%
=================================================
Files 86 86
Lines 4877 4917 +40
=================================================
+ Hits 4004 4053 +49
+ Misses 873 864 -9
Continue to review full report at Codecov.
|
|
The last |
|
thanks, @IanLondon . Builds seem to be passing now. |
mcous
suggested changes
Oct 2, 2017
There was a problem hiding this comment.
This PR is a little too long lived, sorry for such a delay in my review. Unfortunately, because of its age, my commits in the api-client and api-client test files are going to conflict with v3 alpha work. I don't know how you want to handle that. But otherwise, I'd like to see this merged, provided we untrack some files that belong to #347.
Dockerfile.template
Outdated
| @@ -0,0 +1,24 @@ | |||
| # base-image for python on any machine using a template variable, | |||
There was a problem hiding this comment.
This Dockerfile seems out of scope for this PR (in-scope for #347). Can we remove it from this PR to reduce later diff/merge conflict noise?
(To be clear, I'm not suggesting deleting these files, just removing them from tracking in this branch)
api/scripts/compute/start.sh
Outdated
| @@ -0,0 +1,7 @@ | |||
| #!/bin/bash | |||
api/scripts/compute/start_server.py
Outdated
| @@ -0,0 +1,64 @@ | |||
| #!/usr/bin/env python | |||
data-user_storage.mount
Outdated
| @@ -0,0 +1,11 @@ | |||
| [Unit] | |||
There was a problem hiding this comment.
What is this? I'm suspecting I have the same comment as the Dockerfile on this one
…w concurrent calls; add test for concurrent calls
…us connections and disconnects
….py, remove task from the queue once done
…te remote objects in parallel for array
astaff
force-pushed
the
app-3-0-fix-resume-reconnect
branch
from
62bd7a3
to
1bd30cc
Compare
mcous
approved these changes
Oct 3, 2017
astaff
added a commit
that referenced
this pull request
Oct 24, 2017
* Add react-router (with redux bindings) to UI * Reorganize container structure with routes - Removes Root container in favor of more specific containers - Adds ConnectedRunControls container for run controls - Adds some basic routing to App.js (which is now top-level comp) * implement run button in run button header, style updates * Scaffold out routes for pipette and deck setup * clean up run controls layout * cleanup sidebar layout, fix run log scroll * minor style fiddling * fix linting errors * Dispatch a successful connect response if already connected * initial pipetteConfig component and styles * static single pipette config component * API work to enable calibration experience (#352) * Implement `get_contaieners` and `get_instruments` and underlying code to accumulate and interpret command messages. * Introduce `MainRouter` as a root client API object and make it contain `calibration_manager` and `session_manager`. * Remove notification handling from `SessionManager` and `Session` and move it up to `MainRouter`, clean up tests. * Add model data classes for labware and instruments to limit scope of client API surface. * Add `location` to commands where appropriate and change `publish` to translate `self` to `location` if needed. * Consolidate Client API related code under `opentrons.api`. * wire up mock props pipette config container * fix linting errors * render pipettes toggle active pipette route in pipette config * added tip probe messages as bottom panel * shrink sidebar slightly, default tip probe state * Fix API make install task and update client remote for MainRouter * Fix resume and reconnect (#346) * start task instead of await in websocket message handler loop to allow concurrent calls; add test for concurrent calls * add test stub for concurrent connections * fix crashes on disconnect, add tests for concurrent connections and disconnect * send messages to different clients independently, test for simultaneous connects and disconnects * refactor how send queues and tasks are being managed, improve tests * Add run timer cleanup to client disconnect * change levels for logging messages, change logging level in main.py handler, remove task from the queue once done * make robot.smoothie_driver private * move init of seen to top, to have it initiaized before we try to create remote objects in parallel for array * Resolve array remotes serially to avoid type resolution race * Calibration API Implementation (#366) * add calibration API tests, make _simulate call exec instead of relying on run with no arguments * add test for serializing class object attributes * Harmonize shape of notifications, reduce state tree * Make notification payload to be a copy of self across the board * Remove Wells from Container data object to reduce Session state tree * Refactor `placeable.get_container` * Don't treat `WellSeries` as container * Make `placeable.get_trace()` a generator * Add functional test with full protocol to test session model * V3a calibration nav panel (#363) * separate container for SetupPanel * list pipette links with optional axis params, list labware with optional params - only show once pipettes are probed * setup panel layout updates, conditionally render labware links and run link * layout cleanup * conditionally disable non tipracks until tipracks have been confirmed * add proptypes * map -> reduce * fix proptype * always render laware list, disable until intruments are calibrated * (v3 alpha, feature) Calibration API client state (#368) * Add instrument and labware state translation to API client * Add private ID field to RemoteObjects * Add actions for CalibrationManager calls and responses * Wire calibration actions and responses to API client * Track calibration API request state in redux store * V3a ui calibrate labware (#369) * separate container for SetupPanel * list pipette links with optional axis params, list labware with optional params - only show once pipettes are probed * initial deckmap for rework * confirm deck layout, styles * separate deckmap function * initial jod modal nested route, container, component. Remove position, replace with numerical slots, add ids for legacy slot names * convert labware wrapper elements to links, add active container/link styling * jog modal container, component, styling, and placeholder cotent and route * cleanup labware labels * jog modal button placeholder, general cleanup * render default pipette imgs, reset mock state after demo, sync tip prob and labware calibration notifications to state * (v3 alpha) Calibration UI MVP (#372) * static jog buttons for wriring * minor ui style updates, nav icons now router links, splash screen for upload page * render placeholder images for bradford assay labware * remove navlinks from sidebar, labware, pipettes in favor of current instrument and current labware * Wire up instrument calibration and some of labware confirmation * Wire some labware selectors to SetupPanel and DeckConfig * Wire labware review state+button, simplify bool prop names * update setup panel styles, default splash screen and route * nest tip probe notifications within pipette config * fix lint, troubleshoot Prepare for tip probe props * Wire up labware confirmation for all Yes clicks (no jog) * Wire up jog modal and confirmation button / reducers * labware selected, disabled, active, confirmed styles * Fix API calls for calibration and run (and unit tests) * Fix lint errors in Labware.js and .css * Driver, Pose Tracker and Calibration procedure (#374) * driver started * Implements working pipette probing Create a script that serves as a functional version of pipette probing which still needs to be logically placed into correct location in code base. Begin implementation of Gantry and Base classes. * Expand on Probe and Base classes * enable OTtwo to run existing protocols * set the slot and mount offsets * Enable probing with both pipettes Adjust offsets and probe position / dimensions to enable probing for both pipettes. Also adds mounts to pipettes (this is a hack) to enable offset values to be pulled from instruments so they can be used in calibration. This update of the global dicts should also allow calibration to persist between probing and running. * corrects deck offset * driver started * Implements working pipette probing Create a script that serves as a functional version of pipette probing which still needs to be logically placed into correct location in code base. Begin implementation of Gantry and Base classes. * Expand on Probe and Base classes * enable OTtwo to run existing protocols * set the slot and mount offsets * Enable probing with both pipettes Adjust offsets and probe position / dimensions to enable probing for both pipettes. Also adds mounts to pipettes (this is a hack) to enable offset values to be pulled from instruments so they can be used in calibration. This update of the global dicts should also allow calibration to persist between probing and running. * corrects deck offset * driver and api integration work for the demo * added axis to models.Instrument * add jog functionality * change jog function name * remove robot from session, make it home before run * use reset() instead of hard reset in robot test fixture * Use driver.(dis)connect instead of toggling simulating flag directly * Resin for app-3-0 (Chore) (#347) * attempting image build * attempting image build * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * refactor server start * refactor server start * refactor server start * refactor server start * refactor server start * adding motion testing_script * resin image build * resin image build * resin image build * resin image build * resin README added * slight refactor for PR * docs adjustment * docs adjustment * moved filed * linting fix * Change server port and fix life-testing script * add TODOs to Dockerfile.template * Resin for app-3-0 (Chore) (#347) * atempting image build * attempting image build * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * fixing Dockerfile.template * refactor server start * refactor server start * refactor server start * refactor server start * refactor server start * adding motion testing_script * resin image build * resin image build * resin image build * resin image build * resin README added * slight refactor for PR * docs adjustment * docs adjustment * moved filed * linting fix * Change server port and fix life-testing script * add TODOs to Dockerfile.template * make ot_data live on USB on pi * flip well positions for database migration * Functional tests for API - Added position tracking and position log for driver when running in simulate mode for testing purposes - Re-wired calibration_manager.jog to call calibration_functions.jog_instrument properly - Added API functional tests in api/test_functional.py consistent with common use cases. - Added bradford assay edited to fit OT-2 deck * rotate containers for ot-alpha and fix well offsets * Make moving more consistent and tidy things up Make instrument movement more consistent by using it's mover instead of the driver. Also remove a few print statements and an unused functionality in the driver. * prevent homing of nonexistent pip * fix tests * modify target values for homing * Add methods for updating z lenth of pipette in pose tracker instead of adding on the fly * use correct variable in direct movement strategy * adjust pipette position with tip offset * fix ui calibration setting and track relative position * Fixes for relative path access in posetracker, add all files starting with . to gitignore * fix logical errors in relative position access * adjust offsets for avagadro and toss probing deltas * adjust offsets in prep for KV demo * fix height of non-moving pipette during move * functioning calibration cli * output cleanup * move pipette by applying transformation from deck branch of a posetracker bypassing instrument branch of pose-tracker, adjust container definitions based on real life tests, add smoke test to api/test_integration, change tip pickup move sequence and travel, change tip offset to be applied to move command instead of being tracked in posetracker * functioning calibration cli * output cleanup * move pipette by applying transformation from deck branch of a posetracker bypassing instrument branch of pose-tracker, adjust container definitions based on real life tests, add smoke test to api/test_integration, change tip pickup move sequence and travel, change tip offset to be applied to move command instead of being tracked in posetracker * refactorings after review with Ben * integrate factory calibration with pose tracker, make changes in pose tracker to account for transformation alongside with translation, fix tests * 1) Improved smoothie driver to not send duplicate commands if we are already at the point for an axis 2) caching position so we don’t have to make serial calls every time we need position, reduces the risk of error in serial comms which are shaky 3) Fix a bug that was introduced after we made `position` a tuple, after which `+` operator did concatenation, not a vector sum 4) Improved `api/test_integration` to aggregate move logs by axis for easier analysis 5) Added test to confirm trashbox is present in session containers when drop_tip is called with no arguments in the protocol (cc @mike @jared) 6) Fixed api tests WRT to connect attempts being made to actual device - * Add back in assert statement for test * making smoke test work, refactoring calibration tests, refactoring tip probe * fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
When API client disconnects without closing the socket, the server will stop responding once the send buffer overflow. This results in user not being able to reconnect to the robot running a protocol. This is more noticeable on larger protocols, like Bradford Assay, where the buffer is more likely to overflow.
This PR includes:
Changelog
(Fix) Refactored send_loop from being a single task message pump to being a task with send queue created for each client connected. This way, socket send task awaiting buffer to drain will not block other clients from receiving messages.
(Tests)
server/test_integration.pynow opens two sockets, closes socket Error in Documentation #1 and proceeds using socket 20160831 031700 polling motor driver #2 to simulate reconnect.server/test_server.pytests multiple clients connecting, one of them disconnecting and confirming that the rest of the clients received all responses and notifications.Review requests
@mcous please confirm the fix works for you in the app.