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

Allows initialization without Reset permission #673

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

kaczmarczyck
Copy link
Collaborator

This PR is useful for all implementations that can trigger a reboot without user intervention. In these cases, we don't want to allow the Reset command. It should only be allowed after a user initiated power cycle.

Adds tests to the new functionality and a few other coverage holes.

This PR is useful for all implementations that can trigger a reboot
without user intervention. In these cases, we don't want to allow the
Reset command. It should only be allowed after a user initiated power
cycle.

Adds tests to the new functionality and a few other coverage holes.
@kaczmarczyck kaczmarczyck requested a review from ia0 January 9, 2024 15:13
ia0
ia0 previously approved these changes Jan 9, 2024
Copy link
Member

@ia0 ia0 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, but why is soft reset an input to Ctap::new? Could there be an API function that returns whether there was a soft reset? This way Ctap::new doesn't take arguments but instead calls into that API function. I'm afraid that we would start adding too many inputs to Ctap::new, making all call-sites more complicated.

@kaczmarczyck
Copy link
Collaborator Author

Makes sense, changed to be part of Env.

@kaczmarczyck kaczmarczyck merged commit ba0d717 into google:develop Jan 9, 2024
9 checks passed
@kaczmarczyck kaczmarczyck deleted the soft-reset branch January 9, 2024 17:30
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