-
Notifications
You must be signed in to change notification settings - Fork 10
use the new libmultisense api #54
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
Conversation
…robotics/multisense_ros2 into malvarado/new_libmultisense_api
| const rclcpp::NodeOptions& options, | ||
| std::unique_ptr<multisense::Channel> channel, | ||
| const std::string& tf_prefix, | ||
| bool use_image_transport, |
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.
nbd: we could maybe do something to help clarify the purpose of the bool args in calling code (eg this would look like Multisense(..., false, true))
| // Device stream control | ||
|
|
||
| size_t num_subscribers(const rclcpp::Node::SharedPtr node, const std::string &topic); | ||
| size_t num_subscribers(const rclcpp::Node* node, const std::string &topic); |
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.
nit: make these methods const, and maybe switch to const Node &?
they are private, so it isn't much of a practical concern
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 believe I started with a const ref, but I need to pass the pointer to the current node via this for things to work properly
| const size_t num_fields = ros_point_cloud.fields.size(); | ||
|
|
||
| // | ||
| // Unfortunately we need to iterate and write our points to the output buffer since we need to |
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.
not sure what the receiving end looks like, but just curious if it can use the field configuration to pull the color channels out of 3 separate uint8 fields instead of 1 float32?
multisense_ros/src/multisense.cpp
Outdated
|
|
||
| // | ||
| // If our main image pair does not support color, make sure we explicitly disable white balance. If | ||
| // we could undeclare the parameter |
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.
is this an unfinished comment?
looks like parameters can be undeclared by setting to rclcpp::PARAMETER_NOT_SET?
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.
Unfortunately rclcpp::PARAMETER_NOT_SET doesn't actually work
Use the new LibMultiSense API as the base for the multisense_ros2 driver.
To properly use this driver you must use MultiSense firmware >= v7.22. See https://docs.carnegierobotics.com/firmware/update.html for update instructions
Notable changes include: