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

Make MTAirCompressor CsC configurable

    XMLWordPrintable

Details

    • Improvement
    • Status: Done
    • Resolution: Done
    • None
    • ts_main_telescope
    • None
    • 4
    • TSSW Sprint - Jul 18 - Aug 01, TSSW Sprint - Aug 01 - Aug 15, TSSW Sprint - Aug 15 - Aug 29, TSSW Sprint - Aug 29 - Sep 12, TSSW Sprint - Sep 12 - Sep 26
    • Telescope and Site

    Description

      Change hardcoded grace_period from MTAirCompressor into configurable value. Also drop disable/enable state transition for powering up/down and implement command to do that.

      Attachments

        Issue Links

          Activity

            rowen Russell Owen added a comment -

            Please consider also adding hostname, port, and unit to the config schema. Since this is an indexed CSC, you need some way to specify different values for different air compressors ("instances"). The recommended way to handle this is to make this part of the configuration a dict (jsonschema "object)) of salIndex: instance-specific config. Look at ts_ess_csc and ts_mthexapod for two examples. The big advantage to this is that it makes starting the CSC simple: just specify the index and it will figure out which hostname, port and unit to use. If you specify an unsupported index (one with no instance-specific configuration) the CSC should complain and quit.

            rowen Russell Owen added a comment - Please consider also adding hostname, port, and unit to the config schema. Since this is an indexed CSC, you need some way to specify different values for different air compressors ("instances"). The recommended way to handle this is to make this part of the configuration a dict (jsonschema "object)) of salIndex: instance-specific config. Look at ts_ess_csc and ts_mthexapod for two examples. The big advantage to this is that it makes starting the CSC simple: just specify the index and it will figure out which hostname, port and unit to use. If you specify an unsupported index (one with no instance-specific configuration) the CSC should complain and quit.

            This ticket involves updating ts_xml. Note that ts_xml 12.1 will not be released before September 2022.

            wvreeven Wouter van Reeven added a comment - This ticket involves updating ts_xml. Note that ts_xml 12.1 will not be released before September 2022.

            Please see linked PRs. For cRIOpy:

            https://github.com/lsst-ts/ts_cRIOpy/pull/34

            pkubanek Petr Kubanek added a comment - Please see linked PRs. For cRIOpy: https://github.com/lsst-ts/ts_cRIOpy/pull/34
            ttsai Te-Wei Tsai added a comment -

            I reviewed the ts_cRIOpy in GitHub.

            ttsai Te-Wei Tsai added a comment - I reviewed the ts_cRIOpy in GitHub.

            Looks like there are several of us reviewing this ticket... I reviewed https://github.com/lsst-ts/ts_MTAirCompressor/pull/10 and https://github.com/lsst-ts/ts_config_mttcs/pull/43

            One general comment that I left on both PRs that I think can get addressed in the future is that the CSC is now using 2 different mechanisms to get configurations; command line arguments with (hard coded) internal defaults and configurable CSC.

            I think you should ditch the cmd line arguments + internal defaults and go all in the configurable CSC strategy. Again, you can use _init.yaml/_summit.yaml for the defaults..

            tribeiro Tiago Ribeiro added a comment - Looks like there are several of us reviewing this ticket... I reviewed https://github.com/lsst-ts/ts_MTAirCompressor/pull/10 and https://github.com/lsst-ts/ts_config_mttcs/pull/43 One general comment that I left on both PRs that I think can get addressed in the future is that the CSC is now using 2 different mechanisms to get configurations; command line arguments with (hard coded) internal defaults and configurable CSC. I think you should ditch the cmd line arguments + internal defaults and go all in the configurable CSC strategy. Again, you can use _init.yaml/_summit.yaml for the defaults..

            People

              pkubanek Petr Kubanek
              pkubanek Petr Kubanek
              Russell Owen, Te-Wei Tsai, Tiago Ribeiro
              Petr Kubanek, Russell Owen, Te-Wei Tsai, Tiago Ribeiro, Wouter van Reeven
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.