Skip to content

Add a YAML Target pose generator#89

Open
nikhil-sethi wants to merge 2 commits intoros-industrial:masterfrom
sam-xl:add/yaml_target_pose_gen
Open

Add a YAML Target pose generator#89
nikhil-sethi wants to merge 2 commits intoros-industrial:masterfrom
sam-xl:add/yaml_target_pose_gen

Conversation

@nikhil-sethi
Copy link
Copy Markdown

Description

Based on the discussion here, this PR adds another target pose generator that can read YAML files with pose values directly without relying on a point cloud (.pcd) file.

A good use-case would be to save the toolpaths generated from the noether package as a YAML file and then evaluate their reachability offline.

Changes

yaml_target_pose_generator.h: Interface file for the pose generator
yaml_target_pose_generator.cpp: Source code for the generator. Important assumption is that the z-axis of the quaternions defined in the YAML file will be directly used as the target normal vector.
test_target_poses.yaml: A test yaml file which is parsed by the pose generator.

Test

A test poses file is given under the test folder. Comment out the current target pose generator and uncomment the YAMLTargetPoseGenerator along with the poses file. Then run colcon test to test the plugin.

Copy link
Copy Markdown
Member

@marip8 marip8 left a comment

Choose a reason for hiding this comment

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

Looks good overall, and I like having an alternative to the point cloud approach. I just have a few suggestions to address before merging

Comment on lines +5 to +6
#include <pcl/io/pcd_io.h>
#include <pcl/PCLPointField.h>
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.

PCL includes shouldn't be necessary if we're not dealing with point clouds

Suggested change
#include <pcl/io/pcd_io.h>
#include <pcl/PCLPointField.h>

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.

My bad. Copy paste :p

Comment on lines +10 to +27
static VectorIsometry3d parseYAML(YAML::Node pose_file)
{
VectorIsometry3d target_poses;
for (const auto& pose : pose_file["poses"])
{
auto position = pose["position"];
auto orientation = pose["orientation"];

Eigen::Isometry3d target_pose;
target_pose.translation() =
Eigen::Vector3d(position['x'].as<double>(), position['y'].as<double>(), position['z'].as<double>());
target_pose.linear() = Eigen::Quaterniond(orientation['w'].as<double>(), orientation['x'].as<double>(),
orientation['y'].as<double>(), orientation['z'].as<double>())
.toRotationMatrix();
target_poses.push_back(target_pose);
}
return target_poses;
}
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.

Let's use the YAML serialization structs here, and flatten the serialization keys to [x, y, z, qw, qx, qy, qz].

Suggested change
static VectorIsometry3d parseYAML(YAML::Node pose_file)
{
VectorIsometry3d target_poses;
for (const auto& pose : pose_file["poses"])
{
auto position = pose["position"];
auto orientation = pose["orientation"];
Eigen::Isometry3d target_pose;
target_pose.translation() =
Eigen::Vector3d(position['x'].as<double>(), position['y'].as<double>(), position['z'].as<double>());
target_pose.linear() = Eigen::Quaterniond(orientation['w'].as<double>(), orientation['x'].as<double>(),
orientation['y'].as<double>(), orientation['z'].as<double>())
.toRotationMatrix();
target_poses.push_back(target_pose);
}
return target_poses;
}
namespace YAML
{
template <typename FloatT>
struct convert<Eigen::Transform<FloatT, 3, Eigen::Isometry>>
{
using T = Eigen::Transform<FloatT, 3, Eigen::Isometry>;
static bool decode(const Node& node, T& val)
{
Eigen::Matrix<FloatT, 3, 1> trans;
trans.x() = get<FloatT>(node, "x");
trans.y() = get<FloatT>(node, "y");
trans.z() = get<FloatT>(node, "z");
Eigen::Quaternion<FloatT> quat;
quat.w() = get<FloatT>(node, "qw");
quat.x() = get<FloatT>(node, "qx");
quat.y() = get<FloatT>(node, "qy");
quat.z() = get<FloatT>(node, "qz");
val = Eigen::Translation<FloatT, 3>(trans) * quat;
return true;
}
};
// Serialization for std::vectors with Eigen-specific allocators
template <typename T>
struct convert<std::vector<T, Eigen::aligned_allocator<T>>>
{
static bool decode(const Node& node, std::vector<T, Eigen::aligned_allocator<T>>& v)
{
if (!node.IsSequence())
return false;
v.clear();
for (const auto& elem : node)
v.push_back(elem.as<T>());
return true;
}
};
} // namespace YAML

Comment on lines +41 to +47
YAML::Node poses = YAML::LoadFile(filename_);
VectorIsometry3d target_poses = parseYAML(poses);

std::transform(target_poses.begin(), target_poses.end(), std::back_inserter(target_poses),
[](const Eigen::Isometry3d& pose) { return createFrame(pose.translation().cast<float>(),
pose.rotation().cast<float>() * Eigen::Vector3f::UnitZ()); });
return target_poses;
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.

I think the transformation using createFrame is not necessary because we already have a list of fully defined poses from the YAML file. The purpose of createFrame was to create a fully defined pose from just a position vector and a normal vector (e.g., from a point cloud point and normal). Also, unless I'm mistaken, using std::back_inserter(target_poses) is adding all of the outputs of createFrame to the existing list of poses, effectively doubling the number of reach poses. This seems like it might have been unintentional, but was there a reason this was set up this way?

Suggested change
YAML::Node poses = YAML::LoadFile(filename_);
VectorIsometry3d target_poses = parseYAML(poses);
std::transform(target_poses.begin(), target_poses.end(), std::back_inserter(target_poses),
[](const Eigen::Isometry3d& pose) { return createFrame(pose.translation().cast<float>(),
pose.rotation().cast<float>() * Eigen::Vector3f::UnitZ()); });
return target_poses;
YAML::Node poses = YAML::LoadFile(filename_);
return poses.as<VectorIsometry3d>();

class YAMLTargetPoseGenerator : public TargetPoseGenerator
{
public:
YAMLTargetPoseGenerator(std::string filename);
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.

Avoids an unnecessary copy

Suggested change
YAMLTargetPoseGenerator(std::string filename);
YAMLTargetPoseGenerator(const std::string& filename);


namespace reach
{
YAMLTargetPoseGenerator::YAMLTargetPoseGenerator(std::string filename) : filename_(resolveURI(filename))
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.

Suggested change
YAMLTargetPoseGenerator::YAMLTargetPoseGenerator(std::string filename) : filename_(resolveURI(filename))
YAMLTargetPoseGenerator::YAMLTargetPoseGenerator(const std::string& filename) : filename_(resolveURI(filename))

Comment on lines +1 to +19
poses:
- position:
x: 1.0
y: 2.0
z: 3.0
orientation:
x: 0.0
y: 0.0
z: 0.0
w: 1.0
- position:
x: 4.0
y: 5.0
z: 6.0
orientation:
x: 0.0
y: 0.0
z: 0.0
w: 1.0 No newline at end of file
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.

Changes/simplifies the format to align with the suggested YAML serialization changes

Suggested change
poses:
- position:
x: 1.0
y: 2.0
z: 3.0
orientation:
x: 0.0
y: 0.0
z: 0.0
w: 1.0
- position:
x: 4.0
y: 5.0
z: 6.0
orientation:
x: 0.0
y: 0.0
z: 0.0
w: 1.0
- x: 1.0
y: 2.0
z: 3.0
qw: 1.0
qx: 0.0
qy: 0.0
qz: 0.0
- x: 4.0
y: 5.0
z: 6.0
qw: 1.0
qx: 0.0
qy: 0.0
qz: 0.0

Comment thread README.md
Comment on lines +53 to +71
poses:
- position:
x: 0.3629872500896454
y: -0.06363007426261902
z: -0.0005234468844719231
orientation:
x: 0.09166051483190088
y: -0.0017855572788278349
z: 0.9957755116009193
w: 0.005127601962168693
- position:
x: 0.3251079320907593
y: -0.0632486343383789
z: 0.006282307207584381
orientation:
x: 0.07515697809440232
y: -0.002977202008927411
z: 0.9971538575858108
w: 0.005171964196712022
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.

Changed to align with suggested serialization method

Suggested change
poses:
- position:
x: 0.3629872500896454
y: -0.06363007426261902
z: -0.0005234468844719231
orientation:
x: 0.09166051483190088
y: -0.0017855572788278349
z: 0.9957755116009193
w: 0.005127601962168693
- position:
x: 0.3251079320907593
y: -0.0632486343383789
z: 0.006282307207584381
orientation:
x: 0.07515697809440232
y: -0.002977202008927411
z: 0.9971538575858108
w: 0.005171964196712022
- x: 0.3629872500896454
y: -0.06363007426261902
z: -0.0005234468844719231
qw: 0.005127601962168693
qx: 0.09166051483190088
qy: -0.0017855572788278349
qz: 0.9957755116009193
- x: 0.3251079320907593
y: -0.0632486343383789
z: 0.006282307207584381
qw: 0.005171964196712022
qx: 0.07515697809440232
qy: -0.002977202008927411
qz: 0.9971538575858108

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.

2 participants