1. I agree with you; I do not like the name ts_pymoog. However I would like to avoid underscores in package names implies sub-package namespaces (at least the way we do things), for example ts_ctrl_crio_moog implies lsst.ts.ctrl.crio.moog.
After discussion with several people I have renamed the package to ts_hexrotcomm.
2, 5 and 6: In my opinion the Moog "wrapper" code implements three different CSCs. Each has its own states and SAL topics. It is an internal detail that these CSCs are all running in the same process. The only clue for the user is that if one brings down the "wrapper" all three CSCs go away.
I have tried to update my documentation to make that clearer.
3. I have decided to go with "mock" everywhere. I think "emulate" is also a reasonable term, but CSCs have a simulation mode, so I prefer to restrict the number of terms to "simulate" and "mock". I could get rid of mock as well, but it is common and I prefer BaseMockController to BaseControllerSimulator or BaseSimulatedController.
4. I did not understand your point so I asked offline. Here is my understanding now:
The current CSCs (middleware) always say that acknowledge a command as succeeded even if it is rejected. They output the rejectedCommand event to reject a command.
These details are out of scope for this package, but I will say that I find this behavior undesirable. My plan for my rotator and hexapod CSCs to properly reject commands when they can do so.
7. Good catch. I changed to the code to not use salobj, as it seems silly to require the package just for this.
8. Excellent point. I added a code comment that I hope will explain what is going on.
9. I initially thought that readers should be closed, but it turns out that asyncio socket readers cannot be closed; they have no close method.
10. "coros" means "coroutines". I will rename the variable to coroutines. (Note that "coro" is a common abbreviation in Python asyncio examples, but even so, I hope this makes the code easier to read).
11. I hope it be clearer now that I have clarified item 8.
12. Excellent point and I see that the doc string for set_command_writer is wrong, as well. I updated the doc strings to document the parameters and explain why the arguments are ignored. For the record these are called by asyncio.start_server which provides both a reader and a writer.
13. Good point. I tried changing struct to cstruct everywhere but I felt it hampered readability. I am not keen to add the underscore, so I plan to stick with struct despite some reservations.
14. I think asyncio can easily support these data rates. A timing test in salobj shows that one process can both produce and consume DDS messages at a rate of over 5000 messages/second. I would expect TCP/IP to do even better, though perhaps the use of ctypes to encode and decode the data might be a factor (I have not timed that).
Although it is possible to run higher data rates for debugging, the standard rate is 20 Hz and I feel if the CSCs can reliably keep up with that then we should be in fine shape.