-
Notifications
You must be signed in to change notification settings - Fork 178
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
API work to enable calibration experience #352
Changes from 5 commits
23b3035
43445de
ef16b61
0210be5
3aaebf8
3ddf837
ece74ea
b8da76f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -748,7 +748,7 @@ def pick_up_tip(self, location=None, presses=3): | |
presses = (1 if not helpers.is_number(presses) else presses) | ||
|
||
@commands.publish.both(command=commands.pick_up_tip) | ||
def _pick_up_tip(location): | ||
def _pick_up_tip(location, self=self): | ||
self.motor.move(self._get_plunger_position('bottom')) | ||
self.current_volume = 0 | ||
|
||
|
@@ -813,7 +813,7 @@ def drop_tip(self, location=None, home_after=True): | |
location = location.bottom(self._drop_tip_offset) | ||
|
||
@commands.publish.both(command=commands.drop_tip) | ||
def _drop_tip(location): | ||
def _drop_tip(location, self=self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this not match the signature for @commands.publish.both(command=commands.pick_up_tip)
def _pick_up_tip(location, instrument=self): There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
if location: | ||
self.move_to(location, strategy='arc') | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,10 @@ | |
import sys | ||
import logging | ||
from opentrons.server.rpc import Server | ||
from opentrons.session import SessionManager | ||
from opentrons.session import Shell | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't love the name Also, re: outgrowing the scope of Session, I like the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make it 'opentrons.api' |
||
from logging.config import dictConfig | ||
|
||
|
||
log = logging.getLogger(__name__) | ||
|
||
|
||
|
@@ -77,7 +78,7 @@ def log_init(): | |
# TODO(artyom, 20170828): consider moving class name definition into | ||
# command line arguments, so one could us as a shell starting various | ||
# RPC servers with different root objects from a command line | ||
server = Server(SessionManager()) | ||
server = Server(Shell()) | ||
print( | ||
'Started Opentrons API Server listening at ws://{host}:{port}/' | ||
.format(host=host, port=port)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
from .session import Session, SessionManager | ||
from .shell import Shell | ||
from .calibration import CalibrationManager | ||
|
||
__all__ = [ | ||
Session, | ||
SessionManager | ||
SessionManager, | ||
Shell, | ||
CalibrationManager | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
PIPETTE_CHANGE_POSITION = (50, 50, 50) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's this for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will clean up. |
||
|
||
|
||
class CalibrationManager: | ||
def __init__(self, robot, loop=None): | ||
self._loop = loop | ||
self._robot = robot | ||
|
||
def tip_probe(self, instrument): | ||
raise NotImplemented() | ||
|
||
def move_to_front(self, instrument): | ||
# instrument.move_to(PIPETTE_CHANGE_POSITION) | ||
raise NotImplemented() | ||
|
||
def move_to(self, instrument, obj): | ||
# instrument.move_to(obj[0]) | ||
raise NotImplemented() | ||
|
||
def jog(self, instrument, coordinates): | ||
# instrument.jog(coordinates) | ||
raise NotImplemented() | ||
|
||
def update_container_offset(self): | ||
raise NotImplemented() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this
self
parameter toinstrument
(as it is used everywhere)? Also, does it make more sense to have instrument be the first parameter?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^Agreed, using self outside of the context of a class is confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked to @mcous IRL. The signature is this because of the decorator. Possibly we could have implemented actual commands as functions with 'instrument' as an argument, but since they are not, and since the decorator is magic we'll keep magic inside of it and map 'self' to instrument. That will change command builder's signature from 'self' to 'instrument'.