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

Clean up XML for (MT) Rotator and Hexapod

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      3
    • Sprint:
      TSSW Sprint - Dec 9 - Dec 20, TSSW Sprint - Dec 23 - Jan 3, TSSW Sprint - Jan 06 - Jan 17
    • Team:
      Telescope and Site

      Description

      The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):

      • The Hexapod inPosition event has no inPosition field, so there is no way to indicate "not in position". Note that the Rotator inPosition event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named actuatorInPosition.
      • The Rotator tracking and trackLost events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
      • Some telemetry is not output. I will not output them yet, but keep them in mind for the future, should we decide we need them:
        • copley_fault_status_register; note that similar latching registers are output.
        • input_pin_states
      • Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
      • Most events and telemetry topics have a timestamp field. That is a waste of space because it duplicates private_sndStamp.
      • The telemetry topic names are capitalized and so do not meet our current standards.

      These are at least partly fixed in DM-21694:

      • There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
      • The deviceError event has a code field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an applicationStatus field to the new controllerState event that has the bit mask. In addition I output deviceError once for a given set of error conditions, with code set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the deviceError event as duplicated information, even though a string can be nice.

      This task will also clean up the Jira tickets:

      (1) DM-20969 and DM-20971 for the telemetry.

      (2) CAP-362 and CAP-369 for the event.

        Attachments

          Issue Links

            Activity

            No builds found.
            rowen Russell Owen created issue -
            swinbank John Swinbank made changes -
            Field Original Value New Value
            Team Telescope and Site [ 13500 ]
            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):

            - There is no detailedState event, even though that state is crucial for these CSCs. I note that here but plan to fix it in DM-21694 because I need it now.
            - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data.
            - The deviceError event has a "code" field that is a string, and as implemented by the vendor that "code" describes just one error, even if many error conditions are present. For the Python CSC I made "code" a comma separate string of all error conditions. But I think we need to output a bitmask that describes all error conditions, with an associated enumeration. (We could retain the text string as well, though it duplicates information).
            - Most events and telemetry topics have a timestamp field. That is a waste of space because it duplicates {{private_sndStamp}}.
            - The telemetry topic names do not meet our current standards.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):

            - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
              - copley_fault_status_register
              - input_pin_states
            - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
            - Most events and telemetry topics have a timestamp field. That is a waste of space because it duplicates {{private_sndStamp}}.
            - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
            - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
            - The deviceError event has a "code" field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an applicationStatus field to controllerState that has the bit mask and I output deviceError with one string that describes all error conditions at once. I hope we can get rid of deviceError as duplicated information, but a string can be nice.

            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):

            - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
              - copley_fault_status_register
              - input_pin_states
            - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
            - Most events and telemetry topics have a timestamp field. That is a waste of space because it duplicates {{private_sndStamp}}.
            - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
            - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
            - The deviceError event has a "code" field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an applicationStatus field to controllerState that has the bit mask and I output deviceError with one string that describes all error conditions at once. I hope we can get rid of deviceError as duplicated information, but a string can be nice.

            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):

            - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
              -- copley_fault_status_register
              -- input_pin_states
            - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
            - Most events and telemetry topics have a timestamp field. That is a waste of space because it duplicates {{private_sndStamp}}.
            - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
            - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
            - The deviceError event has a "code" field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an applicationStatus field to controllerState that has the bit mask and I output deviceError with one string that describes all error conditions at once. I hope we can get rid of deviceError as duplicated information, but a string can be nice.

            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):

            - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
              -- copley_fault_status_register
              -- input_pin_states
            - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
            - Most events and telemetry topics have a timestamp field. That is a waste of space because it duplicates {{private_sndStamp}}.
            - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
            - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
            - The deviceError event has a "code" field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an applicationStatus field to controllerState that has the bit mask and I output deviceError with one string that describes all error conditions at once. I hope we can get rid of deviceError as duplicated information, but a string can be nice.

            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of deviceError as duplicated information, even though a string can be nice.
            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of deviceError as duplicated information, even though a string can be nice.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.
            ttsai Te-Wei Tsai made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets: [DM-20969|https://jira.lsstcorp.org/browse/DM-20969] and [DM-20971|https://jira.lsstcorp.org/browse/DM-20971] for the telemetry.
            ttsai Te-Wei Tsai made changes -
            Watchers Russell Owen [ Russell Owen ] Russell Owen, Te-Wei Tsai [ Russell Owen, Te-Wei Tsai ]
            ttsai Te-Wei Tsai made changes -
            Watchers Russell Owen, Te-Wei Tsai [ Russell Owen, Te-Wei Tsai ] Rob Bovill, Russell Owen, Te-Wei Tsai [ Rob Bovill, Russell Owen, Te-Wei Tsai ]
            rowen Russell Owen made changes -
            Reviewers Te-Wei Tsai [ ttsai ]
            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets: [DM-20969|https://jira.lsstcorp.org/browse/DM-20969] and [DM-20971|https://jira.lsstcorp.org/browse/DM-20971] for the telemetry.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
            - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets: [DM-20969|https://jira.lsstcorp.org/browse/DM-20969] and [DM-20971|https://jira.lsstcorp.org/browse/DM-20971] for the telemetry.
            rowen Russell Owen made changes -
            Link This issue relates to DM-21949 [ DM-21949 ]
            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
            - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor that "code" describes just one error and the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets: [DM-20969|https://jira.lsstcorp.org/browse/DM-20969] and [DM-20971|https://jira.lsstcorp.org/browse/DM-20971] for the telemetry.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
            - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets: [DM-20969|https://jira.lsstcorp.org/browse/DM-20969] and [DM-20971|https://jira.lsstcorp.org/browse/DM-20971] for the telemetry.
            ttsai Te-Wei Tsai made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
            - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets: [DM-20969|https://jira.lsstcorp.org/browse/DM-20969] and [DM-20971|https://jira.lsstcorp.org/browse/DM-20971] for the telemetry.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets:

            (1) DM-20969 and DM-20971 for the telemetry.

            (2) CAP-362 for the event.
            ttsai Te-Wei Tsai made changes -
            Watchers Rob Bovill, Russell Owen, Te-Wei Tsai [ Rob Bovill, Russell Owen, Te-Wei Tsai ] Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai [ Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai ]
            ttsai Te-Wei Tsai made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets:

            (1) DM-20969 and DM-20971 for the telemetry.

            (2) CAP-362 for the event.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets:

            (1) DM-20969 and DM-20971 for the telemetry.

            (2) CAP-362 and CAP-369 for the event.
            Hide
            ttsai Te-Wei Tsai added a comment - - edited

            I propose this task should begin after the integration test in Dec. In that test, we should see the performance and behavior of new rotator CSC with the cooperation of CCW. The hexapod test might be postponed to Jan, 2020 depend on the test status in Dec. And the time is tight now for the test in Dec.

            In addition, the update of xml interface should have the consensus with Doug and Bo as the product owners. 

            Show
            ttsai Te-Wei Tsai added a comment - - edited I propose this task should begin after the integration test in Dec. In that test, we should see the performance and behavior of new rotator CSC with the cooperation of CCW. The hexapod test might be postponed to Jan, 2020 depend on the test status in Dec. And the time is tight now for the test in Dec. In addition, the update of xml interface should have the consensus with Doug and Bo as the product owners. 
            ttsai Te-Wei Tsai made changes -
            Link This issue relates to DM-21961 [ DM-21961 ]
            Hide
            rowen Russell Owen added a comment -

            That sounds very reasonable to me. I would like to avoid updating the vendor's middleware code (unless we are sure we want a fallback). The only safe way to do that is to be sure that our replacement CSC is acceptable.

            Show
            rowen Russell Owen added a comment - That sounds very reasonable to me. I would like to avoid updating the vendor's middleware code (unless we are sure we want a fallback). The only safe way to do that is to be sure that our replacement CSC is acceptable.
            Hide
            mareuter Michael Reuter added a comment -

            I see a merged PR on this ticket. Does that rectify all of the XML work? If so and it includes the settingsApplied work, can the CAP tickets be closed?

            Show
            mareuter Michael Reuter added a comment - I see a merged PR on this ticket. Does that rectify all of the XML work? If so and it includes the settingsApplied work, can the CAP tickets be closed?
            Hide
            rowen Russell Owen added a comment - - edited

            Michael Reuter The "merged PR" is for a different ticket: DM-21949. I have no idea why Jira is showing it with this ticket, though they are loosely related. I filed https://jira.lsstcorp.org/browse/IT-1587 since it appears to be a Jira bug.

            Show
            rowen Russell Owen added a comment - - edited Michael Reuter The "merged PR" is for a different ticket: DM-21949 . I have no idea why Jira is showing it with this ticket, though they are loosely related. I filed https://jira.lsstcorp.org/browse/IT-1587 since it appears to be a Jira bug.
            rowen Russell Owen made changes -
            Epic Link DM-21473 [ 424069 ]
            rowen Russell Owen made changes -
            Sprint TSSW Sprint - Dec 9 - Dec 20 [ 977 ]
            rowen Russell Owen made changes -
            Story Points 4
            rowen Russell Owen made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            rowen Russell Owen added a comment - - edited

            I had to make changes in ts_hexrotcomm because it uses the Rotator XML. I took the opportunity to make some improvements in hexrotcomm:

            • Rename the cmd field of Command to code and rename the cmd argument of BaseCsc.run_command to match.
            • Rename the cmd argument of CommandTelemetryServer.put_command to command.
            • Add BaseCsc.make_command and BaseCsc.run_multiple_commands. This is useful for safely sending multiple sequential commands to the low-level controller, as the Rotator move command now does.

            Description of XML changes:

            Pull requests:

            Show
            rowen Russell Owen added a comment - - edited I had to make changes in ts_hexrotcomm because it uses the Rotator XML. I took the opportunity to make some improvements in hexrotcomm: Rename the cmd field of Command to code and rename the cmd argument of BaseCsc.run_command to match. Rename the cmd argument of CommandTelemetryServer.put_command to command . Add BaseCsc.make_command and BaseCsc.run_multiple_commands . This is useful for safely sending multiple sequential commands to the low-level controller, as the Rotator move command now does. Description of XML changes: https://confluence.lsstcorp.org/display/LTS/Hexapod+XML+Changes https://confluence.lsstcorp.org/display/LTS/Rotator+XML+Changes Pull requests: https://github.com/lsst-ts/ts_xml/pull/201 https://github.com/lsst-ts/ts_hexrotcomm/pull/7 https://github.com/lsst-ts/ts_hexapod/pull/9 https://github.com/lsst-ts/ts_rotator/pull/8
            rowen Russell Owen made changes -
            Remote Link This issue links to "Page (Confluence)" [ 22739 ]
            rowen Russell Owen made changes -
            Description The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output, including (these may be intentional but I would rather output them unless there is a good reason not to):
             -- {{copley_fault_status_register}}
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets:

            (1) DM-20969 and DM-20971 for the telemetry.

            (2) CAP-362 and CAP-369 for the event.
            The XML for Rotator and Hexapod has some misfeatures I would like to fix if and when we are sure that we are writing our own CSCs. (Actually we could easily fix these even if we keep the vendor's code):
             - The Hexapod {{inPosition}} event has no {{inPosition}} field, so there is no way to indicate "not in position". Note that the Rotator {{inPosition}} event is fine. I suggest also adding an array of 6 booleans to indicate if each actuator is in position, perhaps named {{actuatorInPosition}}.
             - The Rotator {{tracking}} and {{trackLost}} events have no boolean value so cannot be output properly. They should be combined into a single event with a boolean flag.
             - Some telemetry is not output. I will not output them yet, but keep them in mind for the future, should we decide we need them:
             -- {{copley_fault_status_register}}; note that similar latching registers are output.
             -- {{input_pin_states}}
             - Several events are sent only when something (such as tracking, or a problem) starts, but not when it ends. We should send the event when the condition starts and when it ends, with suitable data (e.g. a boolean that is true if the condition is present).
             - Most events and telemetry topics have a {{timestamp}} field. That is a waste of space because it duplicates {{private_sndStamp}}.
             - The telemetry topic names are capitalized and so do not meet our current standards.

            These are at least partly fixed in DM-21694:
             - There is no detailed state event, even though that state is crucial for these CSCs. Fixed in DM-21694 by adding the controllerState event.
             - The deviceError event has a {{code}} field that is a string, and as implemented by the vendor, that "code" describes a single error so the event is output once for each error condition that is present. In DM-21694 I improved this by adding an {{applicationStatus}} field to the new {{controllerState}} event that has the bit mask. In addition I output {{deviceError}} once for a given set of error conditions, with {{code}} set to a comma-separated list of descriptions of each error so the event only when the error conditions change. I hope we can get rid of the {{deviceError}} event as duplicated information, even though a string can be nice.

            This task will also clean up the Jira tickets:

            (1) DM-20969 and DM-20971 for the telemetry.

            (2) CAP-362 and CAP-369 for the event.
            rowen Russell Owen made changes -
            Watchers Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai [ Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai ] Andrew Heyer, Bo Xin, Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai [ Andrew Heyer, Bo Xin, Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai ]
            rowen Russell Owen made changes -
            Remote Link This issue links to "Page (Confluence)" [ 22740 ]
            rowen Russell Owen made changes -
            Status In Progress [ 3 ] In Review [ 10004 ]
            rowen Russell Owen made changes -
            Story Points 4 3
            rowen Russell Owen made changes -
            Link This issue mitigates DM-20971 [ DM-20971 ]
            rowen Russell Owen made changes -
            Link This issue mitigates DM-20969 [ DM-20969 ]
            tribeiro Tiago Ribeiro made changes -
            Sprint TSSW Sprint - Dec 9 - Dec 20 [ 977 ] TSSW Sprint - Dec 9 - Dec 20, TSSW Sprint - Dec 23 - Jan 3 [ 977, 978 ]
            tribeiro Tiago Ribeiro made changes -
            Sprint TSSW Sprint - Dec 9 - Dec 20, TSSW Sprint - Dec 23 - Jan 3 [ 977, 978 ] TSSW Sprint - Dec 9 - Dec 20, TSSW Sprint - Dec 23 - Jan 3, TSSW Sprint - Jan 06 - Jan 17 [ 977, 978, 992 ]
            Hide
            ttsai Te-Wei Tsai added a comment -

            The update looks good and there is the consensus with the product owners already. Great job done!

            Show
            ttsai Te-Wei Tsai added a comment - The update looks good and there is the consensus with the product owners already. Great job done!
            ttsai Te-Wei Tsai made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            rowen Russell Owen made changes -
            Status Reviewed [ 10101 ] In Review [ 10004 ]
            Hide
            rowen Russell Owen added a comment - - edited

            Te-Wei's review for ts_xml:

            1. Not sure this will affect the idl or not:
            https://github.com/lsst-ts/ts_xml/blob/19707a9151cae96e52f5989db1948bad8ee8dde3/sal_interfaces/Hexapod/Hexapod_Commands.xml#L380

            2. I thought offsetLUT will be affected as well:
            https://github.com/lsst-ts/ts_xml/blob/19707a9151cae96e52f5989db1948bad8ee8dde3/sal_interfaces/Hexapod/Hexapod_Commands.xml#L441

            My answer:
            I fixed both issues and improved a documentation string at Tiago's request. (For the record: 1 only affected human readability, but was still worth fixing.)

            Show
            rowen Russell Owen added a comment - - edited Te-Wei's review for ts_xml: 1. Not sure this will affect the idl or not: https://github.com/lsst-ts/ts_xml/blob/19707a9151cae96e52f5989db1948bad8ee8dde3/sal_interfaces/Hexapod/Hexapod_Commands.xml#L380 2. I thought offsetLUT will be affected as well: https://github.com/lsst-ts/ts_xml/blob/19707a9151cae96e52f5989db1948bad8ee8dde3/sal_interfaces/Hexapod/Hexapod_Commands.xml#L441 My answer: I fixed both issues and improved a documentation string at Tiago's request. (For the record: 1 only affected human readability, but was still worth fixing.)
            Hide
            rowen Russell Owen added a comment -

            Te-Wei's review

            [base_csc.py]

            1. Not sure the meaning of "Must include ``SET_STATE``". This is an Enum (CommandCode), right?

            https://github.com/lsst-ts/ts_hexrotcomm/pull/7/files#diff-9884fc357e4ad38105bdf51d866033e9R65

            Do you mean the user need to use "CommandCode.SET_STATE"? Sorry, my intuition is to check this file (enums.py) for such kind of enum. Where is it?

            https://github.com/lsst-ts/ts_hexrotcomm/blob/c99e93d8c93e6cfbb18fb5020d0736dc552b7e8c/python/lsst/ts/hexrotcomm/enums.py

            I am not sure this is what you impied or not?

            https://github.com/lsst-ts/ts_hexrotcomm/blob/c99e93d8c93e6cfbb18fb5020d0736dc552b7e8c/python/lsst/ts/hexrotcomm/simple_mock_controller.py#L36

            When I began to review the PR of ts_rotator, I realized the CommandCode might be in the enum.py of ts_rotator. I thought the ts_hexrotcomm is the upstream of ts_rotator. If this is the case, is it a good idea to let the upstream code to have the knowledge of the code in downstream? Maybe the docstring can point out the CommandCode enum comes from the repo such as the ts_rotator to let other maintainer has the idea if my guess is correct. But maybe their relationship is not what I thought.

            2. Do we need to consider the command might fail?

            https://github.com/lsst-ts/ts_hexrotcomm/pull/7/files#diff-9884fc357e4ad38105bdf51d866033e9R252-R253

            [simple_CSC.py]

            1. I do not know the `salobj.Remote` can call this function in server:

            https://github.com/lsst-ts/ts_hexrotcomm/pull/7/files#diff-2dcda03af7b6b135f00ed9059cc0bc30R119

            How do we do this? `await m.cmd_move_sequentially.set_start()`? Is this correct?

            <ts_rotator>

            https://github.com/lsst-ts/ts_rotator/pull/8/files

            [rotator_csc.py]

            1. General question. I saw many `make_command()` and `run_command()` are used. This makes me thought you are using the Command Factory pattern. Is this correct? But I thought people will create a CommandFactory class and each command will be an object inherit from the default command class. Or maybe I am wrong in the guess of your intention?

            <ts_hexapod>

            https://github.com/lsst-ts/ts_hexapod/pull/9

            <test_csc.py >

            1. I thought the salobj also provides a timeout error?

            https://github.com/lsst-ts/ts_hexapod/pull/9/files#diff-d9365181a6fb138131218b600a0dfccfR136

            Show
            rowen Russell Owen added a comment - Te-Wei's review [base_csc.py] 1. Not sure the meaning of "Must include ``SET_STATE``". This is an Enum (CommandCode), right? https://github.com/lsst-ts/ts_hexrotcomm/pull/7/files#diff-9884fc357e4ad38105bdf51d866033e9R65 Do you mean the user need to use "CommandCode.SET_STATE"? Sorry, my intuition is to check this file (enums.py) for such kind of enum. Where is it? https://github.com/lsst-ts/ts_hexrotcomm/blob/c99e93d8c93e6cfbb18fb5020d0736dc552b7e8c/python/lsst/ts/hexrotcomm/enums.py I am not sure this is what you impied or not? https://github.com/lsst-ts/ts_hexrotcomm/blob/c99e93d8c93e6cfbb18fb5020d0736dc552b7e8c/python/lsst/ts/hexrotcomm/simple_mock_controller.py#L36 When I began to review the PR of ts_rotator, I realized the CommandCode might be in the enum.py of ts_rotator. I thought the ts_hexrotcomm is the upstream of ts_rotator. If this is the case, is it a good idea to let the upstream code to have the knowledge of the code in downstream? Maybe the docstring can point out the CommandCode enum comes from the repo such as the ts_rotator to let other maintainer has the idea if my guess is correct. But maybe their relationship is not what I thought. 2. Do we need to consider the command might fail? https://github.com/lsst-ts/ts_hexrotcomm/pull/7/files#diff-9884fc357e4ad38105bdf51d866033e9R252-R253 [simple_CSC.py] 1. I do not know the `salobj.Remote` can call this function in server: https://github.com/lsst-ts/ts_hexrotcomm/pull/7/files#diff-2dcda03af7b6b135f00ed9059cc0bc30R119 How do we do this? `await m.cmd_move_sequentially.set_start()`? Is this correct? <ts_rotator> https://github.com/lsst-ts/ts_rotator/pull/8/files [rotator_csc.py] 1. General question. I saw many `make_command()` and `run_command()` are used. This makes me thought you are using the Command Factory pattern. Is this correct? But I thought people will create a CommandFactory class and each command will be an object inherit from the default command class. Or maybe I am wrong in the guess of your intention? <ts_hexapod> https://github.com/lsst-ts/ts_hexapod/pull/9 <test_csc.py > 1. I thought the salobj also provides a timeout error? https://github.com/lsst-ts/ts_hexapod/pull/9/files#diff-d9365181a6fb138131218b600a0dfccfR136
            Hide
            ttsai Te-Wei Tsai added a comment -

            All changes look good to me! Great job done!

            Show
            ttsai Te-Wei Tsai added a comment - All changes look good to me! Great job done!
            ttsai Te-Wei Tsai made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            rowen Russell Owen added a comment - - edited

            Thank you for the very helpful review. Here are my responses:

            [base_csc.py]

            1. CommandCode is an enum that specifies the command codes supported by the low-level controller for Rotator or Hexapod (or SimpleCsc). The only command code directly used by BaseCsc is SET_STATE, so the enum must include that item. I have expanded the text of the help string.

            Note: Moog wrote just one command code enum. That one enum is used both for Hexapod and Rotator, even though some values are only relevant to one or the other. That is an implementation detail that I prefer not to duplicate. It is more flexible to allow the command codes for Rotator and Hexapod to be different, and it is less confusing to only include codes for commands that are appropriate to the specific device being controlled

            2. Excellent question. Unfortunately Moog provided no reliable way to tell if a command has failed. It would be quite difficult to determine that at this level. In the long run I hope we can modify the interface to report failure immediately. Meanwhile I do my best to detect a command that cannot be executed in the CSC itself, and only try to command the low-level controller if I am confident the command can be executed.

            [simple_CSC.py]

            1. Great question. This method is only intended for unit tests and seemed reasonable to add to SimpleCsc because that is also only intended for unit tests. But I was still uneasy about it. Now that I look at it, I see it only uses public interfaces of the class, and is only called in test_csc.py,, so I will move the method there and simplify it.

            [rotator_csc.py]

            1. I am using a factory function: BaseCsc.make_command makes a hexrotcomm.Command. I will document the return value. I hope that will make it a bit clearer.

            These commands are a struct sent to the low-level controller. Moog only defines one such struct (and it is used for both Rotator and Hexapod) so there is no need to have different types of Command.

            Having a factory function is helpful for several reasons:

            • I can set the sync_pattern field, which is specific to the device being controlled.
            • the class is a ctypes.Structure, which means I cannot define the constructor. The factory function allows me to document the arguments, handle defaults, and check that the command code is a valid value.

            <test_csc.py>

            1. RemoteCommand.set_start always raises an AckError if it fails, so I provide a subclass of that for the special case that set_start times out before hearing from the CSC. However, for any other timeout I always use the standard asyncio TimeoutError.

            Show
            rowen Russell Owen added a comment - - edited Thank you for the very helpful review. Here are my responses: [base_csc.py] 1. CommandCode is an enum that specifies the command codes supported by the low-level controller for Rotator or Hexapod (or SimpleCsc). The only command code directly used by BaseCsc is SET_STATE , so the enum must include that item. I have expanded the text of the help string. Note: Moog wrote just one command code enum. That one enum is used both for Hexapod and Rotator, even though some values are only relevant to one or the other. That is an implementation detail that I prefer not to duplicate. It is more flexible to allow the command codes for Rotator and Hexapod to be different, and it is less confusing to only include codes for commands that are appropriate to the specific device being controlled 2. Excellent question. Unfortunately Moog provided no reliable way to tell if a command has failed. It would be quite difficult to determine that at this level. In the long run I hope we can modify the interface to report failure immediately. Meanwhile I do my best to detect a command that cannot be executed in the CSC itself, and only try to command the low-level controller if I am confident the command can be executed. [simple_CSC.py] 1. Great question. This method is only intended for unit tests and seemed reasonable to add to SimpleCsc because that is also only intended for unit tests. But I was still uneasy about it. Now that I look at it, I see it only uses public interfaces of the class, and is only called in test_csc.py,, so I will move the method there and simplify it. [rotator_csc.py] 1. I am using a factory function: BaseCsc.make_command makes a hexrotcomm.Command . I will document the return value. I hope that will make it a bit clearer. These commands are a struct sent to the low-level controller. Moog only defines one such struct (and it is used for both Rotator and Hexapod) so there is no need to have different types of Command . Having a factory function is helpful for several reasons: I can set the sync_pattern field, which is specific to the device being controlled. the class is a ctypes.Structure, which means I cannot define the constructor. The factory function allows me to document the arguments, handle defaults, and check that the command code is a valid value. <test_csc.py> 1. RemoteCommand.set_start always raises an AckError if it fails, so I provide a subclass of that for the special case that set_start times out before hearing from the CSC. However, for any other timeout I always use the standard asyncio TimeoutError.
            Hide
            rowen Russell Owen added a comment -

            Merged ts_xml to develop
            Merged ts_hexrotcomm to develop and master and tagged as v0.2.0
            Merged ts_hexapod to develop and master and tagged as v0.3.0
            Merged ts_rotator to develop and master and tagged as v0.2.0

            Show
            rowen Russell Owen added a comment - Merged ts_xml to develop Merged ts_hexrotcomm to develop and master and tagged as v0.2.0 Merged ts_hexapod to develop and master and tagged as v0.3.0 Merged ts_rotator to develop and master and tagged as v0.2.0
            rowen Russell Owen made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            rowen Russell Owen made changes -
            Link This issue relates to DM-20969 [ DM-20969 ]
            rowen Russell Owen made changes -
            Link This issue relates to DM-20969 [ DM-20969 ]
            ksiruno Kevin Siruno made changes -
            Remote Link This issue links to "Page (Confluence)" [ 26701 ]
            hdrass Holger Drass made changes -
            Link This issue relates to SUMMIT-5251 [ SUMMIT-5251 ]
            hdrass Holger Drass made changes -
            Link This issue relates to SUMMIT-5404 [ SUMMIT-5404 ]
            hdrass Holger Drass made changes -
            Link This issue relates to SUMMIT-5796 [ SUMMIT-5796 ]
            ksiruno Kevin Siruno made changes -
            Remote Link This issue links to "Page (Confluence)" [ 26701 ]

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Te-Wei Tsai
              Watchers:
              Andrew Heyer [X] (Inactive), Bo Xin [X] (Inactive), Michael Reuter, Rob Bovill, Russell Owen, Te-Wei Tsai
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.