Pipeline fitting #3
Conversation
…to pipeline-fitting Merge progress
…mage and trying again if we fail to find a line
…mprovements to generalise the model
pipeline-line-fitting/include/pipeline_line_fitting/linedetectorPipe.hpp
Outdated
Show resolved
Hide resolved
| minTurnAngle: minimum angle difference between lines, radians (ofc) | ||
| size: what size of square the image will be resized to during preprocessing | ||
| */ | ||
| randsac = RANDSAC(n, k, t, 2, removeT, finalScorethresh, minTurnAngle); |
There was a problem hiding this comment.
Can this be made into a setter function?
There was a problem hiding this comment.
why? seems like unnecessary bloat?
There was a problem hiding this comment.
From the constructor in the ros node you call a constructor that calls another constructor. Since both have a lot of parameters it is hard to keep track of what goes where.
| float finalScorethresh; | ||
| float minTurnAngle; | ||
| int size; | ||
| cv::Mat orgImg; // Original image |
There was a problem hiding this comment.
Does image have to be a member variable? We should not have to store it here since we are always just working with the most recent frame.
There was a problem hiding this comment.
yes it does, it is used later to extract the pixel values for start/endpoints for the lines, as the other image will have been greatly altered
There was a problem hiding this comment.
Can you pass it down the functions calls?
| } | ||
|
|
||
| vector<Line> LinedetectorPipe::operator()(const cv::Mat &img, const int maxLines=3){ | ||
| orgImg = img.clone(); |
There was a problem hiding this comment.
two copies seems redundant. Can it be rewritten to not require this?
There was a problem hiding this comment.
se above. one is processed and used and one needs to be intact for later
There was a problem hiding this comment.
If one is processed and one is kept for later then you should only need one copy
| processedImg = img.clone(); | ||
| _preprocess(processedImg); | ||
|
|
||
| // Find points where img > 0 |
There was a problem hiding this comment.
can this be combined into loop block?
There was a problem hiding this comment.
this was supposed to be for line 177-191
| cout << "Found line " << i+1 << " with score: " << randsac.bestScore << endl; | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
can use bool for success/fail or use negative numbers (-1) for errors
There was a problem hiding this comment.
is this not the system that is there? a bool for sucess and fail?
There was a problem hiding this comment.
Then I would prefer to explicitly set the function return type as bool and not int
* ci: added industrial ci workflow * commented out install script * renamed opencv dep * feat: added config files for clang and ruff * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Andreas Kluge Svendsrud <[email protected]> Co-authored-by: Andreas Kluge Svendsrud <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Fitting pipelinelines