sot / kadi

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

Fix continuity issue related to transitions beyond continuity time #126

Closed taldcroft closed 5 years ago

taldcroft commented 5 years ago

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:

Fixes #125 Fixes #127

jeanconn commented 5 years ago

@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 commented 5 years ago

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

taldcroft commented 5 years ago

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 commented 5 years ago

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

jeanconn commented 5 years ago

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

taldcroft commented 5 years ago

Could this be a driver to finally:

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 commented 5 years ago

Regarding the bullets in https://github.com/sot/kadi/pull/126#issuecomment-469373421

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.