ros-industrial / ros_canopen

CANopen driver framework for ROS (http://wiki.ros.org/ros_canopen)
GNU Lesser General Public License v3.0
336 stars 271 forks source link

Option to ignore error frames #362

Closed mathias-luedtke closed 4 years ago

mathias-luedtke commented 4 years ago

Alternative to #264 with proper interface definition Parameter parsing is not implemented yet.

ToDo:

mathias-luedtke commented 4 years ago

@yanick-douven, @timple: I have refactored this now, please give it a try.

It features an element-wise mask setting (like #388), but for the entries of the mask, not to ignore them.

CAN_ERR_LOSTARB is part of the default error mask (gets reported to ROS), but it is not fatal anymore. The behavior can be changed by setting error_mask/CAN_ERR_LOSTARB to false or fatal_error_mask/CAN_ERR_LOSTARB to true. Either from ROS or in a SettingsMap.

Timple commented 4 years ago

Code looks good. It's good to have the option tot ignore, inform and error.

From a code perspective it's approved. We will have an opportunity to test this friday.

MCFurry commented 4 years ago

Thanks for the effort @ipa-mdl Indeed codewise looks good, we'll try to verify tomorrow on our hardware!

Timple commented 4 years ago

Running the code with default settings we noticed a lot of errors:

[W 0:29:07:407] Received frame is error: 20000002#0f00000000000000
[W 0:29:07:615] Received frame is error: 20000002#1100000000000000
[W 0:29:07:633] Received frame is error: 20000002#0900000000000000

We know now that these are arbitration errors because of the 2 before the #. But the error description could be more clear I guess.

mathias-luedtke commented 4 years ago

We know now that these are arbitration errors because of the 2 before the #. But the error description could be more clear I guess.

This is how socketcan_bridge reports error frames. Which output would you like to get?

In general I do not recommend socketcan_bridge for production anyway..

Timple commented 4 years ago

Which output would you like to get?

The outcomes of this function seem quite readable. But this is a nice to have, not blocking.

In general I do not recommend socketcan_bridge for production anyway..

In the prototype phase it is very convenient to have all canbus messages in the bagfiles instead of setting up a seperate can logging system :)

mathias-luedtke commented 4 years ago

@ipa-hsd: Do you have time to test this PR with the robot in our lab?

hsd-dev commented 4 years ago

@ipa-mdl I could do this next week.

mathias-luedtke commented 4 years ago

@ipa-hsd: That's fine! Just check if the robot still moves properly :)

Timple commented 4 years ago

plan is to have a solution ready (merged..) until World ROS-Industrial day (July 7) :)

We came a long way, so a subtle ping to not let this fall asleep...

hsd-dev commented 4 years ago

I tried out the PR today with pilz robot with:

roslaunch prbt_moveit_config moveit_planning_execution.launch sim:=false pipeline:=ompl

It "works". But I get the following error message (it is the case even on melodic-devel branch of ros_canopen though):

[ERROR] [1595944295.136523793]: EMCY received: 83#0000000000000000
[ERROR] [1595944295.140129373]: EMCY received: 84#0000000000000000
[ERROR] [1595944295.143812940]: EMCY received: 85#0000000000000000
[ERROR] [1595944295.148101649]: EMCY received: 86#0000000000000000
[ERROR] [1595944295.151605465]: EMCY received: 87#0000000000000000
[ERROR] [1595944295.155340520]: EMCY received: 88#0000000000000000
jschleicher commented 4 years ago

@ipa-hsd Those aren't errors (all zeros) - the message is an Error (was stdout) since #286.

mathias-luedtke commented 4 years ago

@ipa-hsd: Thanks!

As already pointed out, these "errors" can be ignored (tracked in new issue https://github.com/ros-industrial/ros_canopen/issues/396)

Timple commented 4 years ago

Great! I'll make sure we test the debian release as well before the sync

mathias-luedtke commented 4 years ago

I pushed a new release: https://github.com/ros/rosdistro/pull/26275