Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-23092

Check position limits for Hexapod move and offset commands

    XMLWordPrintable

    Details

    • 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.

        Attachments

          Issue Links

            Activity

            Hide
            rowen 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
            rowen 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
            rowen 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
            rowen 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
            rowen 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
            rowen 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
            rowen 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
            rowen 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
            ecoughlin Eric Coughlin added a comment -

            Reviewed in github.

            Show
            ecoughlin Eric Coughlin added a comment - Reviewed in github.
            Hide
            rowen 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
            rowen 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:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Eric Coughlin
              Watchers:
              Eric Coughlin, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.