-
Notifications
You must be signed in to change notification settings - Fork 120
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
Synchronize cpp examples with python #110
Conversation
Thanks for making these. These will be great to have! Will be so nice to have full Python and C++ examples for folks. |
Oh and once these are done @csaba-luxonis - we should do the switcher on our docs so folks can switch between the Python code sample and the C++ code sample on the same page. |
Yeah, that would be amazing. |
I think we should do this in other direction: Documentation should mark examples (code samples) with numbers but just as an easier overview |
Oh, yes. Sorry I didn't meant to say that we should have the numbers on these. I agree we should remove the numbers from the Python examples. And just have the documentation designate the order of the code samples. |
I will remove the example numbers once im done with all |
Also one thing I was thinking, if we allow passing the output file as parameter (as well as the default), to be able to set it to |
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.
Throughout examples: use ImgFrame::Type alias instead of Raw*
manip->initialConfig.setFrameType(dai::ImgFrame::Type::BGR888p);
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.
LGTM apart from:
Style - either same line or with curly braces (in a couple of cases)
if(syncNN) {
...
} else {
...
}
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.
LGTM
This PR is WIP, will be updated once it's ready to review.