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

Full Mission Case #822

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Full Mission Case #822

wants to merge 5 commits into from

Conversation

Duncan-McD
Copy link
Member

Full Mission Case

Fixes #811

Summary of changes

  • Added a full mission case that starts from deployment and then goes through each state.
  • Piksi_off is also validated post boot
  • The radio is first disabled and then set to wait in near field to verify the radio can be disabled and enabled
  • Deployment wait is not skipped/Speedup flag not on

Testing

Ran in hootl (with deployment wait skipped) and it reached Near Field with no assertions being thrown.

Documentation Evidence

  • I added some comments to designate the flow of the ptest case and indicate the reasoning

Copy link
Member

@knk38 knk38 left a 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

firstDetumble = True
firstStandby = True
firstFarField = True
firstNearField = True
Copy link
Member

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

Copy link
Member Author

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

Copy link
Member

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

ptest/cases/full_mission_case.py Outdated Show resolved Hide resolved
ptest/cases/full_mission_case.py Outdated Show resolved Hide resolved
@knk38 knk38 self-requested a review October 2, 2021 16:06
Copy link
Member

@knk38 knk38 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! :) I think we should hitl test it today if possible and then go ahead and merge

@Duncan-McD Duncan-McD requested a review from knk38 October 2, 2021 17:33
@shihaocao
Copy link
Collaborator

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.

@shihaocao shihaocao self-requested a review October 7, 2021 05:24
@shihaocao
Copy link
Collaborator

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?

Copy link
Collaborator

@shihaocao shihaocao left a 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

@tanishqaggarwal
Copy link
Member

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?

@knk38
Copy link
Member

knk38 commented Oct 16, 2021

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

@shihaocao
Copy link
Collaborator

We can merge this PR if you'd like @Duncan-McD

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.

Full Duration Checkout Case
4 participants