Add XY Stage Agent for LATRt Stages #153
Conversation
agents/xy_stage/xy_ocs_agent.py
Outdated
| self.is_moving = self.xy_stage.moving | ||
|
|
||
| data['data']['x'] = pos[0] | ||
| data['data']['y'] = pos[1] |
There was a problem hiding this comment.
Having 'is_moving in the data block might be nice for a grafana dashboard, or might be helpful for analysis later on.
There was a problem hiding this comment.
possibly, but adding it now is messing up the displays of the other data in grafana?
There was a problem hiding this comment.
It shouldn't effect other displays unless your querying all fields with a wildcard or something
agents/xy_stage/xy_ocs_agent.py
Outdated
| import numpy as np | ||
|
|
||
| ## yes I shouldn't have named that module agent | ||
| from xy_agent.xy_connect import XY_Stage |
There was a problem hiding this comment.
Also, where is this driver located? Any chance we can put it in the socs/agents directory?
There was a problem hiding this comment.
Drivers are here: https://github.com/kmharrington/xy_stage_control
I don't want to add the client side of the drivers to socs because I need to be able to use it outside ocs as well. And that would require maintaining two sets of code that do the exact same thing.
|
One more comment, so seems like k2so and sat are using a slightly different xy stage, so it'll also need it's own agent. Should we rename this so that it's unique to the latrt instead of a generic xy_stage? |
There was a problem hiding this comment.
This is look good. One thing that should be added is a docs file in socs/docs/agents/. I usually work from one of the other agent's files. Please include an example ocs site config file block for configuring this Agent, and a description for installing any dependencies (if xy_agent doesn't make it into socs.)
If the plan is to build a Docker container for this, we should do that and include an example docker-compose block in the docs as well.
|
I think the recent commit fix all the comments? |
There was a problem hiding this comment.
Sorry, one other comment to do with naming consistency among Agents that I missed in the first review but noticed after the renaming of the Agent class.
I also think if we're renaming to avoid future name conflicts, then the xy_stage/ directory name should be changed while we're at it.
|
Alright, I think that fixes the other comments? |
docs/agents/latrt_xy_stage.rst
Outdated
| configuration file. Here is an example configuration block using all of | ||
| the available arguments:: | ||
|
|
||
| {'agent-class': 'LATRt_XY_StageAgent', |
There was a problem hiding this comment.
Needs underscores removed, and this needs to match the agent_class passed to site_config.parse_args(), so it should be 'LATRtXYAgent'.
There was a problem hiding this comment.
Great, I push one final fix to make the agent class name consistent with what you're using in your ocs-site-configs definitions. These need to match so that ocs can identify which configuration block to use for your Agent. I also renamed the class to keep consistent, even though this isn't technically required it is what that string is meant to reference.
Going to merge once checks pass one last time.
Description
Adds an agent for interacting with the XY Stage controller we're using with the LATRt.
Motivation and Context
We want position readout in
.g3files. Which means we also need to control the device through OCS.How Has This Been Tested?
It's been tested for the use-cases we need for beam mapping. The lower level controller has the safety features.
Types of changes
Checklist:
developbranch.