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

Acknowledge the SAL Command from Rotator PXI

    XMLWordPrintable

    Details

    • Story Points:
      4
    • Sprint:
      TSSW Sprint - Nov 08 - Nov 22, TSSW Sprint - Nov 22 - Dec 06
    • Team:
      Telescope and Site
    • Urgent?:
      No

      Description

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

      Note. It looks like I may need to use sync pattern to identify the command comes from the DDS or GUI. Only the CSC needs the command acknowledgement. In addition, in the future, the GUI specific code will be deprecated. There should be no difference between the GUI and CSC code at that time. In this case, I may need to update the GUI code to use another sync pattern value instead.

      Note. I do need to change the GUI for sync pattern.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            The current command c struct does have a "counter" field (the 2nd field) that is a unique identifier for each command. This is ideal for identifying the command when reporting command success or failure.

            The minimum needed information is "command rejected" or "command done".

            Other items that would be very helpful:

            • A "command started" acknowledgement for commands that will take a long time, such as moving a hexapod strut.
            • An explanation of why a command was rejected, such as "the CSC cannot send commands" or "command not allowed in the current state" or "parameter out of range". It's a fairly small number so an enum would suffice, though a string allows you to be more informative, e.g. "90 out of range [0, 50]" instead of out_of_range.
            • An estimate of duration in the "command started" acknowledgement.

            It is probably possible to add this information to the Telemetry struct, but please consider making a new struct (if that will not mess up the EUI) because the information really is not very "telemetry-like" and it only needs to be output when a command is accepted, rejected or done.

            I will be delighted to make any necessary changes to the CSC.

            Show
            rowen Russell Owen added a comment - The current command c struct does have a "counter" field (the 2nd field) that is a unique identifier for each command. This is ideal for identifying the command when reporting command success or failure. The minimum needed information is "command rejected" or "command done". Other items that would be very helpful: A "command started" acknowledgement for commands that will take a long time, such as moving a hexapod strut. An explanation of why a command was rejected, such as "the CSC cannot send commands" or "command not allowed in the current state" or "parameter out of range". It's a fairly small number so an enum would suffice, though a string allows you to be more informative, e.g. "90 out of range [0, 50] " instead of out_of_range. An estimate of duration in the "command started" acknowledgement. It is probably possible to add this information to the Telemetry struct, but please consider making a new struct (if that will not mess up the EUI) because the information really is not very "telemetry-like" and it only needs to be output when a command is accepted, rejected or done. I will be delighted to make any necessary changes to the CSC.
            Hide
            ttsai Te-Wei Tsai added a comment -

            The discussion of command status structure with Russell in slack:

            [Russell]
            I suggest considering what fields it needs. I think the ideal struct would have these fields:
            1. The command ID assigned by the user (int)
            2. The response code (int)
            3. An estimated duration (for a long-running command that has started, e.g. a point-to-point move) (double or float)
            4. A description of what went wrong, if a command is rejected or fails (presumably a fixed-length string).
            You could probably get away without that last item, but it would truly make the replies much more useful.

            As to estimated duration…I think a long time is better than nothing. Most commands are short enough that it doesn’t matter. But point-to-point moves do matter. If you can give a decent estimate it will be a big help — not only to the CSC but to users of the CSC.

            Show
            ttsai Te-Wei Tsai added a comment - The discussion of command status structure with Russell in slack: [Russell] I suggest considering what fields it needs. I think the ideal struct would have these fields: 1. The command ID assigned by the user (int) 2. The response code (int) 3. An estimated duration (for a long-running command that has started, e.g. a point-to-point move) (double or float) 4. A description of what went wrong, if a command is rejected or fails (presumably a fixed-length string). You could probably get away without that last item, but it would truly make the replies much more useful. As to estimated duration…I think a long time is better than nothing. Most commands are short enough that it doesn’t matter. But point-to-point moves do matter. If you can give a decent estimate it will be a big help — not only to the CSC but to users of the CSC.
            Hide
            ttsai Te-Wei Tsai added a comment -

            The summarization of discussion with Russell:

            Just to summarize what we talked about in the low-level controller:

            • There is no support for acknowledging completion of long-running commands (point-to-point moves are the only example I can think of) because of the black box nature of the Simulink model.
            • Each command will receive a final code of OK or NotOK. That will arrive very quickly after being sent.
            • NotOK will include a reason. These could be quite terse, but if it’s not too much extra work it’d be really nice to have details. For example: “position=123.1 out of range [-90.0, 90.0]“, “velocity=45 > max=10”, “enabled substate=moving point to point instead of stationary”, “state=fault instead of enabled”. If that’s too much work than short messages such as “position out of bounds”, “enabled_substate not stationary” would be OK.
            • If the low-level controller notices a missing command ID then it will report Missing. I consider this optional. If you implement it, please don’t be fooled when the ID wraps around.
            • On a related note: please consider making the user-assigned command ID some form of unsigned int.
            • For slow commands, such as point-to-point moves, the OK will include an estimate of how long the move will take. A double or float in seconds.
            • The CSC will continue to receive enough information in the telemetry to determine if a point-to-point move succeeded (including an “in position” flag).

            Please also consider reporting the ID of the most recent point to point move in the telemetry. That might help the CSC figure out a bit better if a command is done. In fact it actually allows the low-level controller to report that, but that complicates command reporting since we now need separate “started” and “finished” states for long-running commands.

            Show
            ttsai Te-Wei Tsai added a comment - The summarization of discussion with Russell: Just to summarize what we talked about in the low-level controller: There is no support for acknowledging completion of long-running commands (point-to-point moves are the only example I can think of) because of the black box nature of the Simulink model. Each command will receive a final code of OK or NotOK. That will arrive very quickly after being sent. NotOK will include a reason. These could be quite terse, but if it’s not too much extra work it’d be really nice to have details. For example: “position=123.1 out of range [-90.0, 90.0] “, “velocity=45 > max=10”, “enabled substate=moving point to point instead of stationary”, “state=fault instead of enabled”. If that’s too much work than short messages such as “position out of bounds”, “enabled_substate not stationary” would be OK. If the low-level controller notices a missing command ID then it will report Missing. I consider this optional. If you implement it, please don’t be fooled when the ID wraps around. On a related note: please consider making the user-assigned command ID some form of unsigned int. For slow commands, such as point-to-point moves, the OK will include an estimate of how long the move will take. A double or float in seconds. The CSC will continue to receive enough information in the telemetry to determine if a point-to-point move succeeded (including an “in position” flag). Please also consider reporting the ID of the most recent point to point move in the telemetry. That might help the CSC figure out a bit better if a command is done. In fact it actually allows the low-level controller to report that, but that complicates command reporting since we now need separate “started” and “finished” states for long-running commands.
            Hide
            ttsai Te-Wei Tsai added a comment -

            Please help to review the PRs:
            1. https://github.com/lsst-ts/ts_rotator_controller/pull/29
            2. https://github.com/lsst-ts/ts_rotator_gui/pull/16

            Please ignore the fail of Jenkins for GUI. That is the problem of Jenkins itself. I had emailed Diego in CTIO to fix it.

            Thanks!

            Show
            ttsai Te-Wei Tsai added a comment - Please help to review the PRs: 1. https://github.com/lsst-ts/ts_rotator_controller/pull/29 2. https://github.com/lsst-ts/ts_rotator_gui/pull/16 Please ignore the fail of Jenkins for GUI. That is the problem of Jenkins itself. I had emailed Diego in CTIO to fix it. Thanks!
            Hide
            ttsai Te-Wei Tsai added a comment -

            Russell reviewed and approved the PRs in GitHub.

            Show
            ttsai Te-Wei Tsai added a comment - Russell reviewed and approved the PRs in GitHub.

              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.