API: add hardware control #2349
Conversation
The hardware_control submodule is the controller of the robot's hardware. Its API can move the robot at a level of abstraction that portrays the robot as a whole, not including the labware. In this PR, the module is a stub, and not used by default. Further PRs will introduce more functionality. Closes #2232
sfoster1
force-pushed
the
api_add-hardware-control
branch
from
784b1fa to
20c293f
Compare
Codecov Report
@@ Coverage Diff @@
## edge #2349 +/- ##
==========================================
+ Coverage 30.01% 30.62% +0.61%
==========================================
Files 508 509 +1
Lines 8126 8753 +627
==========================================
+ Hits 2439 2681 +242
- Misses 5687 6072 +385
Continue to review full report at Codecov.
|
| pass | ||
|
|
||
| @_log_call | ||
| async def cache_instrument_models(self): |
There was a problem hiding this comment.
Is there any way we could (or would want to) get rid of the concept of caching the instrument models? Does this HAL have enough state information to know when pinging smoothie for instrument models would be unnecessary / detrimental?
There was a problem hiding this comment.
This HAL would have enough information (though it would probably be implicit, like a smoothie communications lock) to know when pinging would be unnecessary. Some of the shape of the API duplicates what already exists; before committing to removing caching and always querying the hardware I'd want to think harder about the usage patterns that lead people to want to know what pipettes are attached, and when they want to know. In general I bias towards caching for communication that happens over slow serial links.
There was a problem hiding this comment.
Oops, I left out some important words. Definitely think caching instrument models is a good idea, what I meant to ask was "do we want to get rid of the concept of caching instrument models for clients of the hardware API", i.e. move all caching logic to be internal to this module (if possible)
There was a problem hiding this comment.
We could reword the api methods but I think we need to keep a method for clients to explicitly request a cache update at least for the hardware controlled by smoothie, since smoothie can't tell us asynchronously when pipettes change.
|
|
||
| # Global actions API | ||
| @_log_call | ||
| async def pause(self): |
There was a problem hiding this comment.
Would love to chat about this. I'm not convinced pause and resume belong in a hardware_control module. In my mind they could be the domain of some sort of action queuing system that uses hardware_control but is not a part of it.
halt is a different story because a halt is a signal that gets sent to smoothie. If this pause and resume was meant to be a "send a pause or resume signal to smoothie to pause the command that is being acted upon right this instant" then please ignore this comment
There was a problem hiding this comment.
The second one is the way I'm thinking about it. The key thing about this API is that none of these functions will ever be directly used by client or user code. There's always going to be another wrapper around this. So possibly we should rename some of these functions, but the safe assumption is always that even if the name duplicates some other existing or higher level functionality the implementation here will be at a lower level.
The docstrings and types that get filled out as the functionality gets fleshed out will reflect this.
|
|
||
| # Gantry/frame (i.e. not pipette) action API | ||
| @_log_call | ||
| async def home(self, *args, **kwargs): |
There was a problem hiding this comment.
Is the *args, **kwargs here just a copy paste thing? Or "we haven't decided this method shape yet"? I don't see them anywhere else in this file and it was my understanding that we'd be phasing them out in favor of named arguments
There was a problem hiding this comment.
Copy paste, absolutely. I wanted to just get the names of the API down here.
There was a problem hiding this comment.
I'm down with evicting a lot of our use of args and kwargs, but I think that can be done separately from this
| def __init__(self): | ||
| global _lock | ||
|
|
||
| self._tlock_acquired = _lock.acquire(blocking=False) |
There was a problem hiding this comment.
Very nitpicky request, but for clarity could we maybe expand the tlock and flock names to be a little more explicit? I assume it's thread_lock and file_lock respectively?
|
|
||
| # Gantry/frame (i.e. not pipette) action API | ||
| @_log_call | ||
| async def home(self, *args, **kwargs): |
There was a problem hiding this comment.
I'm down with evicting a lot of our use of args and kwargs, but I think that can be done separately from this
* fix(app): Open external links in browser instead of app window (#2327) * feat(api): Remove deck calibration from reset options (#2330) Remove deck calibration from factory reset options, as deck calibration reset should not be done outside of the context of recalibration * docs(contributing): Ask Windows users to set core.autocrlf=input (#2332) Setting core.autocrlf to something that inserts carriage returns on windows means that API pushes to robots from windows will have carriage returns in the scripts in api/opentrons/resources, which prevents the robot from booting. Closes #2305 * fix(api): Update definitions for tuberacks (#2317) * fix(api): Update definitions for 15ml tuberack, 50ml tuberack, and 15/50ml tuberack to match actual Closes #2290 * Modify definitions based on practical test * feat(app): Enable autoupdate on Linux by switching to AppImage builds (#2329) Closes #2303 * chore(chore): add exception in gitattributes for our hex files (#2339) * fix(protocol-designer): fix whitescreen on deleting blowout labware (#2341) * fix unticketed bug: setting blowout to a trash-box in a step and deleting the trash box caused whitescreen *make labware dropdown blank when nonexistent labware selected * fix(api): Update the aluminum block definitions to match drawings (#2342) * fix(api): Update the aluminum block definitions to match drawings and experiments * Rename definition in shared-data to match rename in default_containers.json and fix z height * Closes #2292 * chore(release): 3.4.0 (#2338) * fix(protocol-designer): fix tiprack diagram only displaying right (#2340) Fix a small bug where the tiprack diagram in the new file modal would duplicate the diagram of the selected right pipette tiprack instead of showing both selections. * feat(api): add module firmware update endpoint (#2173) Closes #1654 Adds firmware update api methods and update server functions (for modules with old as well as new bootloader) Adds a udev rule for new bootloader Adds tests for modules and update_server endpoint * feat(protocol-designer): add "app build date" field to PD saved files (#2350) * chore(api): Cleanup unused api root level files (#2336) There were some leftovers from days of old: we don't use pip or anaconda or PyInstaller anymore. * refactor(api): invert control of system and server (#2318) Closes #2185 Separates out the previously entangled server and system functions so that opentrons.main handles system configuration while server.main only configures and runs the server. Entrypoint is switched from opentrons.server.main to opentrons.main * feat(api): Add hardware_control submodule (#2349) The hardware_control submodule is the controller of the robot's hardware. Its API can move the robot at a level of abstraction that portrays the robot as a whole, not including the labware. In this PR, the module is a stub, and not used by default Closes #2232 * fixup: More verbose names for file and thread locks * fixup: use functools.wraps in log_call decorator * ci(codecov): Fix uploading of python coverage (#2360) * fix(protocol-designer): tweak analytics copy for accuracy (#2366) Update the copy in the analytics disclaimer to accurately portray our data collection in beta.
overview
Add a new submodule called
hardware_controlthat will, going forward, be the unit that controls all of the hardware in the robot (the implementation of #2232 ).The
hardware_controlmodule is a HAL that presents an API at the lowest semantic level possible while still considering the OT2 as a "robot" rather than as a collection of disparate hardware. For instance, it considers the robot's motion systems as a coherent whole, but uses the absolute deck coordinate frame and does not know about labware. It presents an API on the level ofmove(specifying the gantry axes and each pipette axis),aspirateanddispense, andpick_up_tipanddrop_tip(over wherever the pipette happens to be, not moving to specific locations) but does not have the higher-level commands such asdistribute. It does not even have the arc motion planning.This module should be initialized once per robot (and there are mechanisms to make sure of this using thread and file locks) and should have a drop in simulating replacement.
As of now, until the rest of the functionality in the Robot Refactor milestone is complete, the module is not integrated into the rest of the system aside from its tests (on platforms other than Windows, anyway). This PR just establishes the structure.
Closes #2232