tud-phi / ros2-mocap_optitrack Goto Github PK

Can you please check if somebody has published the NatNet SDK as a ROS package or on Github which we could simply clone as a separate package into the Docker container ROS workspace? This would be better than including the raw code into the REPO.

If you dont fine anything, its fine to leave it as it is.

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_client/include/NatNet/NatNetClient.h

Please replace all printf with the official ROS logging in all ROS nodes as for example here:

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_client/src/MoCapPublisher.cpp#L57

The ROS logging adds further information such as a severity level to the logging message. It for example is called like this:

RCLCPP_INFO(get_logger(), "Publishing: '%s'", msg->data.c_str());

https://github.com/ros2/demos/blob/foxy/logging_demo/src/logger_usage_component.cpp#L64

You can find further information about ROS2 logging here:
https://docs.ros.org/en/foxy/Concepts/About-Logging.html
https://docs.ros.org/en/foxy/Tutorials/Logging-and-logger-configuration.html

Please also replace all std::cout in the ROS nodes such as here:
https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_w2b/src/WorldToBase.cpp#L107

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_client/config/natnetclient.yaml#L19

Please rename the default topic to something like mocap_rigid_bodies or something similar

The code on the transformation matrices is still a bit confusing. It would be probably better to put more emphasis on the notation than on the memory consumption:

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_w2b/src/WorldToBase.cpp#L136

This would be at least from a notation perspective correct:

T_0_P = T_0_M*T_M_P;

Please also add some more code comments to describe what these transformation matrices are exactly doing.

The Eigen header files will already be installed in the Docker container. So please remove the Eigen folder at the end, when you integrate the REPO into the Docker container

Small issue here:

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_inv_kin/config/inverse_kinematics.yaml#L3

There is one _ too much. It should be ros_parameters

The parameters of the world to base rotation and translation should be ROS parameters configurable at run-time without recompliling. So please also move them into a .yaml file (for example config/world_to_base.yaml) and read them in as ROS parameters instead of putting them into the header file https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_w2b/include/WtoBconfig.h

Please add some description on how to run the nodes to the README (e.g. mostly just how to call the launch file).

Please also add a link to the Overleaf document describing the math behind it.

It might make sense to rely on the ROS2 package by the MOCAP4ROS2 project instead of implementing our own custom code.

I think it could be useful, to in addition to our custom RigidBodyArray message, also publish tf2 transform messages (with the tf2_ros::TransformBroadcaster, which are better supported by the ROS ecosystem. Could you please look into this wether this is useful and easy to implement?

https://docs.ros2.org/foxy/api/tf2_ros/

The idea would be that we still publish the RigidBodyArray, which contain more Mo-Cap-specific information, but also publish these standard tf2 messages which can be easily read by for example rviz. We would publish these tf2 messages for all rigid bodies, both in inthe MoCap client (e.g. in the world frame) and in the w2b package for the base frame of the robot.

I noticed that the MoCapNatNetClient.cpp is printing quite a lot to the terminal which is certainly great for debugging. Do you think it would make sense to add a setting to the classe, to activate / deactivate the logging? Basically adding a debug mode, which only prints if its activated

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_client/src/MoCapNatNetClient.cpp#L224

Please add a description what the package is doing to the package.xml. This will help future students work with the code:

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_client/package.xml#L6

Please also make sure that you have added all dependencies (such as eigen, the natnet sdk, etc.)

As far as I know, Emanuele has set the z-axis to be oriented downwards in the Optitrack Body software. This would mean, that the initial offset of 19mm should also act along the negative z-axis:

https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_w2b/config/world_to_base.yaml#L10
https://github.com/tud-cor-sr/ros2-mocap_optitrack/blob/master/mocap_optitrack_w2b/src/WorldToBase.cpp#L17