Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: ts_main_telescope
-
Labels:
-
Story Points:8
-
Epic Link:
-
Sprint:TSSW Sprint - Sep 29 - Oct 13, TSSW Sprint - Oct 14 - Oct 27
-
Team: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
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.
My changes are extensive enough that I have asked Te-Wei to have another look.
The refactored code looks good and is easy to understand! Great job!
Thank you for the helpful review. It made a big difference.
Te-Wei Tsai'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