Fix continuity issue related to transitions beyond continuity time

[taldcroft]

This is a critical bug fix that should be released ASAP.

This adds explicit handling in continuity for the list of transitions that occur after the stop date but are needed for continuity. The classic example is for maneuvers if starting in the middle of a maneuver, but this also occurs for SPMEnableTransition which puts commands into the future.

The flow is as follows. It might seem a bit circular at first but is not:

• get_states() collects all such "future" transitions and puts them into states.meta['continuity_transitions'].
  • For a call to get_states() that has no "future" or "leftover" transitions, this is an empty list.
  • Conversely, get_states() looks for a __transitions__ key in the supplied continuity, and if it exists uses that to initialize the transitions dict that gets assembled in dynamic state processing.
• get_continuity() collects any "future" transitions from its internal calls to get_states() into a list and stores them in the __transitions__ key in the continuity dict.
  • When get_continuity() calls get_states(), it always supplies continuity={} so that there is no circularity.

Fixes #125
Fixes #127

[jeanconn]

@taldcroft I think I'm following this (what the problem was and how you've addressed it), but if you have time, some more text in the PR description would be helpful. Thanks!

[taldcroft]

@jeanconn - that's an excellent request. I have updated the description.

[taldcroft]

BTW I am happy that the infrastructure in kadi commands states lends itself to this solution which is completely general for any possible dynamic states. In particular there is no need for any kind of ad-hoc solutions like backing up to the previous NPM, which is not enough here because you have other states (sun_pos_mon) for which there would be a different ad-hoc rule.

[taldcroft]

Since it is so tiny I tossed in a fix for #127 here.

[jeanconn]

I note that this is the first fix we've had in a while that will go into both ska2 and ska3.

[taldcroft]

Could this be a driver to finally:

• Decommission the sybase version of commanded states
• Decertify Ska2 for kadi commanded states. We could ask if anyone is using it in Ska2, but the answer should probably be no. Larry might be, but he clearly doesn't need the bug fix.
• Move validate_states to Ska3

This is all infrastructure and might take more time than just installing [1], but you were saying you wanted to spend some effort on infrastructure now that sparkles/proseco is wrapping up.

[1] - though I wonder when you count the GRETA network.

[jeanconn]

Regarding the bullets in #126 (comment)

Do you mean the fact that we still need to install this in ska2 is a driver to do the rest? I somewhat see where you are going, but think it is easier to just install kadi 3.16.4 in ska2 now and deal with the rest as it comes. You've noted the validate_states update that would also be required for the Ska3 transition. Meaning I do want to fix some things that are broken in infrastructure but still think we have higher-priority elements than this.

(And I had a starcheck concern related to the fact that we're still using sybase star histories... not because the star histories are in sybase but because I thought the code that updates the star histories was using sybase cmd_states, but it looks like it isn't using cmds or cmd_states at this time).

And I don't know what MTA is using.

Do we need to set /proj/sot/ska/bin/get_cmd_states (ska2) to warn about sybase being deprecated? Is there a get_cmd_states somewhere for ska3?

And I am also not clear about which version Larry is using.

[jeanconn]

This comment would be less confusing to me if it mentioned that these transitions are all from the commands (cmds) still in the continuity ranges (all before stop).

stop is also a confusing name for me here (date -> stop and really it is just the reference time for continuity, not a start -> stop range, but that's neither here nor there).

[jeanconn]

When get_continuity() calls get_states(), it always supplies continuity={} so that there is no circularity.

It still feels circular, or at least bootstrap-py, but makes sense, is simple overall, and seems to work. The added tests also look good, but I haven't spent enough time in here to comment much on how sufficient the coverage really is.