search

ros-industrial / motoman

ROS-Industrial Motoman support (http://wiki.ros.org/motoman)
146 stars 195 forks source link

motoman_driver accepting joint_states that are not from the robot arm controller #326

Open MarqRazz opened 4 years ago

MarqRazz commented 4 years ago

On my robot I have a gripper that is publishing its joint_states on the topic /my_gripper/joint_states and this information is relayed down the the /joint_states topic. I have recently found that I am randomly receiving errors from the industrial_robot_client stating: isWithinRange::Key vectors are not similar and my trajectory being rejected.

I did some digging and found that in joint_trajectory_interface.cpp in the jointStateCB the software blindly accepts the incoming message on the /joint_states topic as the robots cur_joint_pos_. I added a simple check if(this->all_joint_names_.size() != msg->position.size()) then just return from the callback without updating the robots cur_joint_pos_ and it fixed my errors.

I am happy to put in a pull request with a fix for this error but wanted to get your guy's feedback on how you would like to ensure the received JointState message is for the robot arm we are connected to. Should we check to make sure that all of the names in the all_joint_names_ variable are included in the msg->name list? Should we check to make sure that the JointState message includes some minimum level of information (i.e. the name, position and velocity vectors all have the same length)? Anything else you would like it to check? Another option would be to launch the node in a namespace and we would get /motoman/joint_states but users would be required to relay that topic down to /joint_states for the PlanningSceneMonitor and RobotStatePublisher.

gavanderhoorn commented 4 years ago

So I believe the reason this is not such an issue (or at least not in the systems I use this driver with) is that I typically remap joint_states to more narrow topics. That way the driver only sees the JointStates it's supposed to.

This connects to a larger design discussion about whether consumers should be capable of dealing with "any" incoming message (of the same type) and be responsible for picking out the parts they are interested in, or whether by configuration (by an external entity) they should be able to rely on receiving only messages meant for them specifically.

See https://github.com/gavanderhoorn/motoman/pull/1#issuecomment-249615157 for an earlier discussion I've had about this.

https://github.com/gavanderhoorn/motoman/commit/e369ddc88e4a04adf847d9415ae94a0103ab9fd8 would be a fix for what you describe I believe.

I'm still not happy with having to do that here in this driver, but it does help make things more robust, so a PR with that change ported to current kinetic-devel would be something we can merge.

gavanderhoorn commented 4 years ago

@MarqRazz: would you have any updates?

There appears to be quite some activity in your fork, is any of that related to this issue?

MarqRazz commented 4 years ago

@gavanderhoorn I have been adding new robots to my repo (I am planning on making PR's for each bot after everything is working and verified). When I run on hardware I add this fix... image

So this is still an issue for me but I am assuming that we can get this change permanently added to the repo, I just haven't had time to make a PR.

simonschmeisser commented 2 years ago

We just had the same issue and I feel dump for not reading this here ...