fix gazebo_ros_power_monitor plugin #140
Conversation
|
|
||
| // // Initialize parameters | ||
| // Param::Begin(¶meters); | ||
| // robot_namespace_param_ = new ParamT<string>("robotNamespace", "/", 0); |
There was a problem hiding this comment.
Here any parameters should have been read but no parameters actually read from urdf and all the parameters are zero which can cause zero division error and therefore this plugin publishes wrong trivial data.
|
kindly ping to maintainers. Without this patch, spawning PR2 will cause a crash on gazebo. |
There was a problem hiding this comment.
Sorry @furushchev , I postponed reviewing this because it's rather big changes to fix something seemingly easy.
You rearranged a number of lines for no obvious reason, deleted some others and you did not split the changes into multiple commits that explain why you changed things. Please try to do this so we can review more easily!
"fix a crash" is no good description of rewriting half the plugin (which was fully justified now that I looked at it).
I approve
|
@furushchev please revert unnecessary changes. |
|
@furushchev please revert unnecessary changes.
To me the "unnecessary" changes are quite an improvement and I would be happy to keep them.
However, if you think so too, it would be good to have them in separate commits.
|
furushchev
force-pushed
the
fix-power-monitor
branch
from
2c098b9
to
1f1afe2
Compare
|
@k-okada ping |
This fixes a crash on 32 bit range error in
pr2_gazebo.