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

Bugfixes, timestamp accounting for transmission delay, custom baseframe #12

Merged
merged 3 commits into from
Apr 20, 2023

Conversation

echoGee
Copy link

@echoGee echoGee commented Apr 11, 2023

  1. Fixed bugs for pointcloud publishing
  2. added a base frame variable to change from launch file
  3. publishes header with right timestamp accounting for transmit delay

…ng code, but not reliable. pointcloud publish does not work"
2. Changes the timestamp of the ros messages to account for the delay in transmission from the mocap system
@@ -69,7 +71,7 @@ Launch file `natnet_ros.launch` contains the several configurable arguments. The

#### Publishing the single marker
It is possible to track the single marker as a rigid body with constant orientation. Go to the `config/initiate.yaml` It is suggested to make a copy of the file and rename the new file.
The file contains the details on what to modify.
The file contains the details on what to modify. Basically, the approximate position of an individual marker has to be noted in this file and a basic filtering algorithm will track it over time. The filtering is not that robust and you can see it disappear often
Copy link
Member

Choose a reason for hiding this comment

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

Improved algorithm will be available soon. I am working on major changes and some additional features.

msgRigidBodyPose.header.frame_id = "world";

msgRigidBodyPose.header.stamp = ros::Time::now();
initROSmsgHeader(msgRigidBodyPose.header, internal);
Copy link
Member

Choose a reason for hiding this comment

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

This looks good. I have one question,
It is not a good idea to use initROSmsgHeader(msgRigidBodyPose.header, internal); for each body and markers. Because I think it might cause major time stamp difference when there are multiple bodies and code will loop through all the number of frames for publishing.
I will modify it a bit.

Copy link
Author

Choose a reason for hiding this comment

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

didn't follow. all the bodies in one frame should have the same time frame

Copy link
Member

Choose a reason for hiding this comment

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

suppose we have a huge arena and we have multiple rigid bodies in parallel with multiple single marker tracking, so what your implementation is doing, is assigning all the same time stamp. But publishing is not done at the same time. Publishing is FIFO so all the body and markers will have slightly different time stamp (difference in nano seconds)

Copy link
Author

Choose a reason for hiding this comment

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

The actual publishing is done in different timestamp, but the header timestamp consists of the "real" time at which the mocap captured the frame

Copy link
Member

Choose a reason for hiding this comment

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

Yes that's where I am bit confused.
What makes more sense to you, to consider the time of arrival of data in queue or time of exit minus the communication delay between the systems?

Copy link
Author

Choose a reason for hiding this comment

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

Having a header timestamp pointing to as precise of a time of exact occurrence helps for controls tuning, debugging etc

Copy link
Member

Choose a reason for hiding this comment

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

I see. I will keep it optional and let users choose one of the model for the timestamp in the launch file.

@@ -68,10 +68,10 @@ class Internal
void PubRigidbodyMarker(sMarker &data, Internal &internal);

// Publish Single marker as the Rigidbody and TF
void PubMarkerPose(sMarker &data, Internal &internal);
void PubMarkerPoseWithTrack(sMarker &data, Internal &internal);
Copy link
Member

Choose a reason for hiding this comment

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

I prefer not change the name of this function as I have already developed on top of this.

Copy link
Author

Choose a reason for hiding this comment

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

felt it was more descriptive this way :)

if (replaced_strName)
ROS_INFO("RigidBody found : %s replaced name with %s", pRB->szName, body_name.c_str());
else
ROS_INFO("RigidBody found : %s", pRB->szName);
Copy link
Member

Choose a reason for hiding this comment

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

Never faced this issue but good idea to consider this as it might create some troubles. Good found ;)

@aarsht7
Copy link
Member

aarsht7 commented Apr 14, 2023

I won't be able to merge the pr for now as I am working on some other changes as well. plus I will have to test the code locally before merging it to the main. I will put it in other branch and let user choose the other branch till I finish the update. I will include the changes in the next update. can you re-base the pr to the branch issue_10_11 please.
Thanks!

@aarsht7 aarsht7 changed the base branch from main to issue_10_11 April 20, 2023 07:24
@aarsht7 aarsht7 merged commit c964346 into L2S-lab:issue_10_11 Apr 20, 2023
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