-
Notifications
You must be signed in to change notification settings - Fork 243
Remove many extra conversions from Matrix3x3 to Quaternion #741
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
Changes from 10 commits
246fa84
e3ccee1
e791331
51f75b4
32660ca
46d66fc
0361624
a979245
7e0f4b3
dc2f979
0030dfb
d524ed8
8bd813a
77c8010
b399f8f
bc5db14
be1ba27
86c6181
bb368e5
a2e08e6
5d33825
edb14fe
7c93938
ca99cf9
095195c
eaa507b
6774cb6
2e13e45
9193566
66d5d81
06c5368
bdbbf81
e117aa1
f9f7cea
c0f2108
71e0b94
ee7008f
4b4e499
546dc0e
ac1b450
c0767e0
acefb5c
b24d3dc
61f8228
21883f5
47a5eda
9d145fb
1ecb27e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -182,27 +182,27 @@ bool BufferCore::setTransform( | |
| const geometry_msgs::msg::TransformStamped & transform, | ||
| const std::string & authority, bool is_static) | ||
| { | ||
| tf2::Transform tf2_transform(tf2::Quaternion( | ||
| tf2::Quaternion rotation( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for the next reviewer, this change keeps the rotation quat as a quat and the positions as a Vec3 instead of promoting it to a tf2::Transform, saving us a slight conversion cost.
kyle-basis marked this conversation as resolved.
Outdated
|
||
| transform.transform.rotation.x, | ||
| transform.transform.rotation.y, | ||
| transform.transform.rotation.z, | ||
| transform.transform.rotation.w), | ||
| tf2::Vector3( | ||
| transform.transform.rotation.w); | ||
| tf2::Vector3 origin( | ||
|
kyle-basis marked this conversation as resolved.
Outdated
|
||
| transform.transform.translation.x, | ||
| transform.transform.translation.y, | ||
| transform.transform.translation.z)); | ||
| transform.transform.translation.z); | ||
| TimePoint time_point(std::chrono::nanoseconds(transform.header.stamp.nanosec) + | ||
| std::chrono::duration_cast<std::chrono::nanoseconds>( | ||
| std::chrono::seconds( | ||
| transform.header.stamp.sec))); | ||
| return setTransformImpl( | ||
| tf2_transform, transform.header.frame_id, transform.child_frame_id, | ||
| origin, rotation, transform.header.frame_id, transform.child_frame_id, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to next reviewer: now we just called the new overloaded function. |
||
| time_point, authority, is_static); | ||
| } | ||
|
|
||
| bool BufferCore::setTransformImpl( | ||
| const tf2::Transform & transform_in, const std::string frame_id, | ||
| const std::string child_frame_id, const TimePoint stamp, | ||
| const tf2::Vector3 & origin_in, const tf2::Quaternion & rotation_in, const std::string & frame_id, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're doing some heavy construction here we would wise to add some documentation for the next person who has to work on this code. |
||
| const std::string & child_frame_id, const TimePoint stamp, | ||
| const std::string & authority, bool is_static) | ||
| { | ||
| std::string stripped_frame_id = stripSlash(frame_id); | ||
|
|
@@ -231,36 +231,36 @@ bool BufferCore::setTransformImpl( | |
| error_exists = true; | ||
| } | ||
|
|
||
| if (std::isnan(transform_in.getOrigin().x()) || std::isnan(transform_in.getOrigin().y()) || | ||
| std::isnan(transform_in.getOrigin().z()) || | ||
| std::isnan(transform_in.getRotation().x()) || std::isnan(transform_in.getRotation().y()) || | ||
| std::isnan(transform_in.getRotation().z()) || std::isnan(transform_in.getRotation().w())) | ||
| if (std::isnan(origin_in.x()) || std::isnan(origin_in.y()) || | ||
| std::isnan(origin_in.z()) || | ||
| std::isnan(rotation_in.x()) || std::isnan(rotation_in.y()) || | ||
| std::isnan(rotation_in.z()) || std::isnan(rotation_in.w())) | ||
| { | ||
| RCUTILS_LOG_ERROR( | ||
| "TF_NAN_INPUT: Ignoring transform for child_frame_id \"%s\" from authority \"%s\" because" | ||
| " of a nan value in the transform (%f %f %f) (%f %f %f %f)", | ||
| stripped_child_frame_id.c_str(), authority.c_str(), | ||
| transform_in.getOrigin().x(), transform_in.getOrigin().y(), transform_in.getOrigin().z(), | ||
| transform_in.getRotation().x(), transform_in.getRotation().y(), | ||
| transform_in.getRotation().z(), transform_in.getRotation().w() | ||
| origin_in.x(), origin_in.y(), origin_in.z(), | ||
|
kscottz marked this conversation as resolved.
|
||
| rotation_in.x(), rotation_in.y(), | ||
| rotation_in.z(), rotation_in.w() | ||
| ); | ||
| error_exists = true; | ||
| } | ||
|
|
||
| bool valid = std::abs( | ||
| (transform_in.getRotation().w() * transform_in.getRotation().w() + | ||
| transform_in.getRotation().x() * transform_in.getRotation().x() + | ||
| transform_in.getRotation().y() * transform_in.getRotation().y() + | ||
| transform_in.getRotation().z() * transform_in.getRotation().z()) - 1.0f) < | ||
| const bool valid = std::abs( | ||
| (rotation_in.w() * rotation_in.w() + | ||
| rotation_in.x() * rotation_in.x() + | ||
| rotation_in.y() * rotation_in.y() + | ||
| rotation_in.z() * rotation_in.z()) - 1.0f) < | ||
| QUATERNION_NORMALIZATION_TOLERANCE; | ||
|
|
||
| if (!valid) { | ||
| RCUTILS_LOG_ERROR( | ||
| "TF_DENORMALIZED_QUATERNION: Ignoring transform for child_frame_id \"%s\" from authority" | ||
| " \"%s\" because of an invalid quaternion in the transform (%f %f %f %f)", | ||
| stripped_child_frame_id.c_str(), authority.c_str(), | ||
| transform_in.getRotation().x(), transform_in.getRotation().y(), | ||
| transform_in.getRotation().z(), transform_in.getRotation().w()); | ||
| rotation_in.x(), rotation_in.y(), | ||
| rotation_in.z(), rotation_in.w()); | ||
| error_exists = true; | ||
| } | ||
|
|
||
|
|
@@ -287,8 +287,8 @@ bool BufferCore::setTransformImpl( | |
|
|
||
| if (frame->insertData( | ||
| TransformStorage( | ||
| stamp, transform_in.getRotation(), | ||
| transform_in.getOrigin(), lookupOrInsertFrameNumber(stripped_frame_id), frame_number))) | ||
| stamp, rotation_in, | ||
| origin_in, lookupOrInsertFrameNumber(stripped_frame_id), frame_number))) | ||
| { | ||
| frame_authority_[frame_number] = authority; | ||
| } else { | ||
|
|
@@ -700,15 +700,17 @@ BufferCore::lookupTransform( | |
| { | ||
| tf2::Transform transform; | ||
|
kyle-basis marked this conversation as resolved.
Outdated
|
||
| TimePoint time_out; | ||
| lookupTransformImpl(target_frame, source_frame, time, transform, time_out); | ||
| tf2::Vector3 origin; | ||
| tf2::Quaternion rotation; | ||
| lookupTransformImpl(target_frame, source_frame, time, origin, rotation, time_out); | ||
| geometry_msgs::msg::TransformStamped msg; | ||
| msg.transform.translation.x = transform.getOrigin().x(); | ||
| msg.transform.translation.y = transform.getOrigin().y(); | ||
| msg.transform.translation.z = transform.getOrigin().z(); | ||
| msg.transform.rotation.x = transform.getRotation().x(); | ||
| msg.transform.rotation.y = transform.getRotation().y(); | ||
| msg.transform.rotation.z = transform.getRotation().z(); | ||
| msg.transform.rotation.w = transform.getRotation().w(); | ||
| msg.transform.translation.x = origin.x(); | ||
| msg.transform.translation.y = origin.y(); | ||
| msg.transform.translation.z = origin.z(); | ||
| msg.transform.rotation.x = rotation.x(); | ||
| msg.transform.rotation.y = rotation.y(); | ||
| msg.transform.rotation.z = rotation.z(); | ||
| msg.transform.rotation.w = rotation.w(); | ||
| std::chrono::nanoseconds ns = std::chrono::duration_cast<std::chrono::nanoseconds>( | ||
| time_out.time_since_epoch()); | ||
| std::chrono::seconds s = std::chrono::duration_cast<std::chrono::seconds>( | ||
|
|
@@ -736,10 +738,11 @@ BufferCore::lookupTransform( | |
| msg.transform.translation.x = transform.getOrigin().x(); | ||
| msg.transform.translation.y = transform.getOrigin().y(); | ||
| msg.transform.translation.z = transform.getOrigin().z(); | ||
| msg.transform.rotation.x = transform.getRotation().x(); | ||
| msg.transform.rotation.y = transform.getRotation().y(); | ||
| msg.transform.rotation.z = transform.getRotation().z(); | ||
| msg.transform.rotation.w = transform.getRotation().w(); | ||
| tf2::Quaternion rotation = transform.getRotation(); | ||
|
kyle-basis marked this conversation as resolved.
|
||
| msg.transform.rotation.x = rotation.x(); | ||
| msg.transform.rotation.y = rotation.y(); | ||
| msg.transform.rotation.z = rotation.z(); | ||
| msg.transform.rotation.w = rotation.w(); | ||
| std::chrono::nanoseconds ns = std::chrono::duration_cast<std::chrono::nanoseconds>( | ||
| time_out.time_since_epoch()); | ||
| std::chrono::seconds s = std::chrono::duration_cast<std::chrono::seconds>( | ||
|
|
@@ -755,13 +758,25 @@ BufferCore::lookupTransform( | |
| void BufferCore::lookupTransformImpl( | ||
|
kyle-basis marked this conversation as resolved.
|
||
| const std::string & target_frame, | ||
| const std::string & source_frame, | ||
| const TimePoint & time, tf2::Transform & transform, | ||
| const TimePoint & time, tf2::Transform & transform_out, | ||
| TimePoint & time_out) const | ||
| { | ||
| tf2::Quaternion rotation; | ||
| lookupTransformImpl(target_frame, source_frame, time, transform_out.getOrigin(), rotation, time_out); | ||
|
kyle-basis marked this conversation as resolved.
Outdated
|
||
| transform_out.setRotation(rotation); | ||
| } | ||
|
|
||
| void BufferCore::lookupTransformImpl( | ||
|
kyle-basis marked this conversation as resolved.
|
||
| const std::string & target_frame, | ||
| const std::string & source_frame, | ||
| const TimePoint & time, tf2::Vector3 & origin_out, tf2::Quaternion & rotation_out, | ||
| TimePoint & time_out) const | ||
| { | ||
| std::unique_lock<std::mutex> lock(frame_mutex_); | ||
|
|
||
| if (target_frame == source_frame) { | ||
| transform.setIdentity(); | ||
| rotation_out = Quaternion::getIdentity(); | ||
| origin_out.setValue(tf2Scalar(0.0), tf2Scalar(0.0), tf2Scalar(0.0)); | ||
|
|
||
| if (time == TimePointZero) { | ||
| CompactFrameID target_id = lookupFrameNumber(target_frame); | ||
|
|
@@ -805,8 +820,8 @@ void BufferCore::lookupTransformImpl( | |
| } | ||
|
|
||
| time_out = accum.time; | ||
| transform.setOrigin(accum.result_vec); | ||
| transform.setRotation(accum.result_quat); | ||
| origin_out = accum.result_vec; | ||
| rotation_out = accum.result_quat; | ||
| } | ||
|
|
||
| void BufferCore::lookupTransformImpl( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.