Skip to content

Save and load the system#2

Merged
PamplemousseMR merged 13 commits into
devfrom
feat/Load_Save
May 18, 2018
Merged

Save and load the system#2
PamplemousseMR merged 13 commits into
devfrom
feat/Load_Save

Conversation

@PamplemousseMR

@PamplemousseMR PamplemousseMR commented Apr 6, 2018

Copy link
Copy Markdown
  • Changed the threading system :
    • Added std::join on threads insteed of "sleeps until the thread is finished"
    • Change the "stop" system by adding a wait condition instead of "sleep"
    • Change the "reset" system by adding a wait condition instead of "sleep"
    • Change the "finish" system by adding a wait condition instead of "sleep"
  • Added some destructors to free the memory
  • Added a public save function

Comment thread src/System.cc Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the code is cleaner with your changes, but you manage the case mpLoopCloser->isRunningGBA() ?

@PamplemousseMR PamplemousseMR Apr 23, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

mpLoopCloser->isRunningGBA() is here to wait a thread create by mpLoopCloser (named mpThreadGBA), this thread execute RunGlobalBundleAdjustment.
The pipeline work like that: The main loop of mpLoopCloser call CorrectLoop in each iteration, then CorrectLoop check if mpThreadGBA is running (by calling isRunningGBA()) , and if it's true, mpThreadGBA is detached in order to create another thread with the same function (RunGlobalBundleAdjustment).
I've added a join on this thread in the main loop when it's finished, to wait in mpLoopCloser instead of wait in the system.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok 👍 🍝

Comment thread src/System.cc Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why no test with nullptr on this pointer?

@dweckmann dweckmann Apr 16, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

well, the correct code is delete mptViewer; as delete NULL; or delete nullptr; is not going to break. delete test for nullptr by itself, no need to add boiler code ,-)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread Examples/Monocular/mono_tum.cc Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, but usage message must also be changed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread include/System.h Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

#pragma once or #ifndef SYSTEM_H you have to choose 😄

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

@greenjava

Copy link
Copy Markdown
Member

The description of the pull-request is really very light 😛

@schweitzer schweitzer requested a review from greenjava April 16, 2018 10:10
@PamplemousseMR PamplemousseMR force-pushed the feat/Load_Save branch 4 times, most recently from ba0734d to 96503d5 Compare April 23, 2018 14:33
@PamplemousseMR PamplemousseMR changed the title Save and load the system WIP: Save and load the system Apr 24, 2018
@PamplemousseMR PamplemousseMR force-pushed the feat/Load_Save branch 3 times, most recently from 5149723 to f89c44e Compare April 24, 2018 14:11
@PamplemousseMR PamplemousseMR changed the title WIP: Save and load the system Save and load the system Apr 25, 2018
@PamplemousseMR PamplemousseMR changed the title Save and load the system WIP: Save and load the system May 2, 2018
@PamplemousseMR PamplemousseMR changed the title WIP: Save and load the system Save and load the system May 16, 2018
@PamplemousseMR PamplemousseMR requested review from bfahrer, dweckmann, greenjava and schweitzer and removed request for bfahrer, dweckmann, greenjava and schweitzer May 17, 2018 09:06

@dweckmann dweckmann left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the if(pointer != nullptr)delete pointer; are useless but that's nitpicking...

Comment thread src/Tracking.cc Outdated
int fIniThFAST = orbParams.m_iniThFAST;
int fMinThFAST = orbParams.m_minThFAST;

if(mpORBextractorLeft != nullptr) delete mpORBextractorLeft;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

testing nullptr is not needed delete operator take care of that

@PamplemousseMR PamplemousseMR removed the request for review from bfahrer May 18, 2018 09:57
@PamplemousseMR PamplemousseMR merged commit ce92f2c into dev May 18, 2018
@PamplemousseMR PamplemousseMR deleted the feat/Load_Save branch May 18, 2018 10:00
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.

4 participants