USB camera nodelet, monocular tag detector nodelet, corresponding navigation states #640

alisonryckman · 2024-02-03T18:35:01Z

Summary

#567, #607

What features did you add, bugs did you fix, etc?

New nodelet for long range usb camera
New long range tag detector nodelet
Refactoring tag detection into zed tag and long range tag
LongRangeState, ApproachObjectState, and ApproachTargetBaseState

Did you add documentation to the wiki?

No

How was this code tested?

Tested in sim and with the physical long range camera to verify detection distances

Did you test this in sim?

Yes

Did you test this on the rover?

No

Did you add unit tests?

No

ankithu

didn't have time to finish reviewing the whole thing but here are some initial comments for some of it

launch/long_range_cam.launch

src/navigation/context.py

ankithu

a few more comments. Still not through the whole thing.

src/perception/tag_detector/long_range_cam/long_range_tag_detector.hpp

src/perception/tag_detector/long_range_cam/long_range_tag_detector.processing.cpp

ankithu

a few more comments

src/navigation/context.py

src/navigation/approach_object.py

src/navigation/approach_target_base.py

src/navigation/approach_post.py

src/navigation/long_range.py

src/navigation/search.py

src/navigation/waypoint.py

src/navigation/navigation.py

src/perception/long_range_cam/long_range_cam.cpp

src/navigation/long_range.py

src/navigation/search.py

rbridges12 · 2024-02-10T19:34:59Z

src/perception/long_range_cam/long_range_cam.cpp

+        mImgPub = mNh.advertise<sensor_msgs::Image>("long_range_image", 1);
+        // While dtor not called
+        while (ros::ok()) {
+            cv::VideoCapture mCapture{std::format("v4l2src device=/dev/arducam ! videoconvert ! video/x-raw,width={},height={},format=I420,framerate={}/1 ! appsink", 1920, 1080, 5), cv::CAP_GSTREAMER};


port should be a rosparam

The topic name should also be a rosparam

rbridges12 · 2024-02-10T19:39:03Z

src/perception/long_range_cam/long_range_cam.cpp

+                if (frame.empty()) {
+                    break;
+                }
+                cv::imshow("Sender", frame);


we should add an option to disable this right

i just deleted it lol cuz it'll get published to ros anyway

src/perception/tag_detector/long_range_cam/long_range_tag_detector.hpp

src/navigation/search.py

src/navigation/waypoint.py

src/navigation/context.py

ankithu

few more comments, almost there!

qhdwight · 2024-02-11T23:23:26Z

src/perception/long_range_cam/long_range_cam.cpp

@@ -0,0 +1,86 @@
+#include "long_range_cam.hpp"
+#include <opencv2/imgproc.hpp>


All include should go into the "pch.hpp" file

If you are curious, this is called a pre-compiled header and it speeds up successive compilations.

qhdwight · 2024-02-11T23:24:51Z

src/perception/long_range_cam/long_range_cam.cpp

+        try {
+            mGrabThread = std::jthread(&LongRangeCamNodelet::grabUpdate, this);
+        } catch (std::exception const& e) {
+            NODELET_FATAL("Exception while starting: %s", e.what());


This won't ever get called. Starting a thread always works, and you can't catch exceptions that other threads throw. They have their own stack.

qhdwight · 2024-02-11T23:26:29Z

src/perception/long_range_cam/long_range_cam.cpp

+    }
+
+    void fillImageMessage(cv::Mat& bgra, sensor_msgs::ImagePtr const& msg) {
+        assert(msg);


I would add a couple of asserts. You need to make sure that bgra is continous (i.e. it is not a slice, bc then you can't just memcpy), and more importantly check the opencv mat type

Also, shouldn't bgra be renamed to bgr?

qhdwight · 2024-02-11T23:26:54Z

src/perception/long_range_cam/long_range_cam.cpp

+        }
+    }
+
+    void fillImageMessage(cv::Mat& bgra, sensor_msgs::ImagePtr const& msg) {


bgra can be ref to const

qhdwight · 2024-02-11T23:28:12Z

src/perception/long_range_cam/long_range_cam.cpp

+        mCamInfoPub = mNh.advertise<sensor_msgs::CameraInfo>("long_range_cam/camera_info", 1);
+        mImgPub = mNh.advertise<sensor_msgs::Image>("long_range_image", 1);
+        // While dtor not called
+        cv::VideoCapture mCapture{std::format("v4l2src device=/dev/arducam ! videoconvert ! video/x-raw,width={},height={},format=I420,framerate={}/1 ! appsink", 1920, 1080, 5), cv::CAP_GSTREAMER};


m prefix is only used for member variables

qhdwight · 2024-02-11T23:29:02Z

src/perception/long_range_cam/long_range_cam.cpp

+        while (ros::ok()) {
+            // while (true) {
+            mCapture.read(frame);
+            if (frame.empty()) {


What is the failure condition of cv::VideoCapture? I feel like this should be an exception if empty

qhdwight · 2024-02-11T23:30:25Z

src/perception/long_range_cam/long_range_cam.cpp

+        mCamInfoPub = mNh.advertise<sensor_msgs::CameraInfo>("long_range_cam/camera_info", 1);
+        mImgPub = mNh.advertise<sensor_msgs::Image>("long_range_image", 1);
+        // While dtor not called
+        cv::VideoCapture mCapture{std::format("v4l2src device=/dev/arducam ! videoconvert ! video/x-raw,width={},height={},format=I420,framerate={}/1 ! appsink", 1920, 1080, 5), cv::CAP_GSTREAMER};


I would make the device file configurable, same with the width and height (use rosparams). That way we can use this node for the other USB cams.

qhdwight · 2024-02-11T23:49:56Z

Great job, thank you for getting it working!

The processing files look great.

However I am a little worried about the duplicate code between long_range_tag_detector.cpp and tag_detector.cpp. They are almost identical. Perhaps after SAR I or y'all can refactor this.

qhdwight · 2024-02-11T23:52:52Z

I think a solution could be making a base class between the long range detector and the zed detector. the onInit function could be shared. What do people think?

alisonryckman requested review from qhdwight, ankithu, Emerson-Ramey, emmalinhart and lorraineqiu828 February 3, 2024 18:35

ankithu reviewed Feb 3, 2024

View reviewed changes

qhdwight added perception navigation 2024 labels Feb 4, 2024

qhdwight assigned emmalinhart and lorraineqiu828 Feb 4, 2024

Emerson-Ramey requested review from AjaySum and sbarstys February 4, 2024 18:37

ankithu requested changes Feb 4, 2024

View reviewed changes

ankithu requested changes Feb 9, 2024

View reviewed changes

alisonryckman requested a review from rbridges12 February 10, 2024 18:35

rbridges12 reviewed Feb 10, 2024

View reviewed changes

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/search.py Outdated Show resolved Hide resolved

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/waypoint.py Outdated Show resolved Hide resolved

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/waypoint.py Outdated Show resolved Hide resolved

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/context.py Outdated Show resolved Hide resolved

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/context.py Outdated Show resolved Hide resolved

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/context.py Outdated Show resolved Hide resolved

ankithu reviewed Feb 11, 2024

View reviewed changes

src/navigation/context.py Outdated Show resolved Hide resolved

ankithu requested changes Feb 11, 2024

View reviewed changes

qhdwight reviewed Feb 11, 2024

View reviewed changes

alisonryckman force-pushed the percep-second-cam branch from 9f2d260 to cd237bc Compare February 20, 2024 19:33

qhdwight changed the title ~~Long range cam and long range ARTag detector nodelets + new navigation states~~ USB camera nodelet, monocular tag detector nodelet, corresponding navigation states Mar 3, 2024

qhdwight force-pushed the integration branch from 5873d2b to fe29235 Compare April 3, 2024 18:49

fix overshooting in long range state

b68fa60

qhdwight force-pushed the integration branch 5 times, most recently from 8e721f6 to 65c9dc9 Compare April 19, 2024 04:46

Merge remote-tracking branch 'origin/integration' into percep-second-cam

4f948ef

qhdwight merged commit d44fa8c into integration Apr 29, 2024
1 check failed

qhdwight deleted the percep-second-cam branch April 29, 2024 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

USB camera nodelet, monocular tag detector nodelet, corresponding navigation states #640

USB camera nodelet, monocular tag detector nodelet, corresponding navigation states #640

alisonryckman commented Feb 3, 2024 •

edited by Emerson-Ramey

Loading

ankithu left a comment

ankithu left a comment

ankithu left a comment

rbridges12 Feb 10, 2024

qhdwight Feb 20, 2024

rbridges12 Feb 10, 2024

alisonryckman Feb 11, 2024

ankithu left a comment

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight Feb 11, 2024

qhdwight commented Feb 11, 2024

qhdwight commented Feb 11, 2024

		@@ -0,0 +1,86 @@
		#include "long_range_cam.hpp"
		#include <opencv2/imgproc.hpp>

USB camera nodelet, monocular tag detector nodelet, corresponding navigation states #640

USB camera nodelet, monocular tag detector nodelet, corresponding navigation states #640

Conversation

alisonryckman commented Feb 3, 2024 • edited by Emerson-Ramey Loading

Summary

Did you add documentation to the wiki?

How was this code tested?

Did you test this in sim?

Did you test this on the rover?

Did you add unit tests?

ankithu left a comment

Choose a reason for hiding this comment

ankithu left a comment

Choose a reason for hiding this comment

ankithu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankithu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qhdwight commented Feb 11, 2024

qhdwight commented Feb 11, 2024

alisonryckman commented Feb 3, 2024 •

edited by Emerson-Ramey

Loading