Qrreader changes #371
Qrreader changes #371
Conversation
2. Added 4 props in QR Reader module - tryHarder - saveQRImages - qrImagesPath - frameRotationCounter 3. Added loop test for QR module
There was a problem hiding this comment.
I know this PR was going to have QRReader changed...
But I also see Logger changes...
What is the source of those changes ? have we updated the Logger from main branch ?
or are these changes needed on Main as well ?
Why is the target for PR the main branch ??
it should be tragetted to "v1.0_br" right ?
base/src/QRReader.cpp
Outdated
| mFrameRotationCounter = _props.frameRotationCounter; | ||
| if(mFrameRotationCounter <= 0) | ||
| { | ||
| mFrameRotationCounter=1; |
There was a problem hiding this comment.
There should be a warning log here... the caller should not be setting a 0/negative and if we silently set it to 1 there is "hidden magic" happening. here.
There was a problem hiding this comment.
I have added warning log when user sets frameRotationCounter less than 1
base/src/QRReader.cpp
Outdated
| std::ofstream outFile(savePath.string(), std::ios::binary); | ||
| if (outFile) | ||
| { | ||
| outFile.write(static_cast<char*>(frame->data()), frame->size()); | ||
| outFile.close(); | ||
| } | ||
| else | ||
| { | ||
| LOG_ERROR << "Failed to save frame to " << savePath.string(); | ||
| } |
There was a problem hiding this comment.
this block is poorly indented... and it confuses the reviewer. In general just using the built in code formatter in VScode/VS is enough here
There was a problem hiding this comment.
also if the savePath is not a good directory (eg non existing)... we will be gettting an std::exception thrown... which is not handled here
There was a problem hiding this comment.
Now I have indented properly and also added trycatch block to handle exception
3 participants
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #[Issue]
Description
Precise description of the changes in this pull request
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Type Choose one: (Bug fix | Feature | Documentation | Testing | Other)
Screenshots (if applicable)
Checklist