Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Command and Command not run events #308

Merged
merged 5 commits into from
Jan 4, 2024
Merged

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Dec 25, 2023

Description

This improves the Command and Command not run event types in the Command Events Sheet.

The documentation states that a Command or Command not run can be copy/pasted from the backstop file. In that format the parameters are comma-delimited, but the code was expecting space-delimited. In the flight Sheet we had only previously used a single parameter TLMSID=... so it never came up. The code now uses the parse_cm backstop parser and thus requires comma-delimited parameters.

This PR adds the capability (along with a number of tests) to include multiple parameters in the Command not run event to match only a single command. Previously only the date and TLMSID were actually being used to select the command(s) to ignore. For disabling HRC this turns out to select an additional unwanted command 1.

Note: this PR requires a masters environment to pass tests. (Test fails are unrelated to this PR, however).

Interface impacts

Format for Command and Command not run event Params changed to match the existing documentation (see link in the Sheet).

Testing

Unit tests

  • Mac
(razl) ➜  kadi git:(better-command-not-run) git rev-parse HEAD 
9649618e3f44b3d791d1bba5e3d3167ced1de4f4
(razl) ➜  kadi git:(better-command-not-run) pytest
======================================================== test session starts ========================================================
platform darwin -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/aldcroft/git, configfile: pytest.ini
plugins: timeout-2.1.0, anyio-3.6.2
collected 228 items                                                                                                                 

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

============================================ 226 passed, 2 xfailed in 108.50s (0:01:48) =============================================

Independent check of unit tests by Jean

  • Linux

Functional tests

➜  kadi git:(better-command-not-run) ✗ cat ~/.kadi/scenario_no_events/cmd_events.csv 
State,Date,Event,Params,Author,Reviewer,Comment

➜  kadi git:(better-command-not-run) env PYTHONPATH=$PWD KADI_SCENARIO="scenario_no_events" python utils/make_hrc_disable_events.py
0 cmd(s) found for {'tlmsid': '224PCAON'}
0 cmd(s) found for {'tlmsid': '215PCAON'}
6 cmd(s) found for {'tlmsid': 'COACTSX', 'coacts1': 134}
6 cmd(s) found for {'tlmsid': 'COENASX', 'coenas1': 89}
0 cmd(s) found for {'tlmsid': 'COENASX', 'coenas1': 90}
  State             Date              Event                       Params                     Author       Reviewer                       Comment                     
---------- --------------------- --------------- ---------------------------------------- ------------ ------------- ------------------------------------------------
Definitive 2023:349:21:35:40.736 Command not run  COMMAND_SW | TLMSID=COENASX, COENAS1=89 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:349:21:34:14.736 Command not run COMMAND_SW | TLMSID=COACTSX, COACTS1=134 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:348:17:24:07.233 Command not run  COMMAND_SW | TLMSID=COENASX, COENAS1=89 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:348:17:22:41.233 Command not run COMMAND_SW | TLMSID=COACTSX, COACTS1=134 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:348:02:39:22.863 Command not run  COMMAND_SW | TLMSID=COENASX, COENAS1=89 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:348:02:37:56.863 Command not run COMMAND_SW | TLMSID=COACTSX, COACTS1=134 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:347:04:00:30.727 Command not run  COMMAND_SW | TLMSID=COENASX, COENAS1=89 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:347:03:59:04.727 Command not run COMMAND_SW | TLMSID=COACTSX, COACTS1=134 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:346:10:52:10.084 Command not run  COMMAND_SW | TLMSID=COENASX, COENAS1=89 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:346:10:50:44.084 Command not run COMMAND_SW | TLMSID=COACTSX, COACTS1=134 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:345:12:42:16.541 Command not run  COMMAND_SW | TLMSID=COENASX, COENAS1=89 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00
Definitive 2023:345:12:40:50.541 Command not run COMMAND_SW | TLMSID=COACTSX, COACTS1=134 Tom Aldcroft Jean Connelly Not run due to F_HRC_SAFING at 2023:343:02:00:00

Writing 12 events to /Users/aldcroft/.kadi/hrc_disable/cmd_events.csv
Current flight states:
      datestart              datestop           tstart        tstop     hrc_15v hrc_24v hrc_i hrc_s trans_keys
--------------------- --------------------- ------------- ------------- ------- ------- ----- ----- ----------
2023:343:02:00:00.000 2023:345:12:40:50.541 818474469.184 818685719.725     OFF     OFF   OFF   OFF           
2023:345:12:40:50.541 2023:345:12:42:16.541 818685719.725 818685805.725      ON     OFF   OFF   OFF    hrc_15v
2023:345:12:42:16.541 2023:345:16:39:04.541 818685805.725 818700013.725      ON     OFF    ON   OFF      hrc_i
2023:345:16:39:04.541 2023:345:16:39:16.541 818700013.725 818700025.725      ON     OFF   OFF   OFF      hrc_i
2023:345:16:39:16.541 2023:346:10:50:44.084 818700025.725 818765513.268     OFF     OFF   OFF   OFF    hrc_15v
2023:346:10:50:44.084 2023:346:10:52:10.084 818765513.268 818765599.268      ON     OFF   OFF   OFF    hrc_15v
2023:346:10:52:10.084 2023:346:14:46:41.281 818765599.268 818779670.465      ON     OFF    ON   OFF      hrc_i
2023:346:14:46:41.281 2023:346:14:46:53.281 818779670.465 818779682.465      ON     OFF   OFF   OFF      hrc_i
2023:346:14:46:53.281 2023:347:03:59:04.727 818779682.465 818827213.911     OFF     OFF   OFF   OFF    hrc_15v
2023:347:03:59:04.727 2023:347:04:00:30.727 818827213.911 818827299.911      ON     OFF   OFF   OFF    hrc_15v
2023:347:04:00:30.727 2023:347:05:52:18.727 818827299.911 818834007.911      ON     OFF    ON   OFF      hrc_i
2023:347:05:52:18.727 2023:347:05:52:30.727 818834007.911 818834019.911      ON     OFF   OFF   OFF      hrc_i
2023:347:05:52:30.727 2023:348:02:37:56.863 818834019.911 818908746.047     OFF     OFF   OFF   OFF    hrc_15v
2023:348:02:37:56.863 2023:348:02:39:22.863 818908746.047 818908832.047      ON     OFF   OFF   OFF    hrc_15v
2023:348:02:39:22.863 2023:348:03:16:10.863 818908832.047 818911040.047      ON     OFF    ON   OFF      hrc_i
2023:348:03:16:10.863 2023:348:03:16:22.863 818911040.047 818911052.047      ON     OFF   OFF   OFF      hrc_i
2023:348:03:16:22.863 2023:348:17:22:41.233 818911052.047 818961830.417     OFF     OFF   OFF   OFF    hrc_15v
2023:348:17:22:41.233 2023:348:17:24:07.233 818961830.417 818961916.417      ON     OFF   OFF   OFF    hrc_15v
2023:348:17:24:07.233 2023:348:21:03:53.006 818961916.417 818975102.190      ON     OFF    ON   OFF      hrc_i
2023:348:21:03:53.006 2023:348:21:04:05.006 818975102.190 818975114.190      ON     OFF   OFF   OFF      hrc_i
2023:348:21:04:05.006 2023:349:21:34:14.736 818975114.190 819063323.920     OFF     OFF   OFF   OFF    hrc_15v
2023:349:21:34:14.736 2023:349:21:35:40.736 819063323.920 819063409.920      ON     OFF   OFF   OFF    hrc_15v
2023:349:21:35:40.736 2023:349:23:22:28.736 819063409.920 819069817.920      ON     OFF    ON   OFF      hrc_i
2023:349:23:22:28.736 2023:349:23:22:40.736 819069817.920 819069829.920      ON     OFF   OFF   OFF      hrc_i
2023:349:23:22:40.736 2024:008:06:24:35.427 819069829.920 821082344.611     OFF     OFF   OFF   OFF    hrc_15v

States with HRC disable scenario hrc_disable:
      datestart              datestop           tstart        tstop     hrc_15v hrc_24v hrc_i hrc_s trans_keys
--------------------- --------------------- ------------- ------------- ------- ------- ----- ----- ----------
2023:343:02:00:00.000 2024:008:06:24:35.427 818474469.184 821082344.611     OFF     OFF   OFF   OFF           

Footnotes

  1. This ends up being benign but still not desirable.

@taldcroft taldcroft requested a review from jeanconn December 25, 2023 11:04
@jeanconn
Copy link
Contributor

jeanconn commented Jan 2, 2024

I'm a little confused by the twiki documentation for Commands that says "Additional command parameters PARAM=VALUE are separated by space. These should be upper case."

@@ -194,6 +189,8 @@ def cmd_set_end_scs(*args, date=None):

def cmd_set_command_not_run(*args, date=None):
(cmd,) = cmd_set_command(*args, date=date)
# Save original type which gets used later in CommandTable.remove_not_run_cmds().
cmd["params"]["__type__"] = cmd["type"]
Copy link
Contributor

@jeanconn jeanconn Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine, though the name __type__ wasn't intuitive for me. But I suppose anything else would get pretty long __not_run_orig_type__ etc.

@jeanconn
Copy link
Contributor

jeanconn commented Jan 2, 2024

The inclusion of utils/make_hrc_disable_events.py isn't really explained in the description (and maybe belong in a different PR at this point?). But it is a good idea to get that script into the project even if already OBE with https://github.com/sot/kadi/pull/309/files . And the functional test makes more sense to me on the second reading.

Copy link
Contributor

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me with a lien to update the twiki "Additional command parameters PARAM=VALUE are separated by space" text (if I've understood correctly that is defunct as of this PR).

@taldcroft taldcroft force-pushed the better-command-not-run branch from bbe73f8 to 9649618 Compare January 4, 2024 10:19
@taldcroft
Copy link
Member Author

I rebased to fix the merge conflict in new tests from #309 and fixed the TWiki docs.

@taldcroft taldcroft merged commit 4253c0f into master Jan 4, 2024
4 checks passed
@taldcroft taldcroft deleted the better-command-not-run branch January 4, 2024 10:22
@javierggt javierggt mentioned this pull request Jan 11, 2024
@javierggt javierggt mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants