-
Notifications
You must be signed in to change notification settings - Fork 6
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
Full Mission Case #822
base: master
Are you sure you want to change the base?
Full Mission Case #822
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, just some minor changes to get rid of code that isn't necessary
ptest/cases/full_mission_case.py
Outdated
firstDetumble = True | ||
firstStandby = True | ||
firstFarField = True | ||
firstNearField = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We dont need these booleans since these were for the startup/safehold case where we would go through startup->standby twice. This case should only go through startup->near field once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so it only prints "follower and leader are in _____" the first time and doesn't spam that out every iteration of run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL wait im dumb i literally wrote this code in a diff case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! :) I think we should hitl test it today if possible and then go ahead and merge
This case looks good. Can you just remove the commented-out commands, and upload the logs of this test case passing here? Make sure to rebase as well. |
Actually sorry in addition to this, can we add additional asserts in the post-boot section to make sure we're booting with the radios in the disabled state? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert that radio is disabled during startup, clean up commented out commands, upload any logs if they exist of the older HITL tests if you have them.
Lastly, this test should definitely be added to the github workflow! :D
How long does this test take to run? If it takes a while (as it probably should), wouldn't it be a bad idea to add to CI? |
the case runs pretty quick without skipping the deployment wait. I think it will be ok to add to CI given we commit the version that skips the wait, but when actually running the case changing skip_deployment_wait to false |
We can merge this PR if you'd like @Duncan-McD |
Full Mission Case
Fixes #811
Summary of changes
Testing
Ran in hootl (with deployment wait skipped) and it reached Near Field with no assertions being thrown.
Documentation Evidence