-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Publish IR Opcodes #66
Conversation
As of now this PR is in WIP state. The dock's emitter range and fov parameters are hard-coded to the plugin. The idea is to detect these parameters within the plugin automatically. |
Do you have a screenshot to see how it looks like? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments addressed!
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/docking_manager.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes, Rodri.
Left a round of comments for you. PTAL.
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
d2fa814
to
d3c76b4
Compare
@eborghi10 All your comments were addressed |
5c3bd97
to
6351e58
Compare
@apojomovsky @lchico @LolaSegura @eborghi10 I finally got pipelines to pass. Can I please get a review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, the PR looks very nice! I left some comments! Thanks for the hard works done here!
irobot_create_description/urdf/sensors/ir_opcode_receivers.urdf.xacro
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/docking_manager.hpp
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/docking_manager.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Show resolved
Hide resolved
fixed plugin dock with links Halo detection Force field detection working Buoy detection working Publishing IR opcodes for both Force field and buoys detection Fix clang format Improved Red, Green and Yellow Buoys detection Update gazebo_ros_ir_opcode.cpp Fixes red and green buoy detection Fix corner case when buoys are detected when they shouldn't Update irobot_create_description/urdf/create3.urdf.xacro Co-authored-by: Emiliano Borghi <creativa_eborghi@irobot.com> Update irobot_create_description/urdf/dock/ir_emitter.urdf.xacro Co-authored-by: Emiliano Borghi <creativa_eborghi@irobot.com> Update irobot_create_description/urdf/dock/standard.urdf.xacro Co-authored-by: Emiliano Borghi <creativa_eborghi@irobot.com> Update irobot_create_description/urdf/dock/standard.urdf.xacro Co-authored-by: Emiliano Borghi <creativa_eborghi@irobot.com> Update irobot_create_gazebo_plugins/src/gazebo_ros_ir_opcode.cpp Co-authored-by: Emiliano Borghi <creativa_eborghi@irobot.com> Fix update_rate Created docking_manager class Improvements to class renames file Both sensors in a single plugin to ensure publish rate self review Address peer review comments Unused includes removed, fixed clang_format Address comments Stop using Vector2d to represent polar coordinates const references for docking manager Adding const where needed Address code style comments New opcodes logic Updates IR opcode logic publish rate at 31 Hz corrects comment Debug message instead of WARN fix base_link Lint errors Fix lint restore file
88211d3
to
f3a159e
Compare
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/docking_manager.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
@eborghi10 @lchico @apojomovsky All your comments were addressed will you PTAL? |
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Show resolved
Hide resolved
irobot_create_gazebo_plugins/include/irobot_create_gazebo_plugins/gazebo_ros_ir_opcode.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eborghi10 @lchico @apojomovsky Can I get a review?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR Adds:
Fixes #61 Fixes #84
Type of change
How Has This Been Tested?
To test you can teleop the robot and position it in different poses around the dock. The published IR Opcode should reflect what the rays visuals show in gazebo. When the robot's receivers intersect the dock's emitters the corresponding IR Opcode should be published.
Keep in mind that an IR Opcode msg with sensor = 0 represents the Omni receiver and sensor = 1 represents the forward facing receiver.
Checklist
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
Remove hardcoded visualization flags before merge