Correct results for changing ACIS SIMODE state

[jzuhone]

Description

kadi uses ACISPKT parameter block commands to determine the value of the ACIS si_mode state. The string corresponding to the SIMODE can be deciphered from the string in the parameter block.

Currently, kadi does not compute the value of the SIMODE string correctly. For example, the current code will take this command, which corresponds to computing a bias, and return this SIMODE string:

WT0057A034 --> TE_0057A

for the following non-bias version of this command, this SIMODE string is returned:

WT0057B034 --> TE_0057B

However, this is incorrect. There is no such SIMODE string "TE_0057B". The correct way to determine the strings is documented by ACIS as:

Packet Identifier: WTmmmmmvvs

The "WT" here specifies the "writeTe" command packet format. The remaining hex digits, mmmmmvvs, constitute the parameter block id, and are used by the ACIS flight software to identify the TE parameter block. SACGS enforces the following conventions for the parameter block id:

The first five hex digits, mmmmm, end in an even digit when the pblock calls for taking a bias, and in an odd digit when it does not.

In the ACIS tables, the parameter block ACISPKTs correspond to these SIMODE strings:

Bias version:

WT0057A034 --> TE_0057AB

No-bias version:

WT0057B034 --> TE_0057A

This PR implements code to calculate these for all TE and CC SIMODEs to update the si_mode state. It also includes a mapping for the NIL ACIS SIMODEs that are used during HRC observations. However, for a few ACISPKT values of this kind, there is a one-to-two mapping from packet string to SIMODE string. In order to determine which to use, it is necessary to know if the SIMODE ends in an odd or even number. However, I was not sure how to get at the obsid with the current code. I thought about implementing a callback as in other transitions, but then it wasn't clear how to use the value of tlmsid which contains the ACISPKT string. Help from @taldcroft here would be appreciated.

Once we resolve this last issue I will test and it will no longer be WIP.

Interface impacts

AFAIK, no current code depends on the value of the si_mode state, but this will be the case for chandra_limits.

Testing

New unit tests added.

Unit tests

• [ ] No unit tests
• [x] Mac
• [ ] Linux
• [ ] Windows

Independent check of unit tests by @taldcroft at a9c2369e:

• [x] Mac

Functional tests

Hand-checked values for more complicated cases not included in unit tests

[jzuhone]

The "check format using ruff" failure appears unrelated.

[taldcroft]

@jzuhone - about callbacks with additional arguments, have a look at the code for the FidsTransition. I recently ran into the same issue and came up with a solution that isn't quite elegant but works.

I was not sure how to get at the obsid with the current code

Was "obsid" a typo? Maybe I don't need to know but in general within a callback you have access to the current state and can modify the state as needed.

[taldcroft]

@jzuhone - I pushed a commit to fix the ruff issues to be sure to pull that commit. It looks like a new version of ruff 0.2.1 is out that forces some changes. There is a process here about whether to fix unrelated ruff issues in a separate PR, but overall I'm just going for expediency since we require CI to pass for merging.

[taldcroft]

@jzuhone - do you have pre-commit installed on this? From within the repo:

pre-commit install

That should prevent committing locally unless it is going to pass CI on GH.

[taldcroft]

And a ruff format . at the command line should fix up the issues, along with installing the ruff extension in VS code and making it your linter and formatter.

[taldcroft]

Just needs unit test(s) now.

[jzuhone]

@taldcroft this is ready to go.

[taldcroft]

@jzuhone - I've confirmed the tests pass and that a new test exists but the details of the new code and test are beyond me. If this hasn't already been reviewed by someone else in ACIS that might be a good thing.

[jzuhone]

I agree--good call. @cegrant can you check my work?

[jzuhone]

Thanks @cegrant for your comments--I added some comments in the code about the existence of other SIMODEs that may be used in special circumstances, and I ended up deciding for this reason that if we happen upon one of those that we should just return "undef", since these are not likely to occur in regular loads and the main need for getting this state correct at the moment is to help determine the start and stop times of science data quality limits for the FP thermal model. So returning "undef" would be acceptable under these circumstances.

@taldcroft I think that with Catherine's approving review this is ready to go.