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

Write an MT rotator CSC in Python

    XMLWordPrintable

Details

    • 8
    • TSSW Sprint - Sep 29 - Oct 13, TSSW Sprint - Oct 14 - Oct 27
    • Telescope and Site

    Description

      Write an MT rotator CSC in Python, including a basic simulation mode.

      Note that the simulator is the hard part. I plan to start with a very elementary model (no slewing) in this ticket, then refine it in another ticket.

      Attachments

        Issue Links

          Activity

            rowen Russell Owen added a comment -

            ttsai's review of ts_rotator:
            <ts_rotator>
             
            1. Does this mean we want to restrict the speed can not be 0? I think it might be better say "assert speed >= 0" if it does not hurt.
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R46
             
            2. For the time.time(), how about to use the time.perf_counter() instead?
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R50
             
            This is because I read this article recently:
            https://www.webucator.com/blog/2015/08/python-clocks-explained/
             
            I assume you want to consider the delta time in the future because I saw the self._start_time and self._end_time. But maybe I am wrong.
             
            It looks like I am wrong when I read here:
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R76
             
            You estimate the duration time. However, this estimation depends on how accurate you want it. Maybe the consideration of acceleration is needed if you need the higher accuracy.
             
            3. I think use the time to estimate the actuator is moving or not might be dangeraous. But it looks like this is just a mock class. Maybe this is fine:
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R96
             
            4. When I played with the real hardware, the positionSet command is needed before the move command.
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R153
             
            5. I feel it is easier to understand by just saying: self.encoder_resolution = 200000
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R159
             
            6. Does it really matter that we do not want to import the logging library in the initail beginning of script?
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R185
             
            Although I know we could import the library in any time....
             
            7. I feel there are too many indentation of if loops already.
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R241-R322
             
            I should say it might be hard to debug after 1 months even for the original developer. But it may be fine. I know some AI program in the past did the similar thing.
             
            8. What is "_n" in "self._tracking_started_n"?
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R96
             
            9 In my memory, after I used the SAL to clearError, I did not use the GUI to put the rotator into the DDS mode. But the rotator was still controllable by SAL. But maybe I am wrong in my memory.
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R246-R248
             
            10. Although I recorded two times of "clearError" command are needed with the Rotator, I also recorded that Scott needed to do something between these two commands. Scott and I saw that the rotator will not clear the error if we just send two times of clearError commands without doing something in the hardware part.
             
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R256-R257
             
            https://confluence.lsstcorp.org/pages/viewpage.action?pageId=118589798
             
            11. The acceleration can be 0 althought it might not make sense:
            https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R264
             

            rowen Russell Owen added a comment - ttsai 's review of ts_rotator: <ts_rotator>   1. Does this mean we want to restrict the speed can not be 0? I think it might be better say "assert speed >= 0" if it does not hurt. https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R46   2. For the time.time(), how about to use the time.perf_counter() instead? https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R50   This is because I read this article recently: https://www.webucator.com/blog/2015/08/python-clocks-explained/   I assume you want to consider the delta time in the future because I saw the self._start_time and self._end_time. But maybe I am wrong.   It looks like I am wrong when I read here: https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R76   You estimate the duration time. However, this estimation depends on how accurate you want it. Maybe the consideration of acceleration is needed if you need the higher accuracy.   3. I think use the time to estimate the actuator is moving or not might be dangeraous. But it looks like this is just a mock class. Maybe this is fine: https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R96   4. When I played with the real hardware, the positionSet command is needed before the move command. https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R153   5. I feel it is easier to understand by just saying: self.encoder_resolution = 200000 https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R159   6. Does it really matter that we do not want to import the logging library in the initail beginning of script? https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R185   Although I know we could import the library in any time....   7. I feel there are too many indentation of if loops already. https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-e6ee538b3659b987982aa4710ebeaed8R241-R322   I should say it might be hard to debug after 1 months even for the original developer. But it may be fine. I know some AI program in the past did the similar thing.   8. What is "_n" in "self._tracking_started_n"? https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R96   9 In my memory, after I used the SAL to clearError, I did not use the GUI to put the rotator into the DDS mode. But the rotator was still controllable by SAL. But maybe I am wrong in my memory. https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R246-R248   10. Although I recorded two times of "clearError" command are needed with the Rotator, I also recorded that Scott needed to do something between these two commands. Scott and I saw that the rotator will not clear the error if we just send two times of clearError commands without doing something in the hardware part.   https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R256-R257   https://confluence.lsstcorp.org/pages/viewpage.action?pageId=118589798   11. The acceleration can be 0 althought it might not make sense: https://github.com/lsst-ts/ts_rotator/pull/5/files#diff-03362fafdd1aac9f841e4c5f118184f8R264  

            1 and 11): It is not safe to use a velocity or acceleration limit of 0 because moves will never complete. I don't know if the cRIO accepts 0 but I'd rather not find out.

            2) Good suggestion. I will use time.monotonic instead of time.time.

            As to the accuracy of the model: it would be better if I used the actuator from ts_ATMCSSimulator, which handles slewing and tracking, but I filed a separate ticket for that and I'm not sure we need it.

            3) Yes, it's just a simulator.

            4) Thank you. I will update the comment.

            5) I respectfully disagree. I love the new support for underlines in integer contants. I admit it may take time for folks to become used to them.

            6) Good catch. That is leftover debugging code. I will remove it.

            7) I agree that the MockMTRotatorController run_command method is messy and difficult to understand.

            I refactored the code as follows:

            • Added a method for each command. This method is responsible for checking initial conditions.
            • Added a lookup table that ties commands to those new methods.

            I think it looks a lot better now.

            8) The "_n" means it is a count. I will rename it to _tracking_started_telemetry_counter.

            9) I assume you are correct and remove anything that says clearError goes to OFFLINE/PUBLISH_ONLY.

            10) Based on your notes it appears that you used the interlock system to put the rotator into a FAULT state. I think it likely that the extra operation you had to perform was related to the interlock system.

            rowen Russell Owen added a comment - 1 and 11): It is not safe to use a velocity or acceleration limit of 0 because moves will never complete. I don't know if the cRIO accepts 0 but I'd rather not find out. 2) Good suggestion. I will use time.monotonic instead of time.time. As to the accuracy of the model: it would be better if I used the actuator from ts_ATMCSSimulator, which handles slewing and tracking, but I filed a separate ticket for that and I'm not sure we need it. 3) Yes, it's just a simulator. 4) Thank you. I will update the comment. 5) I respectfully disagree. I love the new support for underlines in integer contants. I admit it may take time for folks to become used to them. 6) Good catch. That is leftover debugging code. I will remove it. 7) I agree that the MockMTRotatorController run_command method is messy and difficult to understand. I refactored the code as follows: Added a method for each command. This method is responsible for checking initial conditions. Added a lookup table that ties commands to those new methods. I think it looks a lot better now. 8) The "_n" means it is a count. I will rename it to _tracking_started_telemetry_counter . 9) I assume you are correct and remove anything that says clearError goes to OFFLINE/PUBLISH_ONLY. 10) Based on your notes it appears that you used the interlock system to put the rotator into a FAULT state. I think it likely that the extra operation you had to perform was related to the interlock system.
            rowen Russell Owen added a comment -

            My changes are extensive enough that I have asked Te-Wei to have another look.

            rowen Russell Owen added a comment - My changes are extensive enough that I have asked Te-Wei to have another look.
            ttsai Te-Wei Tsai added a comment -

            The refactored code looks good and is easy to understand! Great job!

            ttsai Te-Wei Tsai added a comment - The refactored code looks good and is easy to understand! Great job!
            rowen Russell Owen added a comment -

            Thank you for the helpful review. It made a big difference.

            rowen Russell Owen added a comment - Thank you for the helpful review. It made a big difference.

            People

              rowen Russell Owen
              rowen Russell Owen
              Te-Wei Tsai
              Russell Owen, Te-Wei Tsai
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 4 hours
                  4h

                  Jenkins

                    No builds found.