Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: ts_main_telescope
-
Labels:
-
Story Points:1
-
Epic Link:
-
Sprint:TSSW Sprint - Jan 18 - Feb 1
-
Team:Telescope and Site
-
Urgent?:Yes
Description
At present the command and reply ports for the MT hexapod and rotator CSCs are constants in ts_hexrotcomm that are the same for all CSCs. Change this to allow the port to be different for each CSC.
I plan to make the telemetry ports device-specific, but retain the rule that the command port is one greater. This is in hopes that we can eventually eliminate the extra port (each of the two sockets is unidirectional, so it should be easy).
Attachments
Issue Links
- is triggered by
-
- Done
Activity
This update looks good to me! Thanks for the supports of port number!
Te-Wei's review:
[ts_hexrotcomm]
[base_csc.py]
1. Why choose the 6210? Are we using this one? Or is this just a template or place holder?
https://github.com/lsst-ts/ts_hexrotcomm/pull/29/files?file-filters%5B%5D=.py#diff-82220484ebf49d68d0a7d709bfada0f73a28ad4feea02d6ffaa7e61049358b21R34
Same question here:
https://github.com/lsst-ts/ts_hexrotcomm/pull/29/files?file-filters%5B%5D=.py#diff-d60eb8adf02e947db4ab6747b7d49a170328fbc811f171f957014d04be344155R42
In addtion, you say the command port is one larger. In this case, the SIMPLE_TELEMETRY_PORT should be 6209 instead of 6210. Right?
And I began to suspect the values here are just the place holder?
https://github.com/lsst-ts/ts_hexrotcomm/pull/29/files?file-filters%5B%5D=.py#diff-d60eb8adf02e947db4ab6747b7d49a170328fbc811f171f957014d04be344155R42-R110
[ts_mthexapod]
[constants.py]
1. Maybe say the command port is one larger than the telemetry port:
https://github.com/lsst-ts/ts_mthexapod/pull/34/files?file-filters%5B%5D=.py#diff-2e3dedf9fff3aadf3b62a768b5cb3e4d8b281523c4eab69b348e0e4a493fb1aaR58
2. I prefer you keep these:
https://github.com/lsst-ts/ts_mthexapod/pull/34/files?file-filters%5B%5D=.py#diff-2e3dedf9fff3aadf3b62a768b5cb3e4d8b281523c4eab69b348e0e4a493fb1aaL34-L37
Instead of hard-coded them here:
https://github.com/lsst-ts/ts_mthexapod/pull/34/files?file-filters%5B%5D=.py#diff-2e3dedf9fff3aadf3b62a768b5cb3e4d8b281523c4eab69b348e0e4a493fb1aaR77-R84
They will be the magic numbers for the future's developer to try to understand what are the 0x6666 and 0xB4B4.
[mock_controller.py]
1. The command_port and telemetry_port can use the None value or not?
https://github.com/lsst-ts/ts_mthexapod/pull/34/files?file-filters%5B%5D=.py#diff-47c05f327d4918640135d9308a6795f969361944c303492567e5bad0dd3737beR56-R121
Or the None value here is just a placeholder?
[ts_mtrotator]
[rotator_commander.py]
1. You want the digit to be 1 or 2 here?
https://github.com/lsst-ts/ts_mtrotator/pull/34/files?file-filters%5B%5D=.py#diff-0cd527a84bd148fd208dde64caff158f7272e94b88c646f3215bfd083743b149R65
https://github.com/lsst-ts/ts_mtrotator/pull/34/files?file-filters%5B%5D=.py&file-filters%5B%5D=.rst#diff-9a6f568586a374c279e08bc97cf236c41d70380ea6c461a8ea69c5a1c45d3a1eR16
https://github.com/lsst-ts/ts_mtrotator/pull/34/files?file-filters%5B%5D=.py#diff-0cd527a84bd148fd208dde64caff158f7272e94b88c646f3215bfd083743b149R97
My response to Te-Wei's review:
[ts_hexrotcomm]
[base_csc.py]
1. The value for SIMPLE_TELEMETRY_PORT is arbitrary. I will add a code comment explaining this.
The constant BASE_CSC_COMMAND_PORT is a mistake (thank you for catching it). During development I changed BASE_CSC_COMMAND_PORT to SIMPLE_TELEMETRY_PORT (the one you ask about next) and I forgot to remove this old constant.
Regarding "you say the command port is one larger. In this case, the SIMPLE_TELEMETRY_PORT should be 6209 instead of 6210. Right?". I don't think it matters because the value 6210 is arbitrary. The command port will be 6211.
I am not sure about your question of the place holder. I could not find the reference.
[ts_mthexapod]
[constants.py]
1. (Doc string enhancement) Good suggestion. I updated the description.
2. (Values for ControllerConstants in constants.py). Good point, though my resolution may not please you.
The ControllerConstants class records magic numbers used by the vendor.
The doc string is intended to explain what each value means.
At present the way I construct ControllerConstants in constants.py is a confusing mix of numeric and named constants.
I am changing the code code so that all values used to construct ControllerConstants are numbers, rather than named constants.
I strongly prefer numbers because it eliminates a lot of repetion, which I believe enhances readability.
The other extreme is to 8 separate named constants which are then used to construct the ControllerConstants instances
(and not used for anything else).
Note that this means getting rid of the FieldId enum class, which was only being used to construct ControllerConstants.
[mock_controller.py]
1. Excellent question.
This is regarding the ports for MockMTHexapodController.
The default values were, and still are, "the standard port for this device".
Formerly these were two specific numbers, but now that each device has its own standard ports, numeric defaults no longer work.
So I now use None to mean "the standard port for this device", and None is the default.
However, I forgot to update the doc string with this information. I have now done that.
[ts_mtrotator]
[rotator_commander.py]
1. This is in reference to the new digits argument of RotatorCommander._special_telemetry_callback.
I chose the default digits=2 to match the existing code in ts_salobj.
The choice of default makes very little difference; it only affects how I to call this method
from the two custom telemetry handlers.
The motors custom handler uses 1 digit because there is too much noise in the 2nd digit,
resulting in unwanted extra output. It also ignores the "raw" field when deciding whether to print new data.
That ignore could be handled without a custom handler in ts_salobj 6.2,
and a custom handler will be much simpler in salobj 6.3 (there will be no need for _special_telemetry_callback).
The rotation handler uses the standard 2 digits, but ignores the timestamp field.
This custom telemetry handler is unnecessary in ts_salobj 6.2 because it ignores the timestamp field.
So this code is temporary. Some can disappear when we require ts_salobj 6.2 and almost all of it will go away
if and when we require ts_salobj 6.3 (which is in development).
I felt it safest to retain compatibility with salobj 6.1 for now, as we need to deploy this new software as quickly as possible.
I was able to fix the ts_mthexapod Jenkins tests. The timeout for creating the CSC was unreasonably short (though it worked on my computer); I restored it to the standard value.
Released:
- ts_hexrotcomm v0.14.0
- ts_mthexapod v0.13.0
- ts_mtrotator v0.11.0
The new ports are: