Oriental Motor BLH controller agent for stimulator #690
Conversation
There was a problem hiding this comment.
Thank you for the PR! This looks great overall. Many of my comments will be similar/identical to @BrianJKoopman's review of #694.
Since this agent is using a direct serial connection, and you've tested using a single board computer, I imagine this will not be run on the daq-lat node as most other agents. Other cases where this occurs would be the LS240s, whose agents are usually run on a NUC that either has direct USB connection or USB-to-fiber adapters. What is the plan for this agent?
There was a problem hiding this comment.
Thanks for the documentation! Could you add a "Supporting APIs" section for drivers.py?
| 'instance-id': 'blh', | ||
| 'arguments': [ '--port', '/dev/ttyACM0'] | ||
| }, | ||
|
|
There was a problem hiding this comment.
Add a "Docker Compose" section here to include an example of the docker compose configuration.
There was a problem hiding this comment.
Remember to use """triple double quotes""", not single quotes, for docstrings.
| ''' | ||
| Parameters | ||
| ---------- | ||
| port : string | ||
| Port to connect | ||
| ''' |
There was a problem hiding this comment.
Move up to class docstring.
|
|
||
| def start_acq(self, session, params): | ||
| '''Starts acquiring data. | ||
| ''' |
There was a problem hiding this comment.
Please standardize the naming and docstrings according to https://ocs.readthedocs.io/en/main/developer/agent_references/documentation.html#task-and-process-documentation.
|
|
||
| return True, 'Set values for BLH' | ||
|
|
||
| def start_rotation(self, session, params=None): |
There was a problem hiding this comment.
Similar to other tasks, you can add a param decorator here.
| self._blh.start(forward=forward) | ||
|
|
||
| return True, 'BLH rotation started.' |
There was a problem hiding this comment.
You should handle the case where the start command failed to run properly, thus the return statement that the rotation started would be incorrect. See my comment in drivers.py.
| forward : bool, default True | ||
| Move forward if True | ||
| ''' | ||
| self._wr(MOVE_FWD if forward else MOVE_BKW) |
There was a problem hiding this comment.
If this returns something, you should return it at the end of this function too. This could then be handled in the agent to see whether the command worked as intended.
|
|
||
| def stop(self): | ||
| '''Stop rotation''' | ||
| self._wr(STOP) |
There was a problem hiding this comment.
Same comment as for start.
| self._blh.stop() | ||
|
|
||
| return True, 'BLH rotation stop command was published.' |
There was a problem hiding this comment.
Same comment as in start_rotation.
Description
This pull request adds a new agent to operate motor to be used in the stimulator for LAT.
The motor cotroller is Orienta Motor's BLH series with USB controll capability.
This agent communicate with the controller to start or stop rotation and change parameter settings such as rotation speed.
Motivation and Context
Since the BLH series motor controller will be used in the LAT stimulator,
the agent should be contained in this repository.
How Has This Been Tested?
Test was done using an actual motor controller and an ARM-based single board computer.
Types of changes
Checklist: