Add Remote Head Support #44
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… incorrect remote head support
| @@ -44,7 +44,7 @@ | |||
| <param name="sensor_ip" value="$(arg ip_address)" /> | |||
| <param name="sensor_mtu" value="$(arg mtu)" /> | |||
| <param name="tf_prefix" value="$(arg vpb_tf_prefix)" /> | |||
| <param name="head_id" value="0" /> | |||
| <param name="head_id" value="-1" /> | |||
There was a problem hiding this comment.
Why are we using a invalid default? Shouldn't we just use 0?
There was a problem hiding this comment.
The -1 default makes this connect with non-remote head cameras and the VPB. 0 will be remote head ID 0
| else { | ||
| if (has_left_camera_ || has_right_camera_ || has_aux_camera_) | ||
| { | ||
| status = driver_->getImageCalibration(image_calibration); |
There was a problem hiding this comment.
will image_calibration be initialized if has_left_camera_ || has_right_camera_ || has_aux_camera_ is not true?
There was a problem hiding this comment.
You're correct here, and I believe it is fixed now. stereo_calibration_manager_ will be nullptr for the vpb (if there is not a left, right, or aux camera)
|
@QuentinTorg is this ready to land? |
Co-authored-by: David Robinson <[email protected]>
…o feature/remote_head_support
…o feature/remote_head_support
| sensorConfigList.append(SensorConfig(name="mono_cmv4000", sgm=False, motor=False, imu=True, lighting=True, aux=False, ar0234=False, features=False, ptp=False)) | ||
| sensorConfigList.append(SensorConfig(name="s27_sgm_AR0234", sgm=True, motor=False, imu=False, lighting=False, aux=True, ar0234=True, features=True, ptp=True)) | ||
| sensorConfigList.append(SensorConfig(name="ks21_sgm_AR0234", sgm=True, motor=False, imu=False, lighting=True, aux=False, ar0234=True, features=True, ptp=True)) | ||
| sensorConfigList.append(SensorConfig(name="remote_head_vpb", sgm=False, motor=False, imu=False, lighting=False, aux=False, ar0234=False, features=True, ptp=True)) |
There was a problem hiding this comment.
Does the VPB actually support features? Also doesn't the VPB have IMU?
mattalvarado
previously approved these changes
Sep 13, 2022
* fix: changed standard to c++17 because of std::shared_mutex
```
In file included from /usr/include/log4cxx/log4cxx.h:45,
from /usr/include/log4cxx/logstring.h:28,
from /usr/include/log4cxx/level.h:22,
from /usr/include/ros/console.h:46,
from /usr/include/ros/ros.h:40,
from /home/fpasqualini/work/minibot/catkin_ws/src/minibot_meta/multisense_ros/multisense_ros/include/multisense_ros/color_laser.h:41,
from /home/fpasqualini/work/minibot/catkin_ws/src/minibot_meta/multisense_ros/multisense_ros/src/color_laser.cpp:36:
/usr/include/log4cxx/boost-std-configuration.h:10:18: error: ‘shared_mutex’ in namespace ‘std’ does not name a type
10 | typedef std::shared_mutex shared_mutex;
| ^~~~~~~~~~~~
/usr/include/log4cxx/boost-std-configuration.h:10:13: note: ‘std::shared_mutex’ is only available from C++17 onwards
10 | typedef std::shared_mutex shared_mutex;
| ^~~
```
Multiple errors like this appear when using GCC 11 and C++ 14
* Update CMakeLists.txt
|
@mattalvarado I think this is now ready to land. It is tested with 5.24 firmware and working well. The following issues have been made for the outstanding TODO's on this PR: |
mattalvarado
approved these changes
Apr 21, 2023
mattalvarado
added a commit
that referenced
this pull request
Apr 21, 2023
* use the latest version of libmultisense * add led pulse width and inversion support * Set external transform on camera (#52) * update libmultisense to latest * Add a URDF for the KS21 (#57) * Importable xacro fixes to remove duplicate root frames and fix typos (#61) * Importable xacro fixes to remove duplicate root frames and fix typos * Remove importable xacro files which are not used by multisense_ros * fix: added robot_state_publisher to multisense_bringup deps (#63) Launch file validation requires all included packages to be dependencies * feat: Made remote heads required nodes (#64) * Add a S30 URDF (#62) * Update exposure limits for nex-gen cameras (#65) * Update the LibMultiSense submodule (#66) * Add Remote Head Support (#44) * initial remote head support, mostly reconfigure updates * clean up reconfigure for mono cameras and remote head * reorganize the topic publishing so stereo and color topics are not advertised if the camera cannot support them * add a launch file for remote head and update urdf with first pass but incorrect remote head support * fix the urdf issues on launch caused by incorrect file paths and names * fix logical error in reconfigure that caused all remote heads to behave like a vpb * update launch files after debugging * update to latest libmultisense version * Update multisense_ros/cfg/multisense.cfg Co-authored-by: David Robinson <[email protected]> * update default head ID in the ros driver * Add grayscale support for pointcloud colorization * account for the possibility of an uninitialized stereo calibration manager * update the valid gain range for AR0234 imagers * fix typo in config * make camera.cpp robust for cameras with no right imager * Fix stereo_calibration_manager logic check * fix: changed standard to c++17 because of std::shared_mutex (#49) * fix: changed standard to c++17 because of std::shared_mutex ``` In file included from /usr/include/log4cxx/log4cxx.h:45, from /usr/include/log4cxx/logstring.h:28, from /usr/include/log4cxx/level.h:22, from /usr/include/ros/console.h:46, from /usr/include/ros/ros.h:40, from /home/fpasqualini/work/minibot/catkin_ws/src/minibot_meta/multisense_ros/multisense_ros/include/multisense_ros/color_laser.h:41, from /home/fpasqualini/work/minibot/catkin_ws/src/minibot_meta/multisense_ros/multisense_ros/src/color_laser.cpp:36: /usr/include/log4cxx/boost-std-configuration.h:10:18: error: ‘shared_mutex’ in namespace ‘std’ does not name a type 10 | typedef std::shared_mutex shared_mutex; | ^~~~~~~~~~~~ /usr/include/log4cxx/boost-std-configuration.h:10:13: note: ‘std::shared_mutex’ is only available from C++17 onwards 10 | typedef std::shared_mutex shared_mutex; | ^~~ ``` Multiple errors like this appear when using GCC 11 and C++ 14 * Update CMakeLists.txt * update libmultisenes version * Update min gain params --------- Co-authored-by: David Robinson <[email protected]> Co-authored-by: Matt Alvarado <[email protected]> Co-authored-by: Frank Pasqualini <[email protected]> --------- Co-authored-by: David Robinson <[email protected]> Co-authored-by: Matt Alvarado <[email protected]> Co-authored-by: Frank Pasqualini <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add full support for the new remote head camera including the VPB, mono heads, and stereo heads.
TODO: