sot / kadi

Chandra commands and events
https://sot.github.io/kadi
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Add "HRC not run" event to handle HRC being disabled #309

Closed taldcroft closed 6 months ago

taldcroft commented 6 months ago

Description

This PR provides a better way to handle HRC being disabled than adding a bunch of Command not run events. It follows the pattern from Load not run and Observing not run.

I have added this new event type to the FOT MP documentation page just to get it done with now. There is no danger of this accidentally getting into the flight Events Sheet.

Interface impacts

Supports new HRC not run entry on Events Sheet.

Testing

Unit tests

kadi/commands/tests/test_commands.py ............................................................................. [ 34%] kadi/commands/tests/test_states.py ......................x.............................................x................ [ 73%] ....... [ 76%] kadi/commands/tests/test_validate.py .................... [ 85%] kadi/tests/test_events.py .......... [ 90%] kadi/tests/test_occweb.py ...................... [100%]

========================================== 219 passed, 2 xfailed in 141.11s (0:02:21) ==========================================



Independent check of unit tests by Jean
- [x] Linux

### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
No functional testing.
jeanconn commented 6 months ago

As a reviewer on this I'm working just from the kadi states understanding of how HRC is commanded. So from that perspective -- is there a name for the nominal operation of HRC via these SCS activations as compared to the hardware commands? This ends up just being a documentation question really.

taldcroft commented 6 months ago

is there a name for the nominal operation of HRC via these SCS activations

I'm not aware of a good and descriptive name, but they fall within the "HRC return to science operations concept" (along with a hundred other things).

jeanconn commented 6 months ago

OK. Sounds fine as long as it will be clear to the users here that the HRC not run is only going to apply to this nominal operations concept and not to any hardware commands.

taldcroft commented 6 months ago

Sounds fine as long as it will be clear to the users here that the HRC not run is only going to apply to this nominal operations concept and not to any hardware commands.

In fact the hardware commands do actually run on board, they just don't do anything because the instrument is off.

But yes, there is a bit of duct tape in all this. F_HRC_SAFING disables SCSs 92, 93 and 134, but the kadi state values depend on 89, 90, and 134 due to the way HRC is run. So this PR ignores 89, 90 instead of 92, 93, because that is what makes things work.

taldcroft commented 6 months ago

Thinking more, F_HRC_SAFING can be run in the middle of an HRC observation and it turns off HRC immediately. So I need to insert some commands at the event date to do that.

jeanconn commented 6 months ago

"In fact the hardware commands do actually run on board, they just don't do anything because the instrument is off."

Right, I meant hardware commands in the loads, as those are the ones that are defined as state-changing-things in kadi.

jeanconn commented 6 months ago

"So I need to insert some commands at the event date to do that."

Sounds good to take the time to do that update to this PR to future-proof it.

taldcroft commented 6 months ago

I meant hardware commands in the loads, as those are the ones that are defined as state-changing-things in kadi.

OK now I get your drift. There are dozens of HRC hardware commands in the loads. But you were referring to just the two state-changing ones that control the 15V and 24V. The 15V is never commanded in the loads in the Return to science concept and (if it was included) there would be no way to stop it from running besides running SCS-107.

But the 24V hardware command is in the loads and it becomes a no-op when the 15V is off. This is reflected in 1e46442.

taldcroft commented 6 months ago

@jeanconn - thanks for the comments, this PR is definitely better now and hopefully ready!