-
Notifications
You must be signed in to change notification settings - Fork 95
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
Ros1 fixes for 3D Detection and 2D/3D tracking #320
Conversation
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.
Thank you for the fixes on the nodes! I have left several comments below.
Most are minor things apart from the main issue that the nodes are missing the initialization that was there before as well as the listen methods. This is a point of difference between the ROS1 and ROS2 nodes that might have caused the confusion.
There are some updates needed in places where there are no changes on this PR so i couldn't directly comment on the lines, and consequently i added some comments with lists of changes regarding unchanged lines. I tried to make it as clear as possible, i hope they make sense.
projects/opendr_ws/src/perception/scripts/object_tracking_3d_ab3dmot.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_3d_ab3dmot.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/point_cloud_dataset.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_2d_deep_sort.py
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.
Thank you, not much to add beyond what brought up by Kostas, I'll test it when those points have been addressed
projects/opendr_ws/src/perception/scripts/object_tracking_2d_deep_sort.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_detection_3d_voxel.py
Outdated
Show resolved
Hide resolved
Thanks for the changes so far @iliiliiliili, when you feel that all the comments made in the reviews are resolved feel free to re-request 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.
Thanks @iliiliiliili for the fixes so far, there are some stuff we missed the first time around.
projects/opendr_ws/src/perception/scripts/object_tracking_2d_deep_sort.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_2d_deep_sort.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_2d_deep_sort.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_detection_3d_voxel.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_3d_ab3dmot.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_3d_ab3dmot.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_3d_ab3dmot.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Kostas Tsampazis <[email protected]>
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 @iliiliiliili, the nodes look great, apart from some very minor comments.
projects/opendr_ws/src/perception/scripts/object_tracking_2d_deep_sort.py
Outdated
Show resolved
Hide resolved
projects/opendr_ws/src/perception/scripts/object_tracking_2d_fair_mot.py
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.
I've addressed the remaining minor details, it looks good to me. Thank you
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 @ad-daniel and @iliiliiliili. Note that the readme is updated and overhauled in #316 for all nodes.
* Fix ros1 3d detection * Fix ros1 tracking 3d node * Fix ros1 tracking 2d fairmot node * Fix ros1 tracking 2d deep sort node * Fix ros1 point cloud dataset node * Fix ros1 image dataset node * Fix style errors * Fix point cloud dataset init * Fix image dataset init node * Fix fair mot init * Fix image format rgb8 to bgr8 * Fix 3d detection init node * Fix re-download of the dataset * Fix fairmot conditional computing and typing * Fix deepsort init node * Fix ab3dmot init node * Add point cloud dataset anonymous node * Fix typo * Fix phrasing * Remove extra print * Apply suggestions from code review Co-authored-by: Kostas Tsampazis <[email protected]> * Fix sources * Fix names * Add topic comments * Move fairmot to rgb-based * Fix deep sort direction incompatibility * Fix bounding box frame reference * Exlude opendr_ws devel from source check * Optimize deep sort topic computations * Fixes * More fixes Co-authored-by: Illia Oleksiienko <[email protected]> Co-authored-by: Kostas Tsampazis <[email protected]> Co-authored-by: ad-daniel <[email protected]> Co-authored-by: ad-daniel <[email protected]>
Fixed:
object_detection_3d_voxel_node.py
,point_cloud_dataset_node.py
object_tracking_3d_ab3dmot_node.py
object_tracking_2d_fair_mot_node.py
,object_tracking_2d_deep_sort_node.py
,image_dataset_node.py