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

Change time threshold for auto-enable of SPM following eclipse #323

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Feb 16, 2024

Description

The matching time threshold of 125 seconds between battery connect time and eclipse entry appears to be too low. Eclipses on 2024:043 and 2024:046 had a delta of 129 to 130 seconds. This results in the sun_pos_mon remaining disabled coming out of eclipse.

This PR changes that value to 135 sec. This has been confirmed as an acceptable value with FOT eng. See the email thread "Criteria for SPM auto-enable following eclipse" around 2024-Feb-17 for details.

Interface impacts

None

Testing

Unit tests

  • Mac
(ska3) ➜  kadi git:(spm-eclipse-reenable-fix) git rev-parse HEAD                         
1ed12413410dbd49cf1adcf29ced7d5377aa44f1
(ska3) ➜  kadi git:(spm-eclipse-reenable-fix) 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 233 items                                                                                                                     

kadi/commands/tests/test_commands.py .......................................................................................      [ 37%]
kadi/commands/tests/test_states.py .......................x..............................................x....................... [ 77%]
                                                                                                                                  [ 77%]
kadi/commands/tests/test_validate.py ....................                                                                         [ 86%]
kadi/tests/test_events.py ..........                                                                                              [ 90%]
kadi/tests/test_occweb.py ......................                                                                                  [100%]

Independent check of unit tests by Jean

  • Linux ska3-flight
ska3-jeanconn-fido> git rev-parse HEAD
f37a897bdf0d8906766f5e8308cd9ce00b2fe0e1
ska3-jeanconn-fido> pytest
============================= test session starts ==============================
platform linux -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /proj/sot/ska/jeanproj/git, configfile: pytest.ini
plugins: anyio-3.6.2, timeout-2.1.0
collected 233 items                                                            

kadi/commands/tests/test_commands.py ................................... [ 15%]
....................................................                     [ 37%]
kadi/commands/tests/test_states.py .......................x............. [ 53%]
.................................x.......................                [ 77%]
kadi/commands/tests/test_validate.py ....................                [ 86%]
kadi/tests/test_events.py ..........                                     [ 90%]
kadi/tests/test_occweb.py ......................                         [100%]

=============================== warnings summary

Functional tests

For about 30 seconds I changed the status of the two COMMAND_SW | TLMSID=AOFUNCEN, AOPCADSE=30 commands on day 43 and 46 to be In-work. With that in place I ran kadi validate states locally and confirmed that the sun_pos_mon state matches telemetry. This was done in the git repo checked out at 1ed1241.

@taldcroft taldcroft force-pushed the spm-eclipse-reenable-fix branch from 5194671 to 1ed1241 Compare February 17, 2024 14:23
@taldcroft taldcroft changed the title WIP: Change time threshold for auto-enable of SPM following eclipse Change time threshold for auto-enable of SPM following eclipse Feb 17, 2024
@taldcroft taldcroft requested a review from jeanconn February 17, 2024 14:31
@jeanconn
Copy link
Contributor

From the recent data, it seems fine to set spm eclipse enable using this 135 sec check, as, indeed, spm was enabled. However, these data don't tell me about the cases where we want kadi to correctly not enable spm because the time from eclipse entry back to battery connect was longer. Has that happened at 136 seconds or ?

@taldcroft
Copy link
Member Author

I'm hoping to avoid spending too much more time on this (i.e. digging through command history and telemetry to answer your question). The FOT engineers have stated that 135 sec is a good value so I'm accepting their recommendation face value.

@jeanconn
Copy link
Contributor

Sure, I basically just didn't want to see this break just as many times the other way. If the engineering input is that is unlikely, that's fine.

kadi/commands/states.py Outdated Show resolved Hide resolved
@taldcroft taldcroft merged commit 5b70b04 into master Feb 17, 2024
4 checks passed
@taldcroft taldcroft deleted the spm-eclipse-reenable-fix branch February 17, 2024 18:45
This was referenced Mar 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