# Check position limits for Hexapod move and offset commands

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• Sprint:
TSSW Sprint - Jan 21 - Feb 01
• Team:
Telescope and Site

#### Description

The main telescope Hexapod should check limits for position and offset commands and reject the command if out of bounds.

#### Activity

Hide
Russell Owen added a comment -

Eric Coughlin I filed this ticket when you reported to me that the Hexapod was not enforcing position limits after restricting the limits via the configureLimits. I am now looking at the code and I'm not sure how this can be happening. I would like more input from you.

Here is the relevant bit of code for move in hexapod_csc.py:

  def _make_position_set_command(self, data):  """Make a POSITION_SET command for the low-level controller  using data from the move or moveLUT CSC command.  """  utils.check_symmetrical_range(data.x, "x", self.server.config.pos_limits[0],  ExceptionClass=salobj.ExpectedError)  utils.check_symmetrical_range(data.y, "y", self.server.config.pos_limits[0],  ExceptionClass=salobj.ExpectedError)  utils.check_range(data.z, "z", self.server.config.pos_limits[1],  self.server.config.pos_limits[2],  ExceptionClass=salobj.ExpectedError)  utils.check_symmetrical_range(data.u, "u", self.server.config.pos_limits[3],  ExceptionClass=salobj.ExpectedError)  utils.check_symmetrical_range(data.v, "v", self.server.config.pos_limits[3],  ExceptionClass=salobj.ExpectedError)  utils.check_range(data.w, "w", self.server.config.pos_limits[4],  self.server.config.pos_limits[5],  ExceptionClass=salobj.ExpectedError)  return self.make_command(code=enums.CommandCode.POSITION_SET,  param1=data.x,  param2=data.y,  param3=data.z,  param4=data.u,  param5=data.v,  param6=data.w) 

self.server.config is updated whenever configuration is changed, and the current position limits are self.server.config.pos_limits.

Note that it may take a fraction of a second for the low-level controller to report the new configuration. Until this occurs (and the CSC parses it) the old limits will be checked in the CSC. So if you were trying sending the setLimits command immediately followed by the move or offset command, then the problem may be a race condition. If so, I suspect it is not worth the complexity to try to fix it (at least not without replacing the low-level controller). It would require a lot of complex code to fix this, setting our own limits is rare, and the low level controller will protect itself.

Offsets use a very similar bit of code, but the offset is first supplemented by self.server.telemetry.commanded_pos to obtain an absolute position. So if you only see this problem with the offset command then it may be another problem. If you only see the problem with offset then please run the following test: command some moves and see if the demand field of the application event accurately reports the new commanded positions.

If none of the above then I would appreciate more information, including some logs showing the commanded position and reported configuration and application event output.

Show
Russell Owen added a comment - Eric Coughlin I filed this ticket when you reported to me that the Hexapod was not enforcing position limits after restricting the limits via the configureLimits . I am now looking at the code and I'm not sure how this can be happening. I would like more input from you. Here is the relevant bit of code for move in hexapod_csc.py: def _make_position_set_command(self, data): """Make a POSITION_SET command for the low-level controller using data from the move or moveLUT CSC command. """ utils.check_symmetrical_range(data.x, "x", self.server.config.pos_limits[0], ExceptionClass=salobj.ExpectedError) utils.check_symmetrical_range(data.y, "y", self.server.config.pos_limits[0], ExceptionClass=salobj.ExpectedError) utils.check_range(data.z, "z", self.server.config.pos_limits[1], self.server.config.pos_limits[2], ExceptionClass=salobj.ExpectedError) utils.check_symmetrical_range(data.u, "u", self.server.config.pos_limits[3], ExceptionClass=salobj.ExpectedError) utils.check_symmetrical_range(data.v, "v", self.server.config.pos_limits[3], ExceptionClass=salobj.ExpectedError) utils.check_range(data.w, "w", self.server.config.pos_limits[4], self.server.config.pos_limits[5], ExceptionClass=salobj.ExpectedError) return self.make_command(code=enums.CommandCode.POSITION_SET, param1=data.x, param2=data.y, param3=data.z, param4=data.u, param5=data.v, param6=data.w) self.server.config is updated whenever configuration is changed, and the current position limits are self.server.config.pos_limits . Note that it may take a fraction of a second for the low-level controller to report the new configuration. Until this occurs (and the CSC parses it) the old limits will be checked in the CSC. So if you were trying sending the setLimits command immediately followed by the move or offset command, then the problem may be a race condition. If so, I suspect it is not worth the complexity to try to fix it (at least not without replacing the low-level controller). It would require a lot of complex code to fix this, setting our own limits is rare, and the low level controller will protect itself. Offsets use a very similar bit of code, but the offset is first supplemented by self.server.telemetry.commanded_pos to obtain an absolute position. So if you only see this problem with the offset command then it may be another problem. If you only see the problem with offset then please run the following test: command some moves and see if the demand field of the application event accurately reports the new commanded positions. If none of the above then I would appreciate more information, including some logs showing the commanded position and reported configuration and application event output.
Hide
Russell Owen added a comment -

I used this ticket to add a test that the move command does check positions against the current position limits.

I also took the opportunity to simplify the code that checks limits in the CSC. But none of the changes affect what the CSC does. So whatever problem Eric Coughlin was seeing has not been fixed.

Pull request: https://github.com/lsst-ts/ts_hexapod/pull/10

Show
Russell Owen added a comment - I used this ticket to add a test that the move command does check positions against the current position limits. I also took the opportunity to simplify the code that checks limits in the CSC. But none of the changes affect what the CSC does. So whatever problem Eric Coughlin was seeing has not been fixed. Pull request: https://github.com/lsst-ts/ts_hexapod/pull/10
Hide
Russell Owen added a comment -

Please look this over when you have a chance. If there is a problem I have missed then please give me more details and reject the pull request, so I can fix it.

Show
Russell Owen added a comment - Please look this over when you have a chance. If there is a problem I have missed then please give me more details and reject the pull request, so I can fix it.
Hide
Russell Owen added a comment -

I'll file a new ticket and add details from the test report and suggestions for future tests. Meanwhile, let's use this ticket to beef up the unit tests.

Show
Russell Owen added a comment - I'll file a new ticket and add details from the test report and suggestions for future tests. Meanwhile, let's use this ticket to beef up the unit tests.
Hide
Eric Coughlin added a comment -

Reviewed in github.

Show
Eric Coughlin added a comment - Reviewed in github.
Hide
Russell Owen added a comment - - edited

Merged to develop and master and released as v0.3.1.

I realize there is still an issue with the hexapod (DM-23150) but I figured the improvements justified a minor release.

Show
Russell Owen added a comment - - edited Merged to develop and master and released as v0.3.1. I realize there is still an issue with the hexapod ( DM-23150 ) but I figured the improvements justified a minor release.

#### People

Assignee:
Russell Owen
Reporter:
Russell Owen
Reviewers:
Eric Coughlin
Watchers:
Eric Coughlin, Russell Owen