-
Notifications
You must be signed in to change notification settings - Fork 463
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 option to continuously transform visuals in point cloud plugin #1687
base: noetic-devel
Are you sure you want to change the base?
Add option to continuously transform visuals in point cloud plugin #1687
Conversation
…w.r.t. fixed frame This option is particularly useful for messages, whose frame is moving w.r.t. fixed frame.
… behavior of "continuous transform" option
Dear @AndreasR30, |
Hello everyone, I still don't think, that this feature is necessary. The problem described by @AndreasR30 occurs, because they publish data in the map frame but visualize it in the base link. Therefore, they select a fixed frame for their visualization which is not fixed in regards to the data. You could simply fix your visualization by selecting "map" as your fixed frame and lock the camera to your base link. The Best regards, |
Hello @Martin-Oehler , Here we have one Odometry message (red), which represents wheel odometry (dead reckoning) and lives in the With your description it would be possible to visualize one, but not both at the same time in Rviz. However, this is quite useful if you want to compare how well dead reckoning is working w.r.t. ground truth. Even in the case where your description works because you only want to visualize messages in the same frame, I don't find it optimal when the user has to think about, which fixed frame he has to use to make the visualization work. In my opinion the visualization should work always irrespective of the fact what is the fixed frame. So, for me, the fixed frame option should just change where the camera is fixed to but not whether a visualization works or not. I don't understand your second point:
Can you make an example scenario where you want the marker to move but not the data? Why do you think the transform is "wrong"? I think the transforms used in this pull request are correct. I admit that this pull request for PointCloud2 is less important, than the one for Odometry display. Because in our scenario it just makes the visualization smoother, when the TF w.r.t. to fixed frame is updated more frequently than the message rate. But I still think it is important for many users. Maybe we can at least agree that it is very useful for Odometry message? Kind regards, |
You could do the same in the UTM frame, as you can see from the screenshot here, that I took from your bag. However, rviz does not handle these high numbers very well. You could zero your utm frame at the starting position and publish a separate NavSatFix message. See also rviz_satellite.
Well, with your change, only data that is already transformed to the correct fixed frame will be displayed correctly. All other data is transformed wrong, which is much worse, I think.
It is correct for the special case where your data is already transformed to its fixed frame. In all other cases, old data will be transformed with new transforms, leading to wrong results. I can add the equations, if you are interested.
No idea honestly. I don't think it makes much sense in either case. Best regards, |
Not sure what you were doing but in my Rviz the visualization is wrong. It does not show the drift of the odom frame over time. . I also used utm in global options and in Views:
My pull request handles these high numbers automatically. Why not simply use it without having to introduce new intermediate frames to avoid large TFs. In this scenario I want to compare Odometry Messages against each other and not aerial image against Odometry Message.
No that is wrong. You can visualize the point cloud when the fixed frame != message frame. Message in odom frame and fixed frame is base_link and it still works perfectly: demo-2021-12-21_19.35.29.mp4It does not matter which fixed frame you choose. Unlike in the old implementation: demo-2021-12-20_15.22.01.mp4Furthermore, I want to stress that this is just an option now, which can be activated if one wants to.
I would prefer to have a minimal rosbag available of your Gazebo Simulation, where you show problems with the new implementation. I want to understand where the problem comes from.
I can tell you where this feature is useful. For example we recorded UTM coordinates while where were driving on our track. Now we want to visualize this static track by means of markers. By using the frame_locked feature it is enough to publish the marker once because its pose w.r.t. fixed frame is continuously re-computed. Here is the code: #!/usr/bin/env python
import rospkg
import rospy
import scipy.io
import time
from geometry_msgs.msg import Point, Vector3
from std_msgs.msg import ColorRGBA
from visualization_msgs.msg import Marker
rospy.init_node('visualize_test')
rospack = rospkg.RosPack()
path = rospack.get_path('mpc_path_follower') + '/paths/cpg_fast_lap_BagT.mat'
mat = scipy.io.loadmat(path)
x = mat['x'][0]
y = mat['y'][0]
marker = Marker()
marker.header.stamp = rospy.Time.now()
marker.header.frame_id = "utm"
marker.ns = "path_points"
marker.id = 1
marker.type = Marker.SPHERE_LIST
marker.action = Marker.ADD
marker.pose.position = Point(x[0], y[0], 0)
marker.pose.orientation.w = 1
marker.scale = Vector3(.2, .2, .2)
marker.color = ColorRGBA(1, 0, 0, 1)
marker.lifetime = rospy.Duration(0)
marker.frame_locked = True # important: tells Rviz plugin to continuously transform into 'fixed_frame' -> no need to publish message periodically
for i in range(x.shape[0]):
marker.points.append(Point(x[i] - marker.pose.position.x, y[i] - marker.pose.position.y, 550))
pub = rospy.Publisher('path_points', Marker, queue_size=10)
time.sleep(1)
pub.publish(marker)
rospy.spin() path-2021-12-14_16.35.38_short.mp4We think such a feature would be essential for nearly any visual not only Markers. Finally, I want to show you that there are multiple Displays, which constantly re-transform into fixed frame in the update step with the latest stamp available:
Would you also say that their transforms are wrong/bad? |
The displays, you mention @AndreasR30, don't display data recorded at a specific timestamp, but they visualize data w.r.t. to the current time. An exception might be the Effort display, which displays effort data at some timestamp but using the current/latest joint pose. |
@rhaschke and @Martin-Oehler Please give me a rosbag (pointcloud topic, /tf, /tf_static) or a synthetic example, where the new visualization option fails. I am quite sure that there is an issue with the messages' frame_id or stamp. Or with your TF tree. It's a pity that you simply ignore the issues you can definitely see in the synthetic example above. The point cloud there is in map frame and should clearly stay at the same position w.r.t. the map frame. This means the parent frame has to move in the screen because the camera is moving w.r.t. to map frame. The visualization is only correct at the moments, when there is a new message but in between it is wrong. |
@rhaschke Also the Map display shows data from a specific frame and stamp. E.g. you can generate a OccupancyGrid message from a single laser scan. With the option the grid's visualization remains valid even when some time elapsed since the last published message. In general OccupancyGrid and a PointCloud2 are a quite similar thing, when you publish a PointCloud2 in a world fixed frame. Why then not make their visualization similar? |
From your screenshot, I am not sure what you are doing differently. But since both odom paths will end at the current robot position by definition and they are not the same path, you are bound to see the drift.
That's a nice addition then. You should probably split these changes into a separate PR. However, it won't fix the rviz camera view, which for me did not work properly anymore.
The odom frame is still a fixed frame with regards to the data. Your changes break visualization, when the data is in the base link, e.g. data from a lidar or depth camera.
Okay, here you go: https://drive.google.com/drive/folders/17E2ejoBJm8qcFWe0rZeAEmz0XQKXY8ql?usp=sharing The bag contains a lidar scan of a rotating VLP-16, published in the laser frame. An rviz config is provided. This video shows the visualization with the correct behavior previous to your patch: correct_visualization.mp4This video shows the broken behavior with "Continuous Transform" enabled: wrong_visualization_small.mp4
The map is different because it does not represent data from a specific sensor. It is expected to be published in a world-fixed frame, unlike point cloud data. |
Maybe, provide your rviz config, @Martin-Oehler? |
Oh well, I did some more investigation and that one is on me. For some reason, our build server delivered on out-dated version of rviz which actually still had the proposed change from #1686. I agree, that this change makes sense for odometry, but we should discuss this in the other PR. The main difference is that point cloud data can be from a fixed frame (map, odom) or from a frame attached to the robot body. Odom is typically published in some fixed frame. I put a bit more thought into this and think that the new behavior is fine as an option. Using the timestamp is still the more general way, since we don't know the origin of the point cloud, but if the user knows, that the cloud is already given in a fixed frame, this feature can be useful. However, I would change the name of the option to something like "Ignore timestamp" to make the naming in line with the option in the Map display (which is "Use timestamp" and does the opposite). |
@Martin-Oehler Thank you very much for providing your rosbag. It looks fine. Very interesting application of the lidar sensor by the way. I think our discussion mainly comes from the fact that we follow different philosophies for visualization: This, of course, can lead to strange behavior (as one see in your videos). In my philosophy, it would be necessary to transform the sensor data into a world fixed frame based on its time stamp. (That is why the time stamp can be ignored afterwards). I understand it can be annoying to do this extra step. However, I think this is the more powerful and flexible approach as it can handle multiple (pseudo-) fixed frames like odom and utm frames in the odometry example above. Also a multi robot scenario, where each has its own lidar sensor and odom frame, can be handled. Furthermore, I guess, in order to generate an accumulated point cloud, as in the video above, which can be used in a downstream perception task, you have to transform the individual scans to a world fixed frame anyway (because of sensor motion). However, I also understand your approach, because one gets a nice visualization quite quickly without having to transform sensor data before. It works well when there is only one fixed frame to consider, which is the typical use case with point clouds. A very big reason, why we do not work with https://www.dropbox.com/s/aefs56f6zx0nt9g/jittering.mp4?dl=0 vs: https://www.dropbox.com/s/fp0odav15knkz4c/no_jittering.mp4?dl=0 However, this is not a problem of @Martin-Oehler's approach itself but rather because of problems with robot model display. We are already working on a pull request, which fixes this. I would be happy to get get this pull request accepted, so the user can decide whether he or she wants to transform the sensor data into world fixed frame by him-/herself in order to be more flexible or whether rviz should do this.
I am happy that you understand why this option can be useful for some applications. I also agree that your approach is the more typical one for point clouds. So the option should be off by default. I'm not completely happy with "Ignore timestamp" / "Use timestamp" naming, but I can accept it to be consistent with other displays. @rhaschke What do you think? |
Hello,
I made a new pull request, which is essentially the same as #1652, which was merged but then reverted as it broke the visualization of other users. But now the changes are less invasive and do not affect old visualizations as long the option is unchecked (the default). The new option triggers the same behavior as the activated
frame_locked
flag in the Marker Message.