Expose STARBackend.move_channel_probe_z()#961
Expose STARBackend.move_channel_probe_z()#961BioCam wants to merge 10 commits intoPyLabRobot:mainfrom
STARBackend.move_channel_probe_z()#961Conversation
Bypasses the C0 master module (KZ) by sending ZA directly to the pip channel, enabling Z moves with configurable speed/acceleration even when the firmware's tip-picked-up flag is incorrectly set. Also updates `move_channel_z` docstring.
There was a problem hiding this comment.
Pull request overview
This PR exposes a new low-level per-channel Z movement API on the Hamilton STAR backend, intended to bypass master-module routed Z moves when the instrument’s internal “tip picked up” state causes incorrect behavior.
Changes:
- Clarifies
move_channel_zsemantics in its docstring (tip-dependent reference point). - Adds
STARBackend.move_channel_probe_z()to move an individual channel’s probe Z-drive directly via the channel module.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@rickwierenga, @ben-ray, @cmoscy, @jrast Should we call this 
|
| async def move_channel_z(self, channel: int, z: float): | ||
| """Move a channel in the z direction.""" | ||
| """Move a channel in the Z direction. | ||
|
|
||
| The Hamilton firmware interprets this Z position based on its internal | ||
| "tip mounted" state for the specified channel. When the firmware state | ||
| indicates that no tip is mounted, the absolute Z position refers to the | ||
| bottom of the stop disc. In that case, this command is effectively | ||
| equivalent to :meth:`move_channel_probe_z` for the same numeric Z value. | ||
|
|
||
| When the firmware state indicates that a tip is mounted on the channel, | ||
| the same Z position instead refers to the physical end of the tip. In | ||
| this case, the numeric Z value used with this method may differ from the | ||
| probe Z position used with :meth:`move_channel_probe_z` for the same | ||
| physical height above the deck. | ||
| """ | ||
| await self.position_single_pipetting_channel_in_z_direction( |
There was a problem hiding this comment.
should we make this only for the case when tips are mounted, and ask people to use move_channel_probe_z for other cases? this conditional method behavior is confusing
There was a problem hiding this comment.
Yes I think you're right - should be a separate PR though
There was a problem hiding this comment.
Lets do it here since they go nicely together
Also we can consider renaming tips method to move tip bottom or something to make it extra clear
There was a problem hiding this comment.
Are you sure?
STARBackend. move_channel_z() is a core method and will therefore require an extended deprecation period away from currently expected behaviour
I use it in every aP
There was a problem hiding this comment.
It's not uncommon to account for the length of a mounted tool/effector when calling move commands on other hardware too. So it seems like STARBackend. move_channel_z() is plenty clear if it stays with it's current behavior/naming.
It is great to have the STARBackend.move_channel_probe_z() as a clear way to explicitly move wrt the stop disc though, regardless of tip state.
Just my 2c
There was a problem hiding this comment.
sounds ambiguous to me to have move_channel_z and move_channel_probe_z. It's a bad API. We can cleanly transition this in the v1 change through the legacy module if we add this to v1b1. But also a simple deprecation warning for a method rename is easy enough
There was a problem hiding this comment.
Fair enough, maybe something like adding an arg with_respect_to ['end','stopdisc']. 1 function, with a clear callout of what you're moving the channel with respect to?
There was a problem hiding this comment.
I generally like separate functions better than parameters fundamentally changing a functions behavior
| warnings.warn( | ||
| "move_channel_z is deprecated. Use move_channel_stop_disk_z() for moves without a tip attached " | ||
| "or move_channel_tool_z() when a tip/tool is attached.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| await self.position_single_pipetting_channel_in_z_direction( | ||
| pipetting_channel_index=channel + 1, z_position=round(z * 10) | ||
| ) | ||
|
|
There was a problem hiding this comment.
why not use the method you tell people to use?
No description provided.