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

Acknowledge the SAL Command from Hexapod PXI

    XMLWordPrintable

    Details

    • Story Points:
      3
    • Sprint:
      TSSW Sprint - Nov 22 - Dec 06, TSSW Sprint - Dec 06 - Dec 20, TSSW Sprint - Dec 20 - Jan 03
    • Team:
      Telescope and Site
    • Urgent?:
      No

      Description

      Acknowledge the SAL command from Hexapod PXI. This will let the CSC know the status of SAL command.

      Note. This update will need the CSC to use the SyncPattern = 0x5555 instead of 0xB4B4 or 0x6666 to identify the command comes from the DDS instead of GUI. Only the DDS command will get the command acknowledgement.

        Attachments

          Issue Links

            Activity

            No builds found.
            ttsai Te-Wei Tsai created issue -
            ttsai Te-Wei Tsai made changes -
            Field Original Value New Value
            Link This issue relates to DM-23849 [ DM-23849 ]
            ttsai Te-Wei Tsai made changes -
            Description Acknowledge the SAL command from Rotator PXI. This will let the CSC know the status of SAL command. Acknowledge the SAL command from Hexapod PXI. This will let the CSC know the status of SAL command.
            Hide
            rowen Russell Owen added a comment - - edited

            This would be wonderful. It would make the CSC more reliable, since it would not have to guess.

            In my experience there are two basic ways to handle this:

            1) Asynchronous commands:

            Slow commands, such as point to point moves, are not done until the move finishes, fails, or is aborted. In this model you need to support multiple commands running at the same time (e.g. so a user can stop a point-to-point move) and commands need a client-assigned identifier, such as an integer sequence number (private_seqNum in SAL) that is echoed in command status replies. I suggest the following command status replies:

            • started: the command has been accepted and started, but will take awhile to finish. Please include a predicted duration, preferably as a float/double in seconds.
            • rejected: the command was unacceptable for some reason. Please include a string explaining the reason.
            • succeeded: the command finished successfully.
            • failed: the command failed after being acknowledged as started. Please include a string explaining why it failed.
            • superseded: the command was superseded by another command. An example is starting a point-to-point move and then aborting it with a stop command.

            This matches SAL pretty closely, but has no ACK, since I don't think we need it here. Any command should quickly receive one of these status replies.

            2) Synchronous commands:

            Make all commands run quickly, and only allow one command to run at a time.

            In that model, slow commands such as point-to-point moves are reported as done when the command starts, and the client has to use other information to figure out when such commands finish. If you do that then I think you do not need the client to specify a command sequence number and you only need "succeeded", "failed" replies.

            Show
            rowen Russell Owen added a comment - - edited This would be wonderful. It would make the CSC more reliable, since it would not have to guess. In my experience there are two basic ways to handle this: 1) Asynchronous commands: Slow commands, such as point to point moves, are not done until the move finishes, fails, or is aborted. In this model you need to support multiple commands running at the same time (e.g. so a user can stop a point-to-point move) and commands need a client-assigned identifier, such as an integer sequence number (private_seqNum in SAL) that is echoed in command status replies. I suggest the following command status replies: started: the command has been accepted and started, but will take awhile to finish. Please include a predicted duration, preferably as a float/double in seconds. rejected: the command was unacceptable for some reason. Please include a string explaining the reason. succeeded: the command finished successfully. failed: the command failed after being acknowledged as started. Please include a string explaining why it failed. superseded: the command was superseded by another command. An example is starting a point-to-point move and then aborting it with a stop command. This matches SAL pretty closely, but has no ACK, since I don't think we need it here. Any command should quickly receive one of these status replies. 2) Synchronous commands: Make all commands run quickly, and only allow one command to run at a time. In that model, slow commands such as point-to-point moves are reported as done when the command starts , and the client has to use other information to figure out when such commands finish. If you do that then I think you do not need the client to specify a command sequence number and you only need "succeeded", "failed" replies.
            aclements Andy Clements made changes -
            Epic Link DM-27621 [ 441885 ]
            ttsai Te-Wei Tsai made changes -
            Epic Link DM-32113 [ 770361 ]
            ttsai Te-Wei Tsai made changes -
            Epic Link DM-32113 [ 770361 ] DM-27623 [ 441888 ]
            ttsai Te-Wei Tsai made changes -
            Link This issue is triggering DM-32653 [ DM-32653 ]
            Hide
            ttsai Te-Wei Tsai added a comment -

            Russell suggested the following in slack:

            Since you are already modifying the command struct, please consider also adding a 7th float parameter field or a bool field. We won’t use it right now (and never for the rotator) but the hexapod could use it to support a move command that includes all 6 positions plus the synch flag. Maybe. Someday.

            Show
            ttsai Te-Wei Tsai added a comment - Russell suggested the following in slack: Since you are already modifying the command struct, please consider also adding a 7th float parameter field or a bool field. We won’t use it right now (and never for the rotator) but the hexapod could use it to support a move command that includes all 6 positions plus the synch flag. Maybe. Someday.
            ttsai Te-Wei Tsai made changes -
            Sprint TSSW Sprint - Nov 22 - Dec 06 [ 1132 ]
            rowen Russell Owen made changes -
            Link This issue is triggering DM-32693 [ DM-32693 ]
            ttsai Te-Wei Tsai made changes -
            Link This issue relates to DM-32789 [ DM-32789 ]
            wvreeven Wouter van Reeven made changes -
            Sprint TSSW Sprint - Nov 22 - Dec 06 [ 1132 ] TSSW Sprint - Nov 22 - Dec 06, TSSW Sprint - Dec 06 - Dec 20 [ 1132, 1134 ]
            aclements Andy Clements made changes -
            Sprint TSSW Sprint - Nov 22 - Dec 06, TSSW Sprint - Dec 06 - Dec 20 [ 1132, 1134 ] TSSW Sprint - Nov 22 - Dec 06, TSSW Sprint - Dec 06 - Dec 20, TSSW Sprint - Dec 20 - Jan 03 [ 1132, 1134, 1139 ]
            ttsai Te-Wei Tsai made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            ttsai Te-Wei Tsai made changes -
            Description Acknowledge the SAL command from Hexapod PXI. This will let the CSC know the status of SAL command. Acknowledge the SAL command from Hexapod PXI. This will let the CSC know the status of SAL command.

            Note. This update will need the CSC to use the SyncPattern = *0x5555* instead of *0xB4B4* or *0x6666* to identify the command comes from the DDS instead of GUI. Only the DDS command will get the command acknowledgement.
            ttsai Te-Wei Tsai made changes -
            Story Points 3
            Hide
            ttsai Te-Wei Tsai added a comment -

            Please help to review the following PR:
            https://github.com/lsst-ts/ts_hexapod_controller/pull/30

            It is a combination of two PRs in rotator:
            (a) https://github.com/lsst-ts/ts_rotator_controller/pull/29
            (b) https://github.com/lsst-ts/ts_rotator_controller/pull/30

            The main difference is the function: commanding_estDuration().

            Thanks!

            Show
            ttsai Te-Wei Tsai added a comment - Please help to review the following PR: https://github.com/lsst-ts/ts_hexapod_controller/pull/30 It is a combination of two PRs in rotator: (a) https://github.com/lsst-ts/ts_rotator_controller/pull/29 (b)  https://github.com/lsst-ts/ts_rotator_controller/pull/30 The main difference is the function: commanding_estDuration() . Thanks!
            ttsai Te-Wei Tsai made changes -
            Reviewers Russell Owen [ rowen ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            ttsai Te-Wei Tsai made changes -
            Link This issue is triggering DM-33062 [ DM-33062 ]
            Hide
            rowen Russell Owen added a comment -

            Reviewed on github. A very nice improvement.

            Show
            rowen Russell Owen added a comment - Reviewed on github. A very nice improvement.
            rowen Russell Owen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            ttsai Te-Wei Tsai made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            ttsai Te-Wei Tsai made changes -
            Link This issue is triggering DM-33067 [ DM-33067 ]

              People

              Assignee:
              ttsai Te-Wei Tsai
              Reporter:
              ttsai Te-Wei Tsai
              Reviewers:
              Russell Owen
              Watchers:
              Russell Owen, Te-Wei Tsai
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.