Add the hexapod events. #926
Conversation
rbovill
force-pushed
the
tickets/DM-46560
branch
from
0875a66 to
bc77d5f
Compare
teweitsai
force-pushed
the
tickets/DM-46560
branch
from
bc77d5f to
dc7c025
Compare
There was a problem hiding this comment.
Changes looks good, but I think they are a but redundant.
I think there's really no need for the CSC to publish the highCurrect event, as this could be done by the Watcher and also, my impression is that the driver state is already covered by another event.
In any case, I am approving the PR so, feel free to ignore my comments above.
| </SALEvent> | ||
| <SALEvent> | ||
| <Subsystem>MTHexapod</Subsystem> | ||
| <EFDB_Topic>MTHexapod_logevent_transitionToIdle</EFDB_Topic> |
There was a problem hiding this comment.
Isn't this information already available in the MTHexapod_logevent_controllerState event? Is there really a need to publish just this boolean flag?
Don't want to be picky but I think something like driversIdle for a topic name and idle for attribute would probably be better.
There was a problem hiding this comment.
If we do not have this event, people will need to use the events of MTHexapod_logevent_controllerState and MTHexapod_logevent_summaryState together to judge what happens. A little bit complex but is doable.
| @@ -411,4 +411,28 @@ | |||
| <Count>1</Count> | |||
| </item> | |||
| </SALEvent> | |||
| <SALEvent> | |||
| <Subsystem>MTHexapod</Subsystem> | |||
| <EFDB_Topic>MTHexapod_logevent_highCurrent</EFDB_Topic> | |||
There was a problem hiding this comment.
Are you sure you want to add this to the CSC? why not let the Watcher take care of this?
|
The reason to have these two events is to make the implementation of watcher alarms to have very simple logic. But we can remove these two events and put the complexities to the watcher alarms instead as well. Therefore, I will close this PR. Thanks! |
Add the MTHexapod_logevent_highCurrent and MTHexapod_logevent_transitionToIdle.