Improvements and fixes for various issues #13

rhaschke · 2020-10-07T10:44:44Z

Fixes [rviz_tactile_plugin] False impression of missing data with "no recent msg" status #10, by not displaying the "no recent msg" warning anymore. Wrenches silently disappear, as like point cloud points do.
Fixes [rviz_tactile_plugin] contact_state display of "all taxels" is inconsistent #7, by correctly initializing (and exposing) the "Hide small values" property of rviz' WrenchVisuals
Fixes [rviz_tactile_plugin] contacts stored forever, show wrong display on clock loop #9, resetting displays (and merger) when time is reset
Auto-connect to tactile_contact_state(s) topic

rhaschke · 2020-10-07T11:04:31Z

Looks like the "Hide Small Values" property of WrenchVisuals was introduced in Melodic only. This explains that #7 suddenly popped up: The property wasn't correctly initialized in rviz.
I vote for dropping support for Kinetic.

guihomework · 2020-10-07T11:13:52Z

Looks like the "Hide Small Values" property of WrenchVisuals was introduced in Melodic only. This explains that #7 suddenly popped up: The property wasn't correctly initialized in rviz.
I vote for dropping support for Kinetic.

I did not test the new proposed solution yet, but how can the "hide small values" come into play (whether it is on or off) when all values have the same amount (I am publishing a 1.0 on all) ?

rhaschke · 2020-10-07T11:26:17Z

How can the "hide small values" property come into play?

As I have written, the corresponding flag in rviz' WrenchVisual wasn't initialized and thus randomly true or false. If true, the arrows were hidden. What is considered small is hardcoded in rviz.

Looking at that code, I'm wondering why the vectors are hidden so early... Need to double-check.

guihomework · 2020-10-07T11:36:02Z

How can the "hide small values" property come into play?

As I have written, the corresponding flag in rviz' WrenchVisual wasn't initialized and thus randomly true or false. If true, the arrows were hidden. What is considered small is hardcoded in rviz.

So you mean that even with those "quite" large arrows they were hidden; Then the problem is indeed the decision of what is "small" that came into play.

regarding dropping support for kinetic. We have a melodic-branch, one can drop support of kinetic there I suppose.

guihomework · 2020-10-07T15:06:50Z

I confirm that changing the arrow width made the arrow visible even without this fix, so it is related to the way the hidesmallvalues is calculated. I will test this branch now

... considering individual levers from contact positions

rhaschke · 2020-10-07T15:52:02Z

Travis is finally happy as well.

guihomework · 2020-10-07T15:55:21Z

rviz_tactile_plugins/src/tactile_contact_display.cpp

@@ -235,21 +240,18 @@ void TactileContactDisplay::update(float wall_dt, float ros_dt)

  Display::update(wall_dt, ros_dt);

-  ros::Time now = ros::Time::now();
-  ros::Duration timeout(timeout_property_->getFloat());
+  ros::Time deadline = ros::Time::now() - ros::Duration(timeout_property_->getFloat());


I think this line is the cause of a crash when loading the plugin, and rosparam use_sim_time = true and no clock is published yet

terminate called after throwing an instance of 'std::runtime_error' what(): Time is out of dual 32-bit range

previous to this fix the plugin did not crash
here is the trace of the crash

#0 0x00007ffff60a9f47 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x00007ffff60ab8b1 in __GI_abort () at abort.c:79 #2 0x00007ffff6700957 in () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #3 0x00007ffff6706ae6 in () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #4 0x00007ffff6706b21 in () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #5 0x00007ffff6706d54 in () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #6 0x00007ffff3a77f8c in ros::normalizeSecNSecUnsigned(long&, long&) () at /opt/ros/melodic/lib/librostime.so #7 0x00007ffff3a7d00c in ros::TimeBase<ros::Time, ros::Duration>::operator+(ros::Duration const&) const () at /opt/ros/melodic/lib/librostime.so #8 0x00007ffff3a7d07f in ros::TimeBase<ros::Time, ros::Duration>::operator-(ros::Duration const&) const () at /opt/ros/melodic/lib/librostime.so #9 0x00007fff93dc734a in rviz::tactile::TactileContactDisplay::update(float, float) () at /home/.../work/dev/catkin_ws_agni/devel/lib//librviz_tactile_plugins.so #10 0x00007ffff7a7e156 in rviz::DisplayGroup::update(float, float) (this=0x5555558b5800, wall_dt=3.59348774, ros_dt=0) at /media/local/jenkins/jobs/rviz-agni-melodic-devel-ci-deploy-lsp-famula-nightly/workspace/src/rviz/display_group.cpp:240 #11 0x00007ffff7b48595 in rviz::VisualizationManager::onUpdate() (this=0x555556a461e0) at /media/local/jenkins/jobs/rviz-agni-melodic-devel-ci-deploy-lsp-famula-nightly/workspace/src/rviz/visualization_manager.cpp:361 #12 0x00007ffff6cb6555 in QMetaObject::activate(QObject*, int, int, void**) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #13 0x00007ffff6cc2fc7 in QTimer::timeout(QTimer::QPrivateSignal) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #14 0x00007ffff6cc3328 in QTimer::timerEvent(QTimerEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #15 0x00007ffff6cb707b in QObject::event(QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5

rhaschke · 2020-10-07T16:19:47Z

Thanks for the intense testing. Probably sim_time starts at zero and subtracting the timeout duration then would result in a negative deadline. Would be great if you could provide a rosbag file to test this scenario.

…f WrenchVisuals This fixes #7.

The "Hide Small Values" values property of WrenchVisuals was introduced in Melodic only.

rhaschke · 2020-10-07T16:32:18Z

I fixed the offending commit, rewriting history. Could you give it another try?

guihomework · 2020-10-07T17:15:09Z

Thanks for the intense testing. Probably sim_time starts at zero and subtracting the timeout duration then would result in a negative deadline. Would be great if you could provide a rosbag file to test this scenario.

no rosbag needed, I thought this was clear. Just start a roscore, then set rosparam set /use_sim_time true
then start rviz, load a tactile_contact_state plugin, crash.

guihomework · 2020-10-07T18:22:55Z

I am wondering now that this PR introduces changes on the messages and timeouts and stuff like that, could we have information, about newly incoming data, but that is immediately outdated ?
I mean contact data that has a time inferior to (now - timeout) when it arrives ?

I ask because I fear I vaguely remember asking for such a feature in the past, and maybe the "no recent msg" was somehow there to indicate that. One should be sure before deleting it. More details follow.

what happens is that one thinks everything is ok in the contact_state plugin, but there is no display at all. I would not care about empty contact being received, or no new data when a certain contact was not refreshed for a while (which would be a "no recent msg" and I don't need that), but I care about the fact that a new data cannot be published because it is outdated before being even displayed and would like to be informed if possible.
Such an information would mean one has forgotten to set use sim time to true, and publish valid contact state in the past,
or if the processing of contact state is too slow for instance.

Another issues related to that, is in case the tf is also outdated, in this PR, the status messages stays at "not yet published...." when data is clearly published, but tf cannot be processed

Possible reasons are listed at http://wiki.ros.org/tf/Errors%20explained
[ WARN] [1602094688.843153787]: TF_OLD_DATA ignoring data from the past for frame thumb_proximal_link at time 1.60209e+09 according to authority unknown_publisher

As sim time usually starts with zero, we cannot compute ros::Time::now - timeout, which would be negative and throw an exception.

guihomework · 2020-10-07T19:51:42Z

should I test now again with my more advanced examples ?

rhaschke · 2020-10-07T19:51:52Z

@guihomework, I performed a preliminary merge with melodic-devel to resolve the conflicts. If you give it the final approval, I will fast-forward melodic-devel to this PR branch.

rhaschke · 2020-10-07T19:59:34Z

@guihomework, you are asking a new feature (in #13 (comment)), now that this PR is close to ready. My suggestion is that we personally discuss what you want and implement the new feature next week. I need to focus on other things for the rest of the week.

guihomework · 2020-10-07T20:34:00Z

@guihomework, you are asking a new feature (in #13 (comment)), now that this PR is close to ready. My suggestion is that we personally discuss what you want and implement the new feature next week. I need to focus on other things for the rest of the week.

I had to write a comment in this PR before you merge in case the feature I asked in the past WAS the "status" you removed due to Gereon's request. Maybe adapting the deletion would have been necessary. I was not certain.

Just tell that what I am talking about is not affecting this PR, that it is ok to delete the "no recent msg" and we are good.
I was not really asking for more work, delaying the merge.
On the other hand this PR solves various issues at the same time, so maybe my idea as close to the work done in this PR.

Forget what I say. I will test now.

tactile_merger/src/merger_node.cpp

rhaschke added 2 commits October 7, 2020 12:40

Add more comments

669e9be

Improve wording

26c0b99

rhaschke requested a review from guihomework October 7, 2020 10:44

rhaschke added 2 commits October 7, 2020 17:15

Correctly handle negative normal forces when averaging in group

c6b3528

Correctly compute average torque

929f091

... considering individual levers from contact positions

rhaschke force-pushed the fixups branch 4 times, most recently from 1368faa to ba30571 Compare October 7, 2020 15:45

guihomework reviewed Oct 7, 2020

View reviewed changes

rhaschke added 4 commits October 7, 2020 18:31

Remove "no recent msg" warning

826cb39

Retry connecting to TactileContact(s) topic

d8c6056

Add a property "Hide Small Values" to expose the corresponding flag o…

1ddea52

…f WrenchVisuals This fixes #7.

Travis: Test against Melodic

33e609a

The "Hide Small Values" values property of WrenchVisuals was introduced in Melodic only.

rhaschke force-pushed the fixups branch from ba30571 to 33e609a Compare October 7, 2020 16:31

rhaschke added 2 commits October 7, 2020 21:34

Avoid negative ros::Time in TactileStateDisplay

bfba7bd

As sim time usually starts with zero, we cannot compute ros::Time::now - timeout, which would be negative and throw an exception.

Reset displays on jump back of time

a5c8a34

rhaschke mentioned this pull request Oct 7, 2020

Added detection of jumpback in time to clear contacts #14

Closed

Merge PR #13: Fix various smaller issues

976921b

guihomework reviewed Oct 7, 2020

View reviewed changes

tactile_merger/src/merger_node.cpp Show resolved Hide resolved

rhaschke mentioned this pull request Oct 8, 2020

Provide feedback about (no) incoming messages #16

Closed

guihomework approved these changes Oct 8, 2020

View reviewed changes

rhaschke merged commit 976921b into melodic-devel Oct 8, 2020

rhaschke deleted the fixups branch October 8, 2020 10:57

rhaschke mentioned this pull request Oct 8, 2020

Dropped tactile contact messages #18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements and fixes for various issues #13

Improvements and fixes for various issues #13

rhaschke commented Oct 7, 2020 •

edited

Loading

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework Oct 7, 2020 •

edited

Loading

rhaschke commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

guihomework commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

Improvements and fixes for various issues #13

Improvements and fixes for various issues #13

Conversation

rhaschke commented Oct 7, 2020 • edited Loading

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

rhaschke commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

guihomework commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020

rhaschke commented Oct 7, 2020

guihomework commented Oct 7, 2020

rhaschke commented Oct 7, 2020 •

edited

Loading

guihomework Oct 7, 2020 •

edited

Loading