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.

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.

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.

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

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

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

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.

Released:

• ts_hexrotcomm v0.18.0
• ts_mthexapod v0.16.0