Skip to content

HID refactoring#436

Merged
kaczmarczyck merged 4 commits intogoogle:developfrom
kaczmarczyck:hid-refactoring
Mar 7, 2022
Merged

HID refactoring#436
kaczmarczyck merged 4 commits intogoogle:developfrom
kaczmarczyck:hid-refactoring

Conversation

@kaczmarczyck
Copy link
Collaborator

This PR has 2 commits. The first

  • introduces the CtapHidCommand enum
  • splits off logic to parse Messages with a new process_message that contains core logic
  • fixes the documentation

After this bigger cleanup, the second commit actually works on the goal of this PR: To have an API that separates all CTAP logic, and is the first step on the road to a pure HID library. It does so without changing the API. The plan is to move process_hid_packet (which is now very short) out of the library, and have two instances with different process_message functions later. This will also allow removing the wink_permission eventually.

If you rather review in 2 PRs, I'll split the second commit to a new PR (or I could also split commit 1 if necessary). In this case, consider this the overview.

Plan for future submissions is:

  • Refactor the tests in hid/mod.rs, the new interface is much more test friendly.
  • Do the aforementioned move to make this a pure library (possible multiple steps actually).
  • Introduce the second USB transport.

@kaczmarczyck kaczmarczyck requested a review from ia0 March 4, 2022 17:18
@kaczmarczyck kaczmarczyck self-assigned this Mar 4, 2022
ia0
ia0 previously approved these changes Mar 7, 2022
@kaczmarczyck kaczmarczyck requested a review from ia0 March 7, 2022 12:22
ia0
ia0 previously approved these changes Mar 7, 2022
@kaczmarczyck
Copy link
Collaborator Author

The fuzzing check passes locally, therefore I'll submit and fix that separately.

@ia0
Copy link
Member

ia0 commented Mar 7, 2022

The fuzzing check passes locally, therefore I'll submit and fix that separately.

Note also that it's the CIFuzz job that's failing, not the "Cargo fuzz build" one, so it's working locally even for GitHub. It's the docker setup the issue.

@kaczmarczyck kaczmarczyck merged commit 7c1ddcd into google:develop Mar 7, 2022
@kaczmarczyck kaczmarczyck deleted the hid-refactoring branch March 7, 2022 14:19
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