search

ros-industrial / abb

ROS-Industrial ABB support (http://wiki.ros.org/abb)
145 stars 152 forks source link

Should execution_duration_monitoring be disable as the default behavior? #114

Closed Levi-Armstrong closed 5 years ago

tycho94 commented 7 years ago

Hello @Levi-Armstrong I remember that I said the default behavior for execution_duration_monitoring was disabled, but I now realized I mixed it up.

When not set in the trajectory_execution.launch.xml it is enabled, not disabled.

It might be worth it to add this with the monitoring default behaviour set to true for clarification.

Levi-Armstrong commented 7 years ago

@tycho94 I think it would be good to add it with the default value set to true. This way it is easier for developers to change since I know it cause issues sometimes.

tycho94 commented 7 years ago

Indeed, this was my intention in the first place. It has caused issues for me as well.

Sadly I mixed up the default value when I added it in abb-experimental.

gavanderhoorn commented 7 years ago

I would advise not to make disabled the default in the MoveIt configurations. I understand why someone would want to do that (otherwise things won't work, right?), but I feel that it might be a good thing that users have to consciously make the decision to disable the TEM.

Additionally, when not using abb_driver, performance is most likely good enough to not need this disabled at all, and then we'd have to revert this again.

Levi-Armstrong commented 7 years ago

I agree, the default value should set to true.

gavanderhoorn commented 5 years ago

Seeing as there appears to be an agreement between the two main maintainers that TEM should be kept enabled -- and it currently is -- I'll close this issue.

We can reconsider this in the future if/when needed.