add configurable homing timeout#442
add configurable homing timeout#442sgermanserrano wants to merge 1 commit intoros-industrial:melodic-develfrom
Conversation
mathias-luedtke
left a comment
There was a problem hiding this comment.
Thanks! I had a similar patch in mind. Right now I am just not sure, if we should break the ABI like this.
canopen_402/src/motor.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| homing->setHomingTimeout(homing_timeout_); |
There was a problem hiding this comment.
Why did you create a new function and did not pass it as a parameter to executeHoming?
There was a problem hiding this comment.
@ipa-mdl tbh, I didn't know which option was better, I'll refactor to have it as:
executeHoming(canopen::LayerStatus &status, const boost::chrono::seconds &homing_timeout)
and pass it with the call to executeHoming
There was a problem hiding this comment.
Me neither ;)
That's why I wanted to know why you chose that option.
I'll refactor to have it as:
executeHoming(canopen::LayerStatus &status, const boost::chrono::seconds &homing_timeout)
and pass it with the call to executeHoming
👍
Please keep the function without the argument as well (stable API) and call the new one with a timeout of 10s (as before).
I will try to figure out, if we can avoid the ABI break in Motor402.
There was a problem hiding this comment.
I've added the new function and replace the call to executeHoming with a default timeout of 10s
3451669 to
16f1d77
Compare
Signed-off-by: Servando German Serrano <servando.german.serrano@gmail.com>
16f1d77 to
1aa661d
Compare
|
Any updates or help needed on this pull request? I've tested it on a super slow joint and the homing was working really good, so looking forward for a merge :) |
|
Ping. Please merge |
Enables to have a configurable homing timeout for slower joints, e.g.
It defaults to 10s to keep the original behaviour
Signed-off-by: Servando German Serrano servando.german.serrano@gmail.com