feat: Add an option to use still output in RGB node - #783
Conversation
|
@moratom friendly ping |
|
@asahtik Is this branch still maintained? |
|
Hi @bjsowa, this branch was merged with |
|
|
Sorry, my bad. There's no plan to continue active development of v2 based packages, although I will still review PRs. Currently I have a bit of a backlog, but I'm planning on taking care of this and other PRs in early August. |
There was a problem hiding this comment.
Pull request overview
Adds optional support in the RGB (ColorCamera) node to publish the DepthAI still stream and trigger still captures via a ROS 2 service, extending the existing RGB/preview outputs with a higher-resolution still pathway.
Changes:
- Adds
i_enable_stillplusi_still_width/i_still_heightparameters and configuresColorCamera::setStillSize(). - Creates a new XLink output +
ImagePublisherfor the still stream and publishes it under a/still/image_rawsuffix. - Adds a
std_srvs/srv/Triggerservice to request still capture viaCameraControl.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| depthai_ros_driver/src/param_handlers/sensor_param_handler.cpp | Declares still-related parameters and configures setStillSize(). |
| depthai_ros_driver/src/dai_nodes/sensors/rgb.cpp | Adds still XLink output/publisher setup and a Trigger service to request still capture. |
| depthai_ros_driver/include/depthai_ros_driver/dai_nodes/sensors/rgb.hpp | Declares still publisher/service members and callback. |
| depthai_ros_driver/CMakeLists.txt | Adds std_srvs to sensor node dependencies for the new service. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| int stillWidth = declareAndLogParam<int>("i_still_width", width); | ||
| int stillHeight = declareAndLogParam<int>("i_still_height", height); | ||
| colorCam->setStillSize(stillWidth, stillHeight); |
There was a problem hiding this comment.
Agreed, still should use the sensor size by default
There was a problem hiding this comment.
Still resolution cannot be larger than isp output resolution (which is held in width and height variables at this point in code). Running the driver with default parameters out with this modification results in:
what(): ColorCamera(0) - 'still' width or height (1920, 1080) bigger than maximum at current sensor resolution (1280, 720)
| utils::ImgConverterConfig convConfig; | ||
| convConfig.tfPrefix = tfPrefix; | ||
| convConfig.getBaseDeviceTimestamp = ph->getParam<bool>("i_get_base_device_timestamp"); | ||
| convConfig.updateROSBaseTimeOnRosMsg = ph->getParam<bool>("i_update_ros_base_time_on_ros_msg"); | ||
|
|
There was a problem hiding this comment.
The encoding option has only effect when i_low_bandwidth is enabled which I purposefully omitted for still output. For raw frames, Image converter always outputs it in BGR color order.
| pubConfig.width = ph->getParam<int>("i_still_width"); | ||
| pubConfig.height = ph->getParam<int>("i_still_height"); | ||
| pubConfig.maxQSize = ph->getParam<int>("i_max_q_size"); | ||
| pubConfig.topicSuffix = "/still/image_raw"; | ||
| pubConfig.flipImage = ph->getParam<bool>("i_flip_published_image"); |
There was a problem hiding this comment.
I'm not sure about that. Adding this option would make it so that enabling compression for video output would also enable lossy compression for still output which is often not wanted.
Overview
Author: Błażej Sowa
Adds optional support for publishing the ColorCamera “still” output from the RGB node
Changes
ROS distro: jazzy
List of changes:
i_enable_stillplusi_still_width/i_still_heightparameters for the ColorCamera node.std_srvs/srv/Triggerservice to request a still capture viaCameraControl.Testing
Hardware used: Oak-1-MAX, Oak-1-Lite-W, Oak-D-Pro-W
Depthai library version: 2.31.1