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

MTHexapod moves can be delayed or lost if compensation mode is enabled

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Sprint:
      TSSW Sprint - Mar 29 - Apr 12, TSSW Sprint - Apr 12 - Apr 26
    • Team:
      Telescope and Site
    • Urgent?:
      No

      Description

      The low-level controller will ignore move commands if the hexapod is already moving.

      I think the effect of this is that while compensation mode is turned on a move can be delayed – it the move arrives while a compensation move is being applied. But it may be worse than that – the move may be lost. In any, modify the CSC code to make this more predictable. It won't be easy because there is so little feedback from the low-level controller.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            The low-level controller ignores attempts to move while moving.
            So...how should the CSC handle overlapping moves and offsets (which is something I hope will be rare, other than the main driver for this ticket: properly supporting moves and offsets while a compensation offset is being applied).

            Here are the options I've thought of:
            1) Wait for the old move to finish, then issue the new one.
            2) Stop the old move and start a new one.
            3) Reject the new move.

            The primary use cases I have thought of are:

            A new move or offset comes in while an existing compensation update is occurring.

            This is the main driver for this ticket. In my opinion:

            1 waiting for the compensation offset to finish is probably the best choice. Compensation offsets will almost always be small and quick.
            2 stopping the actuators would be a bit harder on the actuators and often a bit less efficient.
            3 rejecting the command is clearly unacceptable.

            A new move or offset comes in while an existing move or offset command is occurring.

            This should be unusual, and in my opinion does not justify a lot of complex code to deal with, but I'd still like to aim for the best solution.

            A subtlety of an offset arriving during another move is that the new offset must be relative to the target position. (Making it relative to the current position means the behavior would be too dependent on the exact time at which the command arrived).

            1 waiting works and is easy to understand, but is inefficient if the old and new positions are far apart.
            2 stopping and restarting is possibly a bit harder on the actuators, but will sometimes be faster than 1
            3 rejecting the command is probably acceptable. It should not be a problem for MTAOS, because that should waits for each move or offset to complete before issuing another.

            What to do?

            I'm still thinking about the problem and would appreciate feedback on these options.

            Show
            rowen Russell Owen added a comment - - edited The low-level controller ignores attempts to move while moving. So...how should the CSC handle overlapping moves and offsets (which is something I hope will be rare, other than the main driver for this ticket: properly supporting moves and offsets while a compensation offset is being applied). Here are the options I've thought of: 1) Wait for the old move to finish, then issue the new one. 2) Stop the old move and start a new one. 3) Reject the new move. The primary use cases I have thought of are: A new move or offset comes in while an existing compensation update is occurring. This is the main driver for this ticket. In my opinion: 1 waiting for the compensation offset to finish is probably the best choice. Compensation offsets will almost always be small and quick. 2 stopping the actuators would be a bit harder on the actuators and often a bit less efficient. 3 rejecting the command is clearly unacceptable. A new move or offset comes in while an existing move or offset command is occurring. This should be unusual, and in my opinion does not justify a lot of complex code to deal with, but I'd still like to aim for the best solution. A subtlety of an offset arriving during another move is that the new offset must be relative to the target position. (Making it relative to the current position means the behavior would be too dependent on the exact time at which the command arrived). 1 waiting works and is easy to understand, but is inefficient if the old and new positions are far apart. 2 stopping and restarting is possibly a bit harder on the actuators, but will sometimes be faster than 1 3 rejecting the command is probably acceptable. It should not be a problem for MTAOS, because that should waits for each move or offset to complete before issuing another. What to do? I'm still thinking about the problem and would appreciate feedback on these options.
            Hide
            rowen Russell Owen added a comment - - edited

            I talked to Bo Xin [X] today and we agreed on the following: If one move or offset command interrupts another, have the second stop the motion and start it again (with offset applied to the target position). Acknowledge the first command as superseded.

            We didn't fully decide on interrupting a compensation move. But I am going to start with the same plan – stop the current motion and start a new one. It is much simpler to implement a single technique for all kinds of motion, and it means that if we have a long compensation move that is slow to implement (unlikely as that is) that the requested move will still finish quickly, with no surprising delays.

            Show
            rowen Russell Owen added a comment - - edited I talked to Bo Xin [X] today and we agreed on the following: If one move or offset command interrupts another, have the second stop the motion and start it again (with offset applied to the target position). Acknowledge the first command as superseded. We didn't fully decide on interrupting a compensation move. But I am going to start with the same plan – stop the current motion and start a new one. It is much simpler to implement a single technique for all kinds of motion, and it means that if we have a long compensation move that is slow to implement (unlikely as that is) that the requested move will still finish quickly, with no surprising delays.
            Hide
            bxin Bo Xin [X] (Inactive) added a comment -

            yes this all makes good sense, I think.

            Even when it is in the middle of a compensation move, we should stop the motion, then reset the target position. The new target position has two parts: uncompensated and compensated. The target position for the old compensation move is no longer relevant.

            Show
            bxin Bo Xin [X] (Inactive) added a comment - yes this all makes good sense, I think. Even when it is in the middle of a compensation move, we should stop the motion, then reset the target position. The new target position has two parts: uncompensated and compensated. The target position for the old compensation move is no longer relevant.
            Hide
            rowen Russell Owen added a comment - - edited

            The changes include:

            • Add a write lock to ts_hexrotcomm BaseCsc. This makes it easy to cancel code that sends commands to the low-level controller without interrupting a send (and so sending partial data, then leaving the rest in the output buffer – which would then corrupt the next command that was sent). Simply acquire the write lock before cancelling the task.
            • Modify ts_mthexapod move and offset commands to start by cancelling the compensation loop, cancelling any executing move or offset command, and stopping any existing motion. Only then do they command the new motion.
            • The compensation loop now waits for motion to stop before issuing a compensation move (it does not stop existing motion).

            Pull requests:

            Show
            rowen Russell Owen added a comment - - edited The changes include: Add a write lock to ts_hexrotcomm BaseCsc. This makes it easy to cancel code that sends commands to the low-level controller without interrupting a send (and so sending partial data, then leaving the rest in the output buffer – which would then corrupt the next command that was sent). Simply acquire the write lock before cancelling the task. Modify ts_mthexapod move and offset commands to start by cancelling the compensation loop, cancelling any executing move or offset command, and stopping any existing motion. Only then do they command the new motion. The compensation loop now waits for motion to stop before issuing a compensation move (it does not stop existing motion). Pull requests: https://github.com/lsst-ts/ts_hexrotcomm/pull/36 https://github.com/lsst-ts/ts_mthexapod/pull/43
            Hide
            ttsai Te-Wei Tsai added a comment -

            Reviewed the hexapod code PR and send the review to Russell already. All the update looks good to me! Thanks!

            Show
            ttsai Te-Wei Tsai added a comment - Reviewed the hexapod code PR and send the review to Russell already. All the update looks good to me! Thanks!
            Hide
            rowen Russell Owen added a comment -

            Te-Wei's review:

            ts_mthexapod python/lsst/ts/mthexapod/hexapod_csc.py

            1. The CompensationInfo class has three attributes actually, but the doc string only has one (although the other two are the parameters):
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R66-R76

            2. Do you want to check the compensation_inputs is None or not based on your doc string?
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R76

            3. Will you be in the while loop forever in compensation_wait()? I prefer to have some timeout unless you run this function in another thread:
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R316

            The similar thing here:
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R763
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-e4738762d295f3c94e7b14c1261fcb1aaac1e753fbdaee4dfb0cad4c8d244730R976

            4. Do you have the doc string of raise error?

            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R820-R827

            Show
            rowen Russell Owen added a comment - Te-Wei's review: ts_mthexapod python/lsst/ts/mthexapod/hexapod_csc.py 1. The CompensationInfo class has three attributes actually, but the doc string only has one (although the other two are the parameters): https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R66-R76 2. Do you want to check the compensation_inputs is None or not based on your doc string? https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R76 3. Will you be in the while loop forever in compensation_wait()? I prefer to have some timeout unless you run this function in another thread: https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R316 The similar thing here: https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R763 https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-e4738762d295f3c94e7b14c1261fcb1aaac1e753fbdaee4dfb0cad4c8d244730R976 4. Do you have the doc string of raise error? https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R820-R827
            Hide
            rowen Russell Owen added a comment - - edited

            Thank you for the helpful review. Here is my response:

            ts_mthexapod python/lsst/ts/mthexapod/hexapod_csc.py

            1. The CompensationInfo class has three attributes actually, but the doc string only has one (although the other two are the parameters):
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R66-R76

            Good point. To avoid duplicating information I added a note explaining that all parameters were also attributes.

            2. Do you want to check the compensation_inputs is None or not based on your doc string?
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R76

            There is no need to check; I just store the parameter as provided.
            The only reason I check the compensation_offset parameter is to compute compenstated_pos.

            3. Will you be in the while loop forever in compensation_wait()? I prefer to have some timeout unless you run this function in another thread:
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R316
            The similar thing here:
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R763
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-e4738762d295f3c94e7b14c1261fcb1aaac1e753fbdaee4dfb0cad4c8d244730R976

            Good point. I found two reasonable places to add timeouts:

            • _move when it tries to halt motion. If this times out then the move or offset command will fail with CMD_TIMEOUT = -304.
            • wait_compensation when it waits for motion to stop. If this happens the compensation loop will fail with an error log message. I don't have accurate information about how long a move may take, so for this timeout I use the longest time any move can take. I want to avoid any false alarms here.

            Regarding the detail of using "while True" loops: I think it is the logical thing to do in this code because these loops are expected to run until they are cancelled (the normal way to stop them) or fail. I have implemented the time limits where I felt it was simplest to add them.

            4. Do you have the doc string of raise error?
            https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R820-R827

            Good point. The code in question is an attempt to restart the compensation loop, in case a move or offset command fails after it has stopped the compensation loop. I added a code comment explaining that.

            I do not have Raises section in the doc string because I don't have a good sense of what errors might occur. The obvious things are checked before cancelling the compensation loop and calling _move.

            Show
            rowen Russell Owen added a comment - - edited Thank you for the helpful review. Here is my response: ts_mthexapod python/lsst/ts/mthexapod/hexapod_csc.py 1. The CompensationInfo class has three attributes actually, but the doc string only has one (although the other two are the parameters): https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R66-R76 Good point. To avoid duplicating information I added a note explaining that all parameters were also attributes. 2. Do you want to check the compensation_inputs is None or not based on your doc string? https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R76 There is no need to check; I just store the parameter as provided. The only reason I check the compensation_offset parameter is to compute compenstated_pos . 3. Will you be in the while loop forever in compensation_wait()? I prefer to have some timeout unless you run this function in another thread: https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R316 The similar thing here: https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R763 https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-e4738762d295f3c94e7b14c1261fcb1aaac1e753fbdaee4dfb0cad4c8d244730R976 Good point. I found two reasonable places to add timeouts: _move when it tries to halt motion. If this times out then the move or offset command will fail with CMD_TIMEOUT = -304. wait_compensation when it waits for motion to stop. If this happens the compensation loop will fail with an error log message. I don't have accurate information about how long a move may take, so for this timeout I use the longest time any move can take. I want to avoid any false alarms here. Regarding the detail of using "while True" loops: I think it is the logical thing to do in this code because these loops are expected to run until they are cancelled (the normal way to stop them) or fail. I have implemented the time limits where I felt it was simplest to add them. 4. Do you have the doc string of raise error? https://github.com/lsst-ts/ts_mthexapod/pull/43/files#diff-aa27843b4f5f8b8e93da82ca8dff98645d300df64b708e8e76f184df93b49ca5R820-R827 Good point. The code in question is an attempt to restart the compensation loop, in case a move or offset command fails after it has stopped the compensation loop. I added a code comment explaining that. I do not have Raises section in the doc string because I don't have a good sense of what errors might occur. The obvious things are checked before cancelling the compensation loop and calling _move .
            Hide
            rowen Russell Owen added a comment -

            Released:

            • ts_hexrotcomm v0.18.0
            • ts_mthexapod v0.16.0
            Show
            rowen Russell Owen added a comment - Released: ts_hexrotcomm v0.18.0 ts_mthexapod v0.16.0

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Te-Wei Tsai
              Watchers:
              Bo Xin [X] (Inactive), Holger Drass, Russell Owen, Te-Wei Tsai
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.