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

Please refine the MT hexapod low-level commands

    XMLWordPrintable

    Details

      Description

      The current low-level MT hexapod controller has a few issues that I would love to have cleaned up, if you can find the time:

      • Commanding a motion requires two separate commands: SET_POSITION followed by MOVE (and something similar for offsets). This introduces a potential race condition if two users are trying to command the hexapod. It is also a nuisance. I believe it was originally chosen because of the MOVE_LUT command, but we don't use that anymore.
      • Remove the unused MOVE_LUT command and any associated code that is easy to pull out. That includes removing the LUT tables and no longer reporting the LUT values in the configuration (making the configuration data much shorter).

      Please also also consider removing the offset command. The CSC isn't using it (it supports offsets, but implements them by commanding absolute moves).

      The hexapod EUI will be affected by this ticket and need to do the related modification.

        Attachments

          Issue Links

            Activity

            No builds found.
            rowen Russell Owen created issue -
            rowen Russell Owen made changes -
            Field Original Value New Value
            Link This issue relates to DM-29578 [ DM-29578 ]
            ttsai Te-Wei Tsai made changes -
            Epic Link DM-27621 [ 441885 ]
            ttsai Te-Wei Tsai made changes -
            Component/s ts_main_telescope [ 16710 ]
            ttsai Te-Wei Tsai made changes -
            Labels HexRot M2Hexapod hexapod
            ttsai Te-Wei Tsai made changes -
            End date 15/Apr/21
            Sprint TSSW Sprint - Apr 12 - Apr 26 [ 1089 ]
            Start date 14/Apr/21
            Story Points 1
            ttsai Te-Wei Tsai made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            ttsai Te-Wei Tsai added a comment - - edited

            Removed the hexapod LUT in Simulink model.
            Removed the moveLUT in commanding.c.
            Removed the LUT related items in ddsConfigTelemetryStreamStructure_t. This triggers the DM-29664.
            Remove the "LUT invalid" in the application status of EUI.

            Show
            ttsai Te-Wei Tsai added a comment - - edited Removed the hexapod LUT in Simulink model. Removed the moveLUT  in commanding.c . Removed the LUT related items in ddsConfigTelemetryStreamStructure_t . This triggers the DM-29664 . Remove the "LUT invalid" in the application status of EUI.
            Hide
            rowen Russell Owen added a comment - - edited

            Wonderful. As far as the MOVE command goes: making it a single command may require a bit of subtlety because of the "sync" flag. Commands only support 6 parameters (other than the command code) and we need all six for x, y, z, u, v, w. Two ways out of this are:

            • Have separate commands for "synchronized move" vs "non-synchronized move". This doesn't scale well if there are many move commands, but might be acceptable for one (move only) or two (move and offset). It is probably the simpler solution. If it's OK with you, I suggest it.
            • Add a command parameter. I have no idea how invasive that would be. Does it matter if the command format is different for the hexapods than for the rotator?
            Show
            rowen Russell Owen added a comment - - edited Wonderful. As far as the MOVE command goes: making it a single command may require a bit of subtlety because of the "sync" flag. Commands only support 6 parameters (other than the command code) and we need all six for x, y, z, u, v, w. Two ways out of this are: Have separate commands for "synchronized move" vs "non-synchronized move". This doesn't scale well if there are many move commands, but might be acceptable for one (move only) or two (move and offset). It is probably the simpler solution. If it's OK with you, I suggest it. Add a command parameter. I have no idea how invasive that would be. Does it matter if the command format is different for the hexapods than for the rotator?
            ttsai Te-Wei Tsai made changes -
            Description The current low-level MT hexapod controller has a few issues that I would love to have cleaned up, if you can find the time:
            * Commanding a motion requires two separate commands: SET_POSITION followed by MOVE (and something similar for offsets). This introduces a potential race condition if two users are trying to command the hexapod. It is also a nuisance. I believe it was originally chosen because of the MOVE_LUT command, but we don't use that anymore.
            * Remove the unused MOVE_LUT command and any associated code that is easy to pull out. That includes removing the LUT tables and no longer reporting the LUT values in the configuration (making the configuration data *much* shorter).

            Please also also consider removing the offset command. The CSC isn't using it (it supports offsets, but implements them by commanding absolute moves).
            The current low-level MT hexapod controller has a few issues that I would love to have cleaned up, if you can find the time:
             * Commanding a motion requires two separate commands: SET_POSITION followed by MOVE (and something similar for offsets). This introduces a potential race condition if two users are trying to command the hexapod. It is also a nuisance. I believe it was originally chosen because of the MOVE_LUT command, but we don't use that anymore.
             * Remove the unused MOVE_LUT command and any associated code that is easy to pull out. That includes removing the LUT tables and no longer reporting the LUT values in the configuration (making the configuration data *much* shorter).

            Please also also consider removing the offset command. The CSC isn't using it (it supports offsets, but implements them by commanding absolute moves).

            The hexapod EUI will be affected by this ticket and need to do the related modification.
            ttsai Te-Wei Tsai made changes -
            Story Points 1 2
            rowen Russell Owen made changes -
            Link This issue is triggering DM-29664 [ DM-29664 ]
            Hide
            ttsai Te-Wei Tsai added a comment -

            Refactor the EUI for the command.

            Show
            ttsai Te-Wei Tsai added a comment - Refactor the EUI for the command.
            Hide
            ttsai Te-Wei Tsai added a comment -

            Based on the EUI code, I realized it might not be a good idea to combine the POSITION_SET and MOVE. The first thing is that this is inconsistent with the design of Simulink model. In addition, the EUI needs to support the moves of hexapod position (x, y, z, rx, ry, rz) and raw position (distance for each actuator). The above two movements have the synchronic and asynchronic movements. In EUI's viewpoint, it is easier to separate these two commands.

            Show
            ttsai Te-Wei Tsai added a comment - Based on the EUI code, I realized it might not be a good idea to combine the POSITION_SET  and MOVE . The first thing is that this is inconsistent with the design of Simulink model. In addition, the EUI needs to support the moves of hexapod position (x, y, z, rx, ry, rz) and raw position (distance for each actuator). The above two movements have the synchronic and asynchronic movements. In EUI's viewpoint, it is easier to separate these two commands.
            Hide
            ttsai Te-Wei Tsai added a comment - - edited

            For the possible race condition (although there should be no two users to command the hexapod in the same time), this will be fixed after I rewrite the low-level controller to support the queue of command (DM-29720).

            Show
            ttsai Te-Wei Tsai added a comment - - edited For the possible race condition (although there should be no two users to command the hexapod in the same time), this will be fixed after I rewrite the low-level controller to support the queue of command ( DM-29720 ).
            ttsai Te-Wei Tsai made changes -
            Link This issue is triggering DM-29720 [ DM-29720 ]
            Show
            ttsai Te-Wei Tsai added a comment - Please help to review the PRs: 1. https://github.com/lsst-ts/ts_hexapod_controller/pull/9 2. https://github.com/lsst-ts/ts_hexapod_gui/pull/5 3. https://github.com/lsst-ts/ts_mt_hexRot_simulink/pull/14 Thanks!
            ttsai Te-Wei Tsai made changes -
            Reviewers Russell Owen [ rowen ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            rowen Russell Owen added a comment -

            Looks great! Thank you for doing this.

            Show
            rowen Russell Owen added a comment - Looks great! Thank you for doing this.
            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 relates to DM-29769 [ DM-29769 ]

              People

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

                Dates

                Created:
                Updated:
                Resolved:
                Start date:
                End date:

                  Jenkins

                  No builds found.