Ability to Enable Laser CSC without Laser Connection #55
Conversation
couger01
force-pushed
the
tickets/DM-45685
branch
2 times, most recently
from
6323830 to
a2cc2f3
Compare
couger01
force-pushed
the
tickets/DM-45685
branch
from
a2cc2f3 to
b0bfd6a
Compare
|
I think this is working! It will be useful to have this capability on the summit. |
There was a problem hiding this comment.
Eric asked me to take a look at this PR. In principle, I think it is perfectly fine to allow the CSC to operate with partial functionality.
I made an inline comment about publishing errorCode without sending the CSC to fault. This is actually not allowed in our system. See my comment inline.
A couple other things for you to consider:
- You might want to expand the error message in the commands that don't work with the laser disconnected. Instead of just doing:
raise salobj.ExpectedError("Not connected")
You might want to say something like
Command cannot be executed, laser controller is not connected. If you expect the laser to be connected you might have to cycle the CSC or check the hardware for problems.
Same for the thermal commands. Make sure you expand the errors so users have more information about what is going on.
On a separate note, you might want to make this functionality configurable. Maybe add a flag to specify that the CSC can or cannot operate without the laser.
python/lsst/ts/tunablelaser/csc.py
Outdated
| TunableLaser.LaserDetailedState.NONPROPAGATING_CONTINUOUS_MODE | ||
| ) | ||
| self.log.exception("Connection to laser failed.") | ||
| await self.evt_errorCode.set_write( |
There was a problem hiding this comment.
you should really not publish this event stand alone like this. This is part of going to fault, which I see you commented below. What is the idea here? you want to publish an error but not go to fault? this is really not allowed.
I think what you might want to do here is expand the error message in the exception above to specify that the laser functionality won't be available. Something like:
self.log.exception("Failed to connect to the laser controller. The CSC will operate with reduced functionality.")
|
@tribeiro thanks very much for taking a look. I think it would be better to set a specific flag in the configuration rather than simply setting the host name to 'none'. I presume this wouldn't require any changes outside ts_TunableLaser, and eventually ts_config_mtcalsys? |
This will allow us to communicate with the Thermal System even if the Laser isn't ON