search

gavanderhoorn / fanuc_driver_exp

An alternative - experimental - Fanuc robot driver for ROS-Industrial
Apache License 2.0
29 stars 16 forks source link

Clear trajectory buffer on E-stop #10

Closed hersh closed 6 years ago

hersh commented 6 years ago

Hi, thanks for making fanuc_driver_exp public, it is very helpful!

I ran into issue #4 here and tried to solve it myself. This change only responds to emergency-stop buttons, not other errors. I'm still quite new to Fanuc and Karel etc so I wanted to limit my changes.

I also added ACC66 to ros_movesm.ls because the robot motion was too jerky for us.

My e-stop handler routine is just a copy of the stop-request handling code, but since that referred to a local variable in a function, I made that a global variable so my new function could set it. That doesn't necessarily seem like good software engineering to me, but again I'm pretty new to Karel.

If you have suggestions how to do it better, I can test them on the Fanuc in our lab if that's helpful.

Thanks again!

gavanderhoorn commented 6 years ago

If you have suggestions how to do it better, I can test them on the Fanuc in our lab if that's helpful.

Thanks for the offer. For now, I would be interested to know what model robot you have in your lab.

gavanderhoorn commented 6 years ago

Btw @hersh: none of the commits you've made are contributed to your github account. Not sure whether you'd want to fix that?

hersh commented 6 years ago

Thanks for mentioning. I was using my work email when doing this. I appreciate your comments, hopefully I'll have time to address them soon.

gavanderhoorn commented 6 years ago

@hersh: I've pushed some commits. These address the few comments I had.

Could you check the attribution of your commits? It'd be ok if you squash everything into a single commit and force push.

gavanderhoorn commented 6 years ago

I'm sorry for changing your copy of ros_movesm btw (removed the ACC66). As you didn't use a branch I had to push to your master.

hersh commented 6 years ago

Thanks for doing all the work for me! I don't care about the attribution, I'm ready to leave it as is. :)

If it causes trouble for you, let me know and I'll fix it.

I'm surprised you had permissions to push to my "master", but I'm glad you fixed the problems. :)

Thanks a bunch.

hersh commented 6 years ago

I will also add that this does not seem to be a total solution to the problem. Many times it does seem to clear the queued motion and sit still when I re-start the Fanuc program, but occasionally it starts moving again anyway. Maybe there are different places where trajectories would be stored? I don't know what else to try.

We have a Fanuc R1000iA-80F arm with the R30iB controller.

gavanderhoorn commented 6 years ago

@hersh wrote:

I'm surprised you had permissions to push to my "master", but I'm glad you fixed the problems. :)

Allowing changes to a pull request branch created from a fork.

Thanks for doing all the work for me! I don't care about the attribution, I'm ready to leave it as is. :)

Ok.

I will also add that this does not seem to be a total solution to the problem. Many times it does seem to clear the queued motion and sit still when I re-start the Fanuc program, but occasionally it starts moving again anyway.

Yes, I still have to do some checks with real hw. The list is really the only place incoming trajectories are stored, but ros_movesm also caches one point. I'll need to verify the interaction between ROS_TRAJ and ros_movesm in case of e-stops and programs being paused.

I'd also like to make this more generic, so it will clear the list upon any error.

gavanderhoorn commented 6 years ago

@hersh wrote:

Many times it does seem to clear the queued motion and sit still when I re-start the Fanuc program

I just want to make sure: you write "restart the Fanuc program". Are you actually restarting things, or unpausing? The former should always clear the list (as the list ctor is invoked), the latter is a situation handled by the additions in this PR, so could perhaps need something more.

gavanderhoorn commented 6 years ago

I've just tested this with our M-430iA, and it seemed to work correctly. I've not been able to reproduce a situation where after an e-stop the robot started moving again.

I'd also like to make this more generic, so it will clear the list upon any error.

I've actually decided not to do this, as there are many errors that may not need to clear the list. At least for now I'd like to be able to 'pause' execution of a trajectory by letting go of shift fi in manual mode. That is a warning, but then letting go of the deadman is an error. If we'd clear the list, I cannot resume the trajectory like I would a regular program.

It's likely I'll reconsider this in the near future, to make the behaviour of this driver more in-line with other drivers (such as MotoROS).

gavanderhoorn commented 6 years ago

I'm going to merge this PR as it is now.

We'll likely want to clear the buffer on any error, as I wrote in my earlier comments, but for now this is already an improvement.

Thanks for the contribution @hersh. Much appreciated.

And my apologies for the poor performance of the driver. Things are unfortunately rather limited (although #12 is interesting).