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

Add update_pose() API call #79

Merged
merged 6 commits into from
May 29, 2021

Conversation

AlexeyMerzlyakov
Copy link
Contributor

Adding new system.h/system.cc API call for updating camera position by known pose. This is a part required for stella-cv/stella_vslam_ros#11 ticket to add the support of the /initialpose intentionally set by developer.

@AlexeyMerzlyakov
Copy link
Contributor Author

AlexeyMerzlyakov commented May 5, 2021

The solution has been verified by calling update_pose() for a known pose(s) in parallel thread. That was checked on a EuRoC dataset in localization mode (using previously pre-build by run_euroc_slam map).

The verification patch looks like follows:

diff --git a/example/run_euroc_localization.cc b/example/run_euroc_localization.cc
index 89be0b3..41716f0 100644
--- a/example/run_euroc_localization.cc
+++ b/example/run_euroc_localization.cc
@@ -122,6 +133,18 @@ void mono_localization(const std::shared_ptr<openvslam::config>& cfg,
 #endif
     });
 
+    std::thread pose_thread([&]() {
+        while (true) {
+            Eigen::Matrix4d pose;
+            pose << 1, 0, 0, 0.0,
+                    0, 1, 0, 0.0,
+                    0, 0, 1, 0.0,
+                    0, 0, 0, 1;
+            SLAM.update_pose(pose);
+            std::this_thread::sleep_for(std::chrono::milliseconds(10));
+       }
+    });
+
     // run the viewer in the current thread
 #ifdef USE_PANGOLIN_VIEWER
     viewer.run();
@@ -130,6 +153,7 @@ void mono_localization(const std::shared_ptr<openvslam::config>& cfg,
 #endif
 
     thread.join();
+    pose_thread.join();
 
     // shutdown the SLAM process
     SLAM.shutdown();

There are two cases were checked:

  • Pose (0.0, 0.0, 0.0) (belonging to track) gives correct localization on the initial part of dataset, as expected.
  • Pose (1.0, 2.0, 3.0) (does not belong to track) gives Lost state, camera is being intentionally placed to a requested position until robot will be localized later:
    Screenshot_2021-04-30_15-07-40

@ymd-stella
Copy link
Contributor

@AlexeyMerzlyakov Is this a draft?

@AlexeyMerzlyakov
Copy link
Contributor Author

@AlexeyMerzlyakov Is this a draft?

There are some changes might be applied later, when openvslam_ros part to be added. But in general, the approach is fixed until be reviewed. I would like appreciate if you could leave a feedback on main approach and its details.

src/openvslam/data/map_database.cc Outdated Show resolved Hide resolved
src/openvslam/data/map_database.cc Outdated Show resolved Hide resolved
src/openvslam/data/map_database.cc Outdated Show resolved Hide resolved
src/openvslam/system.cc Outdated Show resolved Hide resolved
src/openvslam/system.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/data/map_database.cc Show resolved Hide resolved
@AlexeyMerzlyakov
Copy link
Contributor Author

@ymd-stella , @SteveMacenski Thank you for attentive review! Most of the comments I agree with and will make a next patch meeting the review items in next update.

@AlexeyMerzlyakov AlexeyMerzlyakov changed the title [WIP] Add update_pose() API call Add update_pose() API call May 20, 2021
@AlexeyMerzlyakov
Copy link
Contributor Author

Made a force-push update (there were code refactoring conflicts) with review items met.

Copy link
Contributor

@ymd-stella ymd-stella left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!
I have written some comments, please have a look.

src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/system.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.h Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
@AlexeyMerzlyakov
Copy link
Contributor Author

Okay, I've made next push. There were met all second review items & applied clang-format on changed files.

Copy link
Contributor

@ymd-stella ymd-stella left a comment

Choose a reason for hiding this comment

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

It's getting very good. Thank you for your contribution. There are still a few things that annoy me, so let me discuss them.

src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
src/openvslam/tracking_module.cc Show resolved Hide resolved
src/openvslam/tracking_module.cc Outdated Show resolved Hide resolved
@ymd-stella ymd-stella merged commit 10b23ae into stella-cv:main May 29, 2021
@ymd-stella ymd-stella mentioned this pull request Aug 6, 2021
4 tasks
Arthur-CWW pushed a commit to Arthur-CWW/stella_vslam_melodic that referenced this pull request Mar 18, 2024
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.

3 participants