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

feat: Add support for Pico headsets face tracking #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

curoviyxru
Copy link

@curoviyxru curoviyxru commented Jan 26, 2025

Resolves #3.

Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting out the code into multiple files seems good, but there are some things I would change, which is to reuse SetParam without duplicating its code, and also the ConsumePacket functions feel a bit weird. Overall i'm not so sure about the architecture of the code but also I was not happy with the prior architecture, even though I was the author. I'm not a C# developer anymore (I have only a couple of projects literally 10 years ago) and I cannot tell you which would be good code patterns to use. I'd also be ok to merge as it is, if you say the code works as expected.

@curoviyxru
Copy link
Author

which is to reuse SetParam without duplicating its code

good idea, actually

ConsumePacket functions feel a bit weird

yeah, reading from packet is not the best idea at least, will refactor it further

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.

Can we support the surface capture of pico4pro
2 participants