Conversation
|
Is it compatible with the RVC2? - Since the |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRenames sparse→organized semantics and adds length-unit support; implements a PointCloud Pimpl with CPU/CPU_MT/GPU backends, intrinsics/extrinsics and coordinate-system transforms; updates bindings, examples, tests, PCL conversions, serialization, and protobuf emission to match new organized semantics. Changes
Sequence Diagram(s)sequenceDiagram
participant Cam as Camera(s)
participant Stereo as StereoDepth
participant PCNode as PointCloud Node
participant Impl as PointCloud::Impl
participant Backend as Backend (CPU / CPU_MT / GPU)
participant Host as Host / Bindings / Output
Cam->>Stereo: produce depth frames
Stereo->>PCNode: deliver depth buffer
PCNode->>Impl: forward depth & runtime config
Impl->>Backend: compute dense point cloud (organized or filtered)
Backend-->>Impl: return points
Impl->>Impl: apply intrinsics/extrinsics & unit scaling
Impl->>Host: emit PointCloudData (organized flag, bbox, instanceNum)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
MaticTonin
left a comment
MaticTonin
left a comment
There was a problem hiding this comment.
Will require some polishing, but overall it looks OK.
pheec
force-pushed
the
dev_tf_PointCloudNode
branch
2 times, most recently
from
e632c7a to
5fa2556
Compare
pheec
force-pushed
the
dev_tf_PointCloudNode
branch
from
5fa2556 to
e9d7117
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (6)
bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp (1)
173-174:⚠️ Potential issue | 🟡 MinorDuplicate
setInstanceNumbinding and missinggetInstanceNum.
setInstanceNumis defined twice (lines 117 and 173). Remove the duplicate at line 173. Additionally, consider adding a binding forgetInstanceNum()which exists in the C++ API but appears to be missing from the bindings.🐛 Proposed fix
.def("setMaxZ", &PointCloudData::setMaxZ, DOC(dai, PointCloudData, setMaxZ)) - .def("setInstanceNum", &PointCloudData::setInstanceNum, DOC(dai, PointCloudData, setInstanceNum)) + .def("getInstanceNum", &PointCloudData::getInstanceNum, DOC(dai, PointCloudData, getInstanceNum)) .def("updateBoundingBox", &PointCloudData::updateBoundingBox, DOC(dai, PointCloudData, updateBoundingBox));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp` around lines 173 - 174, Remove the duplicate Python binding for PointCloudData::setInstanceNum (the second .def("setInstanceNum", &PointCloudData::setInstanceNum, ...) entry) and instead add a binding for PointCloudData::getInstanceNum so the getter is exposed; locate the block where PointCloudData methods are registered (the .def calls including .def("updateBoundingBox", &PointCloudData::updateBoundingBox, ...)) and replace the duplicate setInstanceNum line with a .def that binds getInstanceNum (using the same DOC(...) pattern).src/utility/ProtoSerialize.cpp (1)
971-978: 🧹 Nitpick | 🔵 TrivialRemove dead commented-out code.
This commented-out block serves no purpose and reduces maintainability. The backward-compatibility intent is already documented in the serialization side. If this code is not needed, remove it entirely.
♻️ Proposed fix
obj.setColor(pcl->color()); - - // // Read sparse flag for backward compatibility, but don't use it - // // Width and height already determine organization - // `#pragma` GCC diagnostic push - // `#pragma` GCC diagnostic ignored "-Wdeprecated-declarations" - // // Ignore the sparse value - we determine organization from height - // (void)pcl->sparse(); - // `#pragma` GCC diagnostic pop - if(!metadataOnly) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/ProtoSerialize.cpp` around lines 971 - 978, The commented-out dead code block in ProtoSerialize.cpp that references pcl->sparse() and surrounding pragma diagnostics should be removed to improve maintainability; delete the entire commented block (the lines containing the commented pragmas and the "(void)pcl->sparse();" comment) so only active serialization logic remains and no leftover backward-compatibility stubs exist.src/pipeline/node/PointCloud.cpp (1)
107-110:⚠️ Potential issue | 🔴 CriticalGuard
useCPUMT(0)before enabling the multi-threaded backend.
computePointCloudDenseCPUMT()divides bythreadNum, souseCPUMT(0)becomes a divide-by-zero on the first frame. Clamp to at least 1 before switching toComputeMethod::CPU_MT.🧵 Suggested fix
void PointCloud::Impl::useCPUMT(uint32_t numThreads) { + if(numThreads == 0) { + if(logger) logger->warn("PointCloud CPU_MT requested with 0 threads, falling back to 1"); + numThreads = 1; + } threadNum = numThreads; computeMethod = ComputeMethod::CPU_MT; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipeline/node/PointCloud.cpp` around lines 107 - 110, Guard against zero threads in PointCloud::Impl::useCPUMT by clamping the incoming numThreads to at least 1 before assigning threadNum and switching computeMethod to ComputeMethod::CPU_MT (e.g., threadNum = std::max(1u, numThreads) or if (numThreads == 0) threadNum = 1), since computePointCloudDenseCPUMT() divides by threadNum and a value of 0 would cause divide-by-zero.bindings/python/src/pipeline/node/PointCloudBindings.cpp (1)
50-54:⚠️ Potential issue | 🟠 MajorKeep the housing overload default consistent across C++ and Python.
include/depthai/pipeline/node/PointCloud.hppdefaultssetTargetCoordinateSystem(HousingCoordinateSystem, bool)touseSpecTranslation = true, but this binding hard-codesfalse. The same call therefore produces different transforms depending on language.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/python/src/pipeline/node/PointCloudBindings.cpp` around lines 50 - 54, The Python binding for PointCloud::setTargetCoordinateSystem currently sets py::arg("useSpecTranslation") = false which differs from the C++ default (true); change the binding in PointCloudBindings.cpp so the py::arg default matches the C++ overload by setting py::arg("useSpecTranslation") = true for the py::overload_cast<HousingCoordinateSystem, bool>(&PointCloud::setTargetCoordinateSystem) declaration, ensuring the HousingCoordinateSystem overload uses the same default behavior as the C++ API.bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cpp (1)
44-45:⚠️ Potential issue | 🟠 MajorKeep a deprecated
setSparse()alias in Python.
getSparse()is still preserved for compatibility, but old Python code callingsetSparse()will now fail immediately withAttributeError. Add a deprecated wrapper that forwards tosetOrganized(!sparse)so existing pipelines get a warning instead of a hard break.🐍 Suggested fix
.def("getLengthUnit", &PointCloudConfig::getLengthUnit, DOC(dai, PointCloudConfig, getLengthUnit)) .def("setLengthUnit", &PointCloudConfig::setLengthUnit, DOC(dai, PointCloudConfig, setLengthUnit)) // Deprecated + .def( + "setSparse", + [](PointCloudConfig& cfg, bool sparse) -> PointCloudConfig& { + PyErr_WarnEx( + PyExc_DeprecationWarning, + "setSparse() is deprecated, use setOrganized() instead (note: sparse == !organized).", + 1); + return cfg.setOrganized(!sparse); + }, + py::arg("sparse"), + "**Deprecated:** Use setOrganized() instead (sparse == !organized).") + .def( + "getSparse", + [](const PointCloudConfig& cfg) { + PyErr_WarnEx(PyExc_DeprecationWarning, "getSparse() is deprecated, use getOrganized() instead (note: sparse == !organized).", 1); + return !cfg.getOrganized(); + }, + "**Deprecated:** Use getOrganized() instead (sparse == !organized)."); - .def( - "getSparse", - [](const PointCloudConfig& cfg) { - PyErr_WarnEx(PyExc_DeprecationWarning, "getSparse() is deprecated, use getOrganized() instead (note: sparse == !organized).", 1); - return !cfg.getOrganized(); - }, - "**Deprecated:** Use getOrganized() instead (sparse == !organized).");Also applies to: 57-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cpp` around lines 44 - 45, Add a deprecated Python alias method named setSparse that forwards to setOrganized(!sparse) and emits a DeprecationWarning instead of removing the attribute; specifically, add a wrapper binding for setSparse that accepts a bool, calls PointCloudConfig::setOrganized(!sparse) (or instance.setOrganized(!sparse)), and issues a Python deprecation warning, and repeat the same wrapper for the other binding block referenced (the duplicate region using getSparse/setSparse) so old Python code gets a warning rather than AttributeError.examples/python/PointCloud/PointCloudShowcase.py (1)
91-95:⚠️ Potential issue | 🟠 MajorRemove hardcoded device IP address.
This example hardcodes a specific device IP (
10.11.0.29), which will fail for any other user. Use the default device discovery instead.🐛 Proposed fix
- # device = dai.Device() - # print(f"Device: {device.getDeviceName()} (ID: {device.getDeviceId()})\n") - deviceIp = "10.11.0.29" - deviceInfo = dai.DeviceInfo(deviceIp) - device = dai.Device(deviceInfo) + device = dai.Device() + print(f"Device: {device.getDeviceName()} (ID: {device.getDeviceId()})\n")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudShowcase.py` around lines 91 - 95, The sample in PointCloudShowcase.py hardcodes deviceIp ("10.11.0.29") and constructs dai.DeviceInfo/device with it; revert to automatic device discovery by removing the hardcoded deviceIp and dai.DeviceInfo usage and instantiate the device using the default constructor (e.g., dai.Device()) or the library's discovery API; restore the previously commented lines (device = dai.Device() and the device.getDeviceName()/getDeviceId() print) and ensure no hardcoded IP remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/cpp/PointCloud/PointCloudShowcase.cpp`:
- Around line 1-50: Add a README.md to the examples/cpp/PointCloud directory
that documents the PointCloudShowcase.cpp example (purpose, how to build/run
it), summarizes the coordinate-system transform options covered in the file
(CameraBoardSocket, HousingCoordinateSystem, custom 4x4 matrix) and their flags,
and include at least one visual (PNG/GIF) or diagram showing the coordinate
frames and transform flow; reference PointCloudShowcase.cpp in the README and
include example command(s) to run the sample and where to find/replace the
visual assets in the directory.
- Around line 70-82: Change the printPointCloudInfo signature to take the
PointCloudData by const reference and update any uses accordingly: make
printPointCloudInfo(const dai::PointCloudData& pcd, int frameNum) and ensure all
accessed methods remain const; because PointCloudData::getPoints() is currently
non-const, update the API to const-qualify getPoints() (e.g.,
PointCloudData::getPoints() const) so the call from printPointCloudInfo compiles
and still returns the same container type without mutation.
In `@examples/cpp/PointCloud/PointCloudVisualizer.cpp`:
- Around line 41-44: Guard against the case where vals has no non-zero elements
before computing percentiles: compute size_t start = std::distance(vals.begin(),
nonZeroIt) and size_t remaining = vals.size() - start, and if vals.empty() or
remaining == 0 set minD and maxD to 0 (or return early) instead of indexing into
vals; otherwise compute minD using vals[start + remaining/100] and maxD using
vals[start + static_cast<size_t>(remaining*0.99) - 1] (or an appropriate clamp)
so you never index out of range when calculating minD/maxD from nonZeroIt, vals,
minD, and maxD.
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 63-66: The print_header function uses f-strings for constant
strings without placeholders; remove the unnecessary f prefix on the first and
third print calls in print_header so only the middle line that interpolates
{title} remains an f-string (i.e., keep f on the print that uses {title} but
drop f from the two constant border strings).
- Line 56: Remove the leftover commented debug line setting
os.environ["DEPTHAI_DEVICE_RVC4_FWP"] in the PointCloudShowcase example; delete
the commented-out environment override (the line starting with #
os.environ["DEPTHAI_DEVICE_RVC4_FWP"] = "...") so the user-facing example
contains no development-only debug code or commented overrides.
In `@examples/python/PointCloud/PointCloudVisualizer.py`:
- Around line 55-58: The preview heatmap should read from the exact depth frame
that produced the emitted point cloud, so replace uses of
stereo.depth.createOutputQueue(...) with
pc.passthroughDepth.createOutputQueue(...) (e.g., change qDepth =
stereo.depth.createOutputQueue(...) to use pc.passthroughDepth and keep the same
maxSize/blocking options), and do the same replacement for the other occurrence
later in the file; update any variable names (qDepth or similar) and downstream
references as needed to reflect the passthrough source.
- Around line 24-31: The percentile computation uses zeros from downscaled for
maxD, causing maxD <= minD on sparse frames; change the code that sets maxD to
compute from the valid nonZero array (i.e., use np.percentile(nonZero, 99)
instead of np.percentile(downscaled, 99)) so both minD and maxD come from
non-zero pixels (variables: downscaled, nonZero, minD, maxD, frame, colored);
also ensure downstream interpolation remains robust by keeping the existing
branch that handles nonZero.size == 0.
In `@include/depthai/pipeline/datatype/PointCloudConfig.hpp`:
- Around line 33-35: The docstring for the transform in PointCloudConfig is
incorrect about layout; update the comment on the transform getter in
PointCloudConfig (the method that "Retrieve[s] transformation matrix applied to
every output point") to state it is a 4x4 row-major transformation matrix
(identity by default) so it matches how
src/pipeline/node/PointCloud.cpp::transformPointsCPU() applies matrix rows to
coordinates; ensure the description explicitly says "row-major" and retains the
4x4 identity-by-default note.
In `@include/depthai/pipeline/datatype/PointCloudData.hpp`:
- Around line 23-29: The PointCloudData struct's numeric members (width, height,
minx, miny, minz, maxx, maxy, maxz) are uninitialized; update their declarations
(or the PointCloudData constructor if one exists) to set safe defaults (e.g.,
width=0, height=0, minx/miny/minz=FLT_MAX or 0 and maxx/maxy/maxz=FLT_MIN or 0
depending on intended semantics) so reads can't observe undefined values; ensure
consistency with existing instanceNum and color initializers and use the same
initialization style for all members (initialize width, height, minx, miny,
minz, maxx, maxy, maxz).
In `@src/pipeline/datatype/PointCloudData.cpp`:
- Around line 159-201: Split the compact triple-assignment at the end of
PointCloudData::updateBoundingBox() into clearer separate statements: replace
the single-line "minx = mnX; miny = mnY; minz = mnZ;" and the single-line "maxx
= mxX; maxy = mxY; maxz = mxZ;" with one assignment per line (minx = mnX; miny =
mnY; minz = mnZ; → each on its own line, and similarly for maxx/maxy/maxz) to
improve readability while keeping the same behavior.
In `@src/pipeline/node/PointCloud.cpp`:
- Around line 397-409: initialize() currently always reads the transformation
matrix from initialConfig, which reverts runtime updates; change it to use the
active runtime config (e.g. use latestConfig if non-null/updated, otherwise fall
back to initialConfig) when handling CoordinateSystemType::NONE so the custom
matrix applied at runtime via inputConfig persists; replace uses of
initialConfig->getTransformationMatrix() in initialize() (both the shown block
and the similar block at 436-458) with a computed activeConfig pointer
(latestConfig ?: initialConfig or equivalent) and call getTransformationMatrix()
on that before logging and calling pimplPointCloud->setExtrinsics(...).
- Around line 245-252: When tensorsInitialized is true, the code currently only
calls depthTensor->setData(depthDataFloat) and leaves intrinsicsTensor with
stale fx/fy/cx/cy/scale/width/height; update the GPU intrinsics state by also
calling intrinsicsTensor->setData(intrinsics) (and refresh xyzTensor data if
appropriate) whenever reusing existing tensors; apply the same fix in the other
reuse site that mirrors this pattern so runtime calibration or unit changes
update intrinsicsTensor (and xyzTensor) instead of only depthTensor.
- Around line 131-143: In PointCloud::Impl::setExtrinsics the code only
validates transformMatrix.size()==4 and transformMatrix[0].size()==4, which
misses malformed second/third/fourth rows; update setExtrinsics to validate
every row by iterating over transformMatrix (or indices 0..3) and ensure each
row.size()==4 (throw std::runtime_error if any row is wrong, include the bad row
index in the message), then assign extrinsics and hasExtrinsics and proceed with
logging as before so logging/usage won't read past a short row.
- Around line 489-493: The current sequence computes densePoints, applies
applyTransformation, then calls filterValidPoints which filters by transformed z
and thus can drop originally valid depth samples; to fix, call
pimplPointCloud->filterValidPoints on the computed densePoints before applying
the coordinate transform so validity is determined in the depth/raw frame (i.e.
computePointCloudDense -> auto filtered = filterValidPoints(densePoints) ->
pimplPointCloud->applyTransformation(filtered) -> assign points = filtered),
referencing computePointCloudDense, filterValidPoints, applyTransformation, and
points.
In `@tests/src/onhost_tests/point_cloud_test.cpp`:
- Around line 38-47: The function makeDepthGradient computes (row / (h - 1))
which is undefined when h == 1; update makeDepthGradient to guard against h <= 1
by either asserting the precondition (add `#include` <cassert> and assert(h > 1))
or by handling the single-row case explicitly (compute depth using minDepth when
h == 1), and ensure any casted arithmetic still yields the intended uint16_t
range; reference function name makeDepthGradient when applying the change.
---
Duplicate comments:
In `@bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cpp`:
- Around line 44-45: Add a deprecated Python alias method named setSparse that
forwards to setOrganized(!sparse) and emits a DeprecationWarning instead of
removing the attribute; specifically, add a wrapper binding for setSparse that
accepts a bool, calls PointCloudConfig::setOrganized(!sparse) (or
instance.setOrganized(!sparse)), and issues a Python deprecation warning, and
repeat the same wrapper for the other binding block referenced (the duplicate
region using getSparse/setSparse) so old Python code gets a warning rather than
AttributeError.
In `@bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp`:
- Around line 173-174: Remove the duplicate Python binding for
PointCloudData::setInstanceNum (the second .def("setInstanceNum",
&PointCloudData::setInstanceNum, ...) entry) and instead add a binding for
PointCloudData::getInstanceNum so the getter is exposed; locate the block where
PointCloudData methods are registered (the .def calls including
.def("updateBoundingBox", &PointCloudData::updateBoundingBox, ...)) and replace
the duplicate setInstanceNum line with a .def that binds getInstanceNum (using
the same DOC(...) pattern).
In `@bindings/python/src/pipeline/node/PointCloudBindings.cpp`:
- Around line 50-54: The Python binding for
PointCloud::setTargetCoordinateSystem currently sets
py::arg("useSpecTranslation") = false which differs from the C++ default (true);
change the binding in PointCloudBindings.cpp so the py::arg default matches the
C++ overload by setting py::arg("useSpecTranslation") = true for the
py::overload_cast<HousingCoordinateSystem,
bool>(&PointCloud::setTargetCoordinateSystem) declaration, ensuring the
HousingCoordinateSystem overload uses the same default behavior as the C++ API.
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 91-95: The sample in PointCloudShowcase.py hardcodes deviceIp
("10.11.0.29") and constructs dai.DeviceInfo/device with it; revert to automatic
device discovery by removing the hardcoded deviceIp and dai.DeviceInfo usage and
instantiate the device using the default constructor (e.g., dai.Device()) or the
library's discovery API; restore the previously commented lines (device =
dai.Device() and the device.getDeviceName()/getDeviceId() print) and ensure no
hardcoded IP remains.
In `@src/pipeline/node/PointCloud.cpp`:
- Around line 107-110: Guard against zero threads in PointCloud::Impl::useCPUMT
by clamping the incoming numThreads to at least 1 before assigning threadNum and
switching computeMethod to ComputeMethod::CPU_MT (e.g., threadNum = std::max(1u,
numThreads) or if (numThreads == 0) threadNum = 1), since
computePointCloudDenseCPUMT() divides by threadNum and a value of 0 would cause
divide-by-zero.
In `@src/utility/ProtoSerialize.cpp`:
- Around line 971-978: The commented-out dead code block in ProtoSerialize.cpp
that references pcl->sparse() and surrounding pragma diagnostics should be
removed to improve maintainability; delete the entire commented block (the lines
containing the commented pragmas and the "(void)pcl->sparse();" comment) so only
active serialization logic remains and no leftover backward-compatibility stubs
exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19cbc522-47d7-4d15-8ef4-16e988c53363
📒 Files selected for processing (24)
bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cppbindings/python/src/pipeline/datatype/PointCloudDataBindings.cppbindings/python/src/pipeline/node/PointCloudBindings.cppcmake/Depthai/DepthaiDeviceRVC4Config.cmakeexamples/cpp/CMakeLists.txtexamples/cpp/PointCloud/CMakeLists.txtexamples/cpp/PointCloud/PointCloudShowcase.cppexamples/cpp/PointCloud/PointCloudVisualizer.cppexamples/python/CMakeLists.txtexamples/python/PointCloud/PointCloudShowcase.pyexamples/python/PointCloud/PointCloudVisualizer.pyinclude/depthai/pipeline/datatype/PointCloudConfig.hppinclude/depthai/pipeline/datatype/PointCloudData.hppinclude/depthai/pipeline/node/PointCloud.hppsrc/pcl/PointCloudData.cppsrc/pipeline/datatype/PointCloudConfig.cppsrc/pipeline/datatype/PointCloudData.cppsrc/pipeline/node/PointCloud.cppsrc/rtabmap/RTABMapSLAM.cppsrc/utility/ProtoSerialize.cpptests/CMakeLists.txttests/src/ondevice_tests/pointcloud_test.cpptests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpptests/src/onhost_tests/point_cloud_test.cpp
💤 Files with no reviewable changes (1)
- src/rtabmap/RTABMapSLAM.cpp
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (11)
examples/python/PointCloud/PointCloudShowcase.py (1)
89-93:⚠️ Potential issue | 🟠 MajorRemove the hard-coded device address from this example.
The sample only works when a device happens to be reachable at
10.11.0.29, which makes the public example fail for USB users and most PoE setups. Default to normal device discovery, or accept the address from user input instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudShowcase.py` around lines 89 - 93, The example currently hard-codes deviceIp = "10.11.0.29" and constructs dai.DeviceInfo/device with that address, which breaks for most users; remove the hard-coded address and either use the default dai.Device() constructor to rely on normal discovery (replace the dai.Device(deviceInfo) code with dai.Device()) or add simple user input/argparse to accept a --device-ip/positional argument and only construct dai.DeviceInfo(deviceIp) when the user provides an address; update variables deviceIp and deviceInfo and the dai.Device(...) call accordingly so the example works with USB/discovery by default and still supports an override.tests/src/onhost_tests/point_cloud_test.cpp (1)
37-40:⚠️ Potential issue | 🟡 Minor
makeDepthGradient()still has a single-row divide-by-zero path.Line 40 divides by
h - 1, soh == 1is still UB. Either asserth > 1or handle the one-row case explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/onhost_tests/point_cloud_test.cpp` around lines 37 - 40, makeDepthGradient currently divides by (h - 1) which is UB when h == 1; update the function (makeDepthGradient) to guard that case by either asserting h > 1 or explicitly handling h == 1 (e.g., set every row depth to minDepth or to the midpoint) before the loop so the expression static_cast<uint16_t>(minDepth + (maxDepth - minDepth) * row / (h - 1)) never performs a divide-by-zero; modify the depth computation path accordingly to use the guarded denominator or the single-row fallback.examples/cpp/PointCloud/PointCloudVisualizer.cpp (1)
43-46:⚠️ Potential issue | 🟡 MinorGuard the percentile calculation when the frame is empty.
If
downscaledhas no samples, Line 46 indexesvals[0]and hits UB. Return a blank preview or short-circuit before reading percentile indices.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cpp/PointCloud/PointCloudVisualizer.cpp` around lines 43 - 46, The percentile calculation assumes vals (from downscaled samples) is non-empty and can index vals[0], causing UB for empty frames; update the logic in PointCloudVisualizer.cpp where vals, nonZeroIt, minD, and maxD are computed to first check if vals.empty() (or downscaled.empty()) and short-circuit—return a blank preview or skip percentile computation—otherwise proceed to compute nonZeroIt, minD and maxD; ensure any use of vals[...] or std::next is guarded so indices are only computed when vals.size() > 0 and the computed percentile index is within range.bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp (1)
118-118:⚠️ Potential issue | 🟠 MajorRestore
getInstanceNuminstead of registeringsetInstanceNumtwice.There are now two
setInstanceNumbindings (Line 118 and Line 174), but nogetInstanceNumbinding, so Python can writeinstanceNumand no longer read it back. Replace one of these with the getter.Suggested fix
.def("isColor", &PointCloudData::isColor, DOC(dai, PointCloudData, isColor)) .def("setInstanceNum", &PointCloudData::setInstanceNum, py::arg("instanceNum"), DOC(dai, PointCloudData, setInstanceNum)) @@ - .def("setInstanceNum", &PointCloudData::setInstanceNum, DOC(dai, PointCloudData, setInstanceNum)) + .def("getInstanceNum", &PointCloudData::getInstanceNum, DOC(dai, PointCloudData, getInstanceNum)) .def("updateBoundingBox", &PointCloudData::updateBoundingBox, DOC(dai, PointCloudData, updateBoundingBox));Also applies to: 174-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp` at line 118, The binding registers PointCloudData::setInstanceNum twice and omits PointCloudData::getInstanceNum; replace the duplicate .def("setInstanceNum", &PointCloudData::setInstanceNum, ...) with a getter registration .def("getInstanceNum", &PointCloudData::getInstanceNum, DOC(dai, PointCloudData, getInstanceNum)) so Python can read instanceNum; keep one setInstanceNum binding and use the getInstanceNum doc macro for the getter.bindings/python/src/pipeline/node/PointCloudBindings.cpp (1)
50-54:⚠️ Potential issue | 🟠 MajorDefault value mismatch for
useSpecTranslationin HousingCoordinateSystem overload.The C++ declaration at
include/depthai/pipeline/node/PointCloud.hpp:177usesuseSpecTranslation = trueas the default, but this Python binding usesfalse. This inconsistency will cause different behavior between C++ and Python APIs.🐛 Proposed fix
.def("setTargetCoordinateSystem", py::overload_cast<HousingCoordinateSystem, bool>(&PointCloud::setTargetCoordinateSystem), py::arg("housingCS"), - py::arg("useSpecTranslation") = false, + py::arg("useSpecTranslation") = true, DOC(dai, node, PointCloud, setTargetCoordinateSystem, 2));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/python/src/pipeline/node/PointCloudBindings.cpp` around lines 50 - 54, The Python binding for PointCloud::setTargetCoordinateSystem in PointCloudBindings.cpp sets py::arg("useSpecTranslation") = false which conflicts with the C++ signature default true in PointCloud.hpp (line ~177); update the binding to use py::arg("useSpecTranslation") = true so the Python API default matches the C++ declaration for the HousingCoordinateSystem overload.src/pipeline/node/PointCloud.cpp (4)
130-143:⚠️ Potential issue | 🔴 CriticalValidate all four rows of the extrinsics matrix.
The validation only checks
transformMatrix[0].size(). A malformed matrix with a short second, third, or fourth row will pass validation but cause out-of-bounds access in the logging loop andtransformPointsCPU.🐛 Proposed fix
void PointCloud::Impl::setExtrinsics(const std::vector<std::vector<float>>& transformMatrix) { - if(transformMatrix.size() != 4 || transformMatrix[0].size() != 4) { + if(transformMatrix.size() != 4) { throw std::runtime_error("Transformation matrix must be 4x4"); } + for(size_t i = 0; i < 4; ++i) { + if(transformMatrix[i].size() != 4) { + throw std::runtime_error("Transformation matrix row " + std::to_string(i) + " must have 4 elements"); + } + } extrinsics = transformMatrix;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipeline/node/PointCloud.cpp` around lines 130 - 143, The current PointCloud::Impl::setExtrinsics only checks transformMatrix.size() == 4 and transformMatrix[0].size(), allowing rows 1-3 to be shorter and causing OOB in the logging loop and transformPointsCPU; update validation in setExtrinsics to ensure transformMatrix has exactly 4 rows and that each row's size is exactly 4 (check transformMatrix[i].size() for i=0..3), and if any row is invalid throw a descriptive std::runtime_error (e.g., "Transformation matrix must be 4x4"); after this robust check, assign extrinsics, set hasExtrinsics, and proceed with logging as before.
401-417:⚠️ Potential issue | 🔴 CriticalInitialize from runtime config, not always
initialConfig.In
CoordinateSystemType::NONEhandling, the transformation matrix is always read frominitialConfig. After a runtimeinputConfigupdate with a new matrix, callinginitialize()(on calibration change or length unit change) would revert to the initial matrix instead of preserving the runtime update.The fix requires passing the active config to
initialize()or storing it as a member. Consider refactoring to use the samelatestConfigthatrun()maintains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipeline/node/PointCloud.cpp` around lines 401 - 417, The code in CoordinateSystemType::NONE reads the matrix from initialConfig causing runtime updates to be lost; change initialize() to use the active runtime config (the same latestConfig used by run()) or store the current config as a member and call its getTransformationMatrix() instead of initialConfig->getTransformationMatrix(); then pass that matrix to pimplPointCloud->setExtrinsics and keep the same logging via pimpl->logger so runtime inputConfig/calibration changes are preserved after initialize().
490-496:⚠️ Potential issue | 🔴 CriticalFilter sparse points before applying coordinate transformation.
In sparse mode, filtering is applied after transformation. A valid depth point (z > 0 in camera frame) can legitimately become z ≤ 0 in a transformed coordinate system (e.g., when the target frame flips or significantly translates the z-axis). This causes valid points to be incorrectly dropped.
🐛 Proposed fix
} else { // For sparse output, we need to filter, so compute dense first std::vector<Point3f> densePoints; pimplPointCloud->computePointCloudDense(depthData, densePoints); - pimplPointCloud->applyTransformation(densePoints); points = pimplPointCloud->filterValidPoints(densePoints); + pimplPointCloud->applyTransformation(points); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipeline/node/PointCloud.cpp` around lines 490 - 496, In the sparse-output branch, filter points before applying the coordinate transformation to avoid dropping points that are valid in camera frame but become z ≤ 0 after transform; specifically, after pimplPointCloud->computePointCloudDense(depthData, densePoints) call, call pimplPointCloud->filterValidPoints(densePoints) to produce points, then call pimplPointCloud->applyTransformation(points) (not applyTransformation(densePoints)), and assign the transformed result back into points so downstream logic uses the filtered-then-transformed set.
249-256:⚠️ Potential issue | 🔴 CriticalGPU intrinsics tensor not updated on reinitialization.
When
tensorsInitializedis true, onlydepthTensoris updated with new data. If intrinsics or length unit change (triggering reinitialization),intrinsicsTensorretains stalefx/fy/cx/cy/scalevalues, causing incorrect GPU output.🐛 Proposed fix
if(!tensorsInitialized) { depthTensor = mgr->tensor(depthDataFloat); intrinsicsTensor = mgr->tensor(intrinsics); xyzTensor = mgr->tensor(xyzOut); tensorsInitialized = true; } else { depthTensor->setData(depthDataFloat); + intrinsicsTensor->setData(intrinsics); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipeline/node/PointCloud.cpp` around lines 249 - 256, The else branch only updates depthTensor, leaving intrinsicsTensor (and xyzTensor) with stale values; update intrinsicsTensor by calling its setData with the new intrinsics (and also update xyzTensor via setData(xyzOut) if applicable) when tensorsInitialized is true so GPU sees new fx/fy/cx/cy/scale; locate the block using tensorsInitialized, depthTensor, intrinsicsTensor, xyzTensor and replace the single-depth setData path with calls that update all three tensors.include/depthai/pipeline/datatype/PointCloudData.hpp (1)
23-28:⚠️ Potential issue | 🟡 MinorInitialize member variables to avoid undefined behavior.
The members
width,height,minx,miny,minz,maxx,maxy,maxzare declared without initializers. Accessing them before setting could result in undefined behavior.🛡️ Proposed fix
- unsigned int width; // width in pixels (for organized) or number of points (for unorganized) - unsigned int height; // height in pixels (for organized) or 1 (for unorganized) + unsigned int width = 0; // width in pixels (for organized) or number of points (for unorganized) + unsigned int height = 0; // height in pixels (for organized) or 1 (for unorganized) uint32_t instanceNum = 0; // Which source created this frame - float minx, miny, minz; - float maxx, maxy, maxz; + float minx = 0.f, miny = 0.f, minz = 0.f; + float maxx = 0.f, maxy = 0.f, maxz = 0.f;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/depthai/pipeline/datatype/PointCloudData.hpp` around lines 23 - 28, Initialize the uninitialized members in the PointCloudData declaration to avoid UB: give width and height default values (e.g., width = 0, height = 0) and initialize the bounding floats (minx, miny, minz, maxx, maxy, maxz) (e.g., = 0.0f or appropriate sentinel values) alongside the already-initialized instanceNum so that any default-constructed PointCloudData has well-defined values.examples/cpp/PointCloud/PointCloudShowcase.cpp (1)
70-80: 🧹 Nitpick | 🔵 TrivialUse const reference for read-only parameter.
printPointCloudInfodoesn't modifypcd, so the parameter should be a const reference for const-correctness. Note that this requiresPointCloudData::getPoints()to be const-qualified (which it currently is not).♻️ Proposed fix (requires API change)
-static void printPointCloudInfo(dai::PointCloudData& pcd, int frameNum) { +static void printPointCloudInfo(const dai::PointCloudData& pcd, int frameNum) {Note: This change requires
PointCloudData::getPoints()to be const-qualified in the API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cpp/PointCloud/PointCloudShowcase.cpp` around lines 70 - 80, printPointCloudInfo takes a non-const PointCloudData& but does not modify it; change its signature to accept a const PointCloudData& (i.e., static void printPointCloudInfo(const dai::PointCloudData& pcd, int frameNum)) and update all call sites accordingly, and then make PointCloudData::getPoints() a const-qualified method (or provide a const overload) so callers like printPointCloudInfo can call pcd.getPoints() on a const object; ensure any implementation and header changes for getPoints() reflect const-correctness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/depthai/pipeline/node/PointCloud.hpp`:
- Around line 34-98: The Impl class is currently public and exposes
implementation details; change it to be private or forward-declared: replace the
public definition of class Impl with a forward declaration (class Impl;) in the
header and move the full Impl definition into the .cpp, or nest Impl inside the
parent PointCloud class and mark it private; update the header to expose only a
pimpl pointer (e.g., std::unique_ptr<Impl> pimpl) and adjust
constructors/destructor and any places referencing Impl (symbols: Impl,
pimplPointCloud if present, fx/fy/cx/cy, computePointCloudDense,
applyTransformation, filterValidPoints) so compilation uses the opaque pointer
to the private Impl implementation. Ensure includes needed by Impl live in the
.cpp and maintain API compatibility.
- Around line 88-92: The Impl struct in PointCloud.hpp declares fx, fy, cx, cy,
width, height, and size without initializers which can lead to UB if
computePointCloudDense or transformPointsCPU run before setIntrinsics;
initialize these members in-place (e.g., fx=0.0f, fy=0.0f, cx=0.0f, cy=0.0f,
width=0u, height=0u, size=0) to provide safe defaults and keep threadNum's
existing initializer; ensure these defaults are applied where the Impl is
defined so setIntrinsics can still overwrite them as intended.
In `@src/pipeline/datatype/PointCloudConfig.cpp`:
- Around line 34-36: The 3x3 overload of
PointCloudConfig::setTransformationMatrix currently duplicates the assignment;
instead build a 4x4 std::array<std::array<float,4>,4> from the incoming 3x3
(copying the 3x3 into the top-left and filling the last column/row with
{0,0,0,1}) and delegate to the existing 4x4 setter by calling
PointCloudConfig::setTransformationMatrix(the4x4) so there is a single write
path (preserve the returned *this).
In `@src/pipeline/datatype/PointCloudData.cpp`:
- Around line 154-156: The deprecated PointCloudData::setSparse(bool) currently
no-ops and can silently flip object organization causing
getPoints()/getPointsRGB() size assertions to fail; change it to either (A)
convert legacy "sparse" true into the new N×1 representation by setting height =
1 and width = previous point count (or recompute width/height so isOrganized ==
false for sparse), updating any internal buffers accordingly, or (B) fail fast
by throwing or returning an error when called with a value that would change
organization; implement this logic inside PointCloudData::setSparse(bool) and
ensure it interacts correctly with the isOrganized logic (height > 1) and the
code paths in getPoints() and getPointsRGB() so callers no longer experience
silent, implicit reorganization.
- Around line 167-182: The update lambda inside updateBoundingBox() currently
uses the transformed z > 0 test to determine point validity, which is unsafe
after camera/housing/custom transforms; change the logic to base inclusion on
the original depth-validity flag (or original depth value) rather than the
transformed z sign: propagate the original validity when iterating points into
update (or add an explicit bool parameter) and use that to decide whether to
update mnX/mxX, mnY/mxY, mnZ/mxZ and foundValid (leave the min/max updates
intact), so transformed points with z <= 0 are still counted when their source
depth was valid.
In `@tests/src/ondevice_tests/pointcloud_test.cpp`:
- Around line 55-59: The test wrongly assumes sparse output must be smaller than
the organized cloud; update the assertions around the PointCloudData (pcl from
outQ->get<dai::PointCloudData>()) to enforce the N×1 invariant: keep REQUIRE(pcl
!= nullptr), ensure REQUIRE(pcl->getHeight() == 1), replace the width/size
checks with REQUIRE(pcl->getWidth() == pcl->getPoints().size()) and
REQUIRE(pcl->getPoints().size() <= WIDTH * HEIGHT) so width equals the number of
points and the point count never exceeds WIDTH*HEIGHT.
---
Duplicate comments:
In `@bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp`:
- Line 118: The binding registers PointCloudData::setInstanceNum twice and omits
PointCloudData::getInstanceNum; replace the duplicate .def("setInstanceNum",
&PointCloudData::setInstanceNum, ...) with a getter registration
.def("getInstanceNum", &PointCloudData::getInstanceNum, DOC(dai, PointCloudData,
getInstanceNum)) so Python can read instanceNum; keep one setInstanceNum binding
and use the getInstanceNum doc macro for the getter.
In `@bindings/python/src/pipeline/node/PointCloudBindings.cpp`:
- Around line 50-54: The Python binding for
PointCloud::setTargetCoordinateSystem in PointCloudBindings.cpp sets
py::arg("useSpecTranslation") = false which conflicts with the C++ signature
default true in PointCloud.hpp (line ~177); update the binding to use
py::arg("useSpecTranslation") = true so the Python API default matches the C++
declaration for the HousingCoordinateSystem overload.
In `@examples/cpp/PointCloud/PointCloudShowcase.cpp`:
- Around line 70-80: printPointCloudInfo takes a non-const PointCloudData& but
does not modify it; change its signature to accept a const PointCloudData&
(i.e., static void printPointCloudInfo(const dai::PointCloudData& pcd, int
frameNum)) and update all call sites accordingly, and then make
PointCloudData::getPoints() a const-qualified method (or provide a const
overload) so callers like printPointCloudInfo can call pcd.getPoints() on a
const object; ensure any implementation and header changes for getPoints()
reflect const-correctness.
In `@examples/cpp/PointCloud/PointCloudVisualizer.cpp`:
- Around line 43-46: The percentile calculation assumes vals (from downscaled
samples) is non-empty and can index vals[0], causing UB for empty frames; update
the logic in PointCloudVisualizer.cpp where vals, nonZeroIt, minD, and maxD are
computed to first check if vals.empty() (or downscaled.empty()) and
short-circuit—return a blank preview or skip percentile computation—otherwise
proceed to compute nonZeroIt, minD and maxD; ensure any use of vals[...] or
std::next is guarded so indices are only computed when vals.size() > 0 and the
computed percentile index is within range.
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 89-93: The example currently hard-codes deviceIp = "10.11.0.29"
and constructs dai.DeviceInfo/device with that address, which breaks for most
users; remove the hard-coded address and either use the default dai.Device()
constructor to rely on normal discovery (replace the dai.Device(deviceInfo) code
with dai.Device()) or add simple user input/argparse to accept a
--device-ip/positional argument and only construct dai.DeviceInfo(deviceIp) when
the user provides an address; update variables deviceIp and deviceInfo and the
dai.Device(...) call accordingly so the example works with USB/discovery by
default and still supports an override.
In `@include/depthai/pipeline/datatype/PointCloudData.hpp`:
- Around line 23-28: Initialize the uninitialized members in the PointCloudData
declaration to avoid UB: give width and height default values (e.g., width = 0,
height = 0) and initialize the bounding floats (minx, miny, minz, maxx, maxy,
maxz) (e.g., = 0.0f or appropriate sentinel values) alongside the
already-initialized instanceNum so that any default-constructed PointCloudData
has well-defined values.
In `@src/pipeline/node/PointCloud.cpp`:
- Around line 130-143: The current PointCloud::Impl::setExtrinsics only checks
transformMatrix.size() == 4 and transformMatrix[0].size(), allowing rows 1-3 to
be shorter and causing OOB in the logging loop and transformPointsCPU; update
validation in setExtrinsics to ensure transformMatrix has exactly 4 rows and
that each row's size is exactly 4 (check transformMatrix[i].size() for i=0..3),
and if any row is invalid throw a descriptive std::runtime_error (e.g.,
"Transformation matrix must be 4x4"); after this robust check, assign
extrinsics, set hasExtrinsics, and proceed with logging as before.
- Around line 401-417: The code in CoordinateSystemType::NONE reads the matrix
from initialConfig causing runtime updates to be lost; change initialize() to
use the active runtime config (the same latestConfig used by run()) or store the
current config as a member and call its getTransformationMatrix() instead of
initialConfig->getTransformationMatrix(); then pass that matrix to
pimplPointCloud->setExtrinsics and keep the same logging via pimpl->logger so
runtime inputConfig/calibration changes are preserved after initialize().
- Around line 490-496: In the sparse-output branch, filter points before
applying the coordinate transformation to avoid dropping points that are valid
in camera frame but become z ≤ 0 after transform; specifically, after
pimplPointCloud->computePointCloudDense(depthData, densePoints) call, call
pimplPointCloud->filterValidPoints(densePoints) to produce points, then call
pimplPointCloud->applyTransformation(points) (not
applyTransformation(densePoints)), and assign the transformed result back into
points so downstream logic uses the filtered-then-transformed set.
- Around line 249-256: The else branch only updates depthTensor, leaving
intrinsicsTensor (and xyzTensor) with stale values; update intrinsicsTensor by
calling its setData with the new intrinsics (and also update xyzTensor via
setData(xyzOut) if applicable) when tensorsInitialized is true so GPU sees new
fx/fy/cx/cy/scale; locate the block using tensorsInitialized, depthTensor,
intrinsicsTensor, xyzTensor and replace the single-depth setData path with calls
that update all three tensors.
In `@tests/src/onhost_tests/point_cloud_test.cpp`:
- Around line 37-40: makeDepthGradient currently divides by (h - 1) which is UB
when h == 1; update the function (makeDepthGradient) to guard that case by
either asserting h > 1 or explicitly handling h == 1 (e.g., set every row depth
to minDepth or to the midpoint) before the loop so the expression
static_cast<uint16_t>(minDepth + (maxDepth - minDepth) * row / (h - 1)) never
performs a divide-by-zero; modify the depth computation path accordingly to use
the guarded denominator or the single-row fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50e61623-b7d1-4c14-918a-7e9af54af3fb
📒 Files selected for processing (14)
bindings/python/src/pipeline/datatype/PointCloudDataBindings.cppbindings/python/src/pipeline/node/PointCloudBindings.cppexamples/cpp/PointCloud/PointCloudShowcase.cppexamples/cpp/PointCloud/PointCloudVisualizer.cppexamples/python/PointCloud/PointCloudShowcase.pyinclude/depthai/pipeline/datatype/PointCloudData.hppinclude/depthai/pipeline/node/PointCloud.hppsrc/pipeline/datatype/PointCloudConfig.cppsrc/pipeline/datatype/PointCloudData.cppsrc/pipeline/node/PointCloud.cppsrc/utility/ProtoSerialize.cpptests/src/ondevice_tests/pointcloud_test.cpptests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpptests/src/onhost_tests/point_cloud_test.cpp
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
examples/python/PointCloud/PointCloudShowcase.py (1)
46-50:⚠️ Potential issue | 🟠 MajorRemove hardcoded device IP address.
The example still contains a hardcoded device IP
10.11.0.29which was flagged for removal in a previous review. User-facing examples should use the default device discovery pattern shown in the commented code.🐛 Proposed fix: Use default device discovery
- # device = dai.Device() - # print(f"Device: {device.getDeviceName()} (ID: {device.getDeviceId()})\n") - deviceIp = "10.11.0.29" - deviceInfo = dai.DeviceInfo(deviceIp) - device = dai.Device(deviceInfo) + device = dai.Device() + print(f"Device: {device.getDeviceName()} (ID: {device.getDeviceId()})\n")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudShowcase.py` around lines 46 - 50, Remove the hardcoded device IP (deviceIp = "10.11.0.29") and the explicit DeviceInfo instantiation; instead use the default device discovery pattern by creating the device via dai.Device() as shown in the commented code and then log the device name and ID (replace uses of deviceInfo and Device(deviceInfo) with a plain dai.Device() and the subsequent getDeviceName()/getDeviceId() logging).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/cpp/PointCloud/PointCloudShowcase.cpp`:
- Around line 66-71: The comment above the PointCloud creation is misleading: it
claims "multi-threaded, large pool" but the PointCloud node created in pcSparse
does not call useCPU_MT() or setNumFramesPool(); either change the comment to
reflect the current configuration or enable the described behavior by calling
pcSparse->useCPU_MT(true) (or the appropriate overload) and
pcSparse->setNumFramesPool(<desired_pool_size>) on the dai::node::PointCloud
instance (pcSparse) after creation and before linking; keep the existing
initialConfig->setLengthUnit(...) call as-is if you only change the comment.
- Around line 102-105: Remove the redundant comment block that repeats the
housing coordinate system guidance near the pc->setTargetCoordinateSystem
example (mentioning dai::HousingCoordinateSystem::VESA_A and CameraBoardSocket
values); keep the original comment at the earlier location and delete this
duplicate block to reduce clutter while leaving any unique file-level comments
intact.
- Around line 73-79: The comment saying "single-threaded" is misleading because
no threading mode is set on the PointCloud node; either update the comment to
remove "single-threaded" or explicitly set threading if supported. Locate the
PointCloud creation code (pcOrganized =
pipeline.create<dai::node::PointCloud>(), pcOrganized->setRunOnHost(true),
pcOrganized->initialConfig->setOrganized(true)) and change the comment to
something like "organized point cloud (MILLIMETER)" or add the appropriate
threading configuration call on pcOrganized if you intend to enforce
single-threaded execution.
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 63-68: The comment claiming "multi-threaded, large pool" is
inaccurate for the PointCloud node configured as pcSparse; either remove that
phrase or actually enable those features on the PointCloud node. Locate the
pcSparse setup (pipeline.create(dai.node.PointCloud), pcSparse.setRunOnHost,
pcSparse.initialConfig.setLengthUnit,
pcSparse.outputPointCloud.createOutputQueue) and either (A) change the comment
to only describe what is actually configured (e.g., "Sparse point cloud
(METER)") or (B) add the proper PointCloud initialConfig calls to enable
multi-threading and a larger pool using the node’s API before creating the
output queue so the comment matches the implementation.
- Around line 63-68: The Python showcase uses setRunOnHost(False) for PointCloud
nodes while the C++ example sets setRunOnHost(true); update all PointCloud node
configurations created via pipeline.create(dai.node.PointCloud) (e.g., pcSparse
and the other PointCloud variables) to call setRunOnHost(True) instead of False
so the Python example matches the C++ behavior and is consistent across
examples.
- Around line 160-162: The loop using enumerate(zip(customFrames, depthFrames))
should use zip(..., strict=True) to enforce equal-length validation at runtime;
update the for loop in PointCloudShowcase.py where enumerate(zip(customFrames,
depthFrames)) is used (the block that calls print_point_cloud_info and
depth.getWidth/getHeight) to call zip(customFrames, depthFrames, strict=True) so
a ValueError is raised if lengths differ.
---
Duplicate comments:
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 46-50: Remove the hardcoded device IP (deviceIp = "10.11.0.29")
and the explicit DeviceInfo instantiation; instead use the default device
discovery pattern by creating the device via dai.Device() as shown in the
commented code and then log the device name and ID (replace uses of deviceInfo
and Device(deviceInfo) with a plain dai.Device() and the subsequent
getDeviceName()/getDeviceId() logging).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0b5d0ce-fa1b-4427-8b10-1f82af9d68a0
📒 Files selected for processing (3)
examples/cpp/PointCloud/PointCloudShowcase.cppexamples/cpp/PointCloud/README.mdexamples/python/PointCloud/PointCloudShowcase.py
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (2)
examples/cpp/PointCloud/PointCloudShowcase.cpp (3)
examples/cpp/HostNodes/host_camera.cpp (1)
cout(26-60)include/depthai/pipeline/datatype/PointCloudData.hpp (2)
points(44-44)points(45-45)include/depthai/pipeline/node/PointCloud.hpp (5)
points(44-44)points(61-61)device(52-52)device(60-60)device(162-162)
examples/python/PointCloud/PointCloudShowcase.py (3)
include/depthai/pipeline/datatype/PointCloudData.hpp (3)
PointCloudData(39-39)PointCloudData(40-40)PointCloudData(252-254)src/pipeline/datatype/PointCloudData.cpp (23)
PointCloudData(12-12)getPoints(19-37)getPoints(19-19)getWidth(72-74)getWidth(72-72)getHeight(75-77)getHeight(75-75)isOrganized(100-102)isOrganized(100-100)isColor(104-106)isColor(104-104)getMinX(78-80)getMinX(78-78)getMaxX(87-89)getMaxX(87-87)getMinY(81-83)getMinY(81-81)getMaxY(90-92)getMaxY(90-90)getMinZ(84-86)getMinZ(84-84)getMaxZ(93-95)getMaxZ(93-93)include/depthai/pipeline/node/PointCloud.hpp (5)
device(52-52)device(60-60)device(162-162)PointCloud(105-105)PointCloud(106-106)
🪛 Cppcheck (2.20.0)
examples/cpp/PointCloud/PointCloudShowcase.cpp
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingInclude)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 1-1: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
[information] 19-19: Include file
(missingInclude)
[information] 20-20: Include file
(missingInclude)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[error] 23-23: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingInclude)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
🪛 Ruff (0.15.5)
examples/python/PointCloud/PointCloudShowcase.py
[error] 19-19: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 21-21: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 28-28: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
[warning] 28-28: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
[warning] 41-41: Too many statements (70 > 50)
(PLR0915)
[warning] 53-53: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
[warning] 88-88: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF003)
[warning] 160-160: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
[warning] 162-162: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
🔇 Additional comments (5)
examples/cpp/PointCloud/README.md (1)
1-63: LGTM!The README provides comprehensive documentation for the PointCloud examples, including clear explanations of coordinate-system transform modes, build instructions, and representative code snippets. This addresses the previous reviewer's suggestion to add a README with documentation.
examples/cpp/PointCloud/PointCloudShowcase.cpp (2)
28-38: Use const reference for read-only parameter.
printPointCloudInfoonly reads frompcd, so it should take a const reference. Note: This requiresPointCloudData::getPoints()to be const-qualified in the API (as noted in the previous review).
45-174: LGTM!The main function demonstrates a well-structured pipeline with four distinct PointCloud configurations. The frame collection logic with warm-up delay and queue draining is appropriate for example code. Exception handling is in place.
examples/python/PointCloud/PointCloudShowcase.py (2)
18-21: Remove unnecessary f-string prefixes.Lines 19 and 21 use f-strings without placeholders.
109-133: LGTM!The frame collection logic properly drains stale frames after warm-up and collects frames from all queues evenly.
- PointCloudConfig.hpp: fix docstring 'column-major' -> 'row-major' - PointCloudData.hpp: initialize width/height/min*/max* members to 0 - PointCloudVisualizer.cpp: compute maxD from non-zero values only; add empty/all-zero guards before percentile computation - PointCloudShowcase.py: remove hardcoded IP, restore dai.Device(); remove redundant f-prefix from print_header border strings - PointCloudVisualizer.py: use pc.passthroughDepth for depth queue; compute maxD percentile from non-zero samples - PointCloud.cpp: clamp numThreads>=1 in useCPUMT(); validate all 4 rows in setExtrinsics(); update stale intrinsicsTensor when GPU tensors are reused; fix initialize() to accept active config so NONE case uses latestConfig instead of initialConfig; filter sparse points before applying transformation (preserves camera-space z validity) - point_cloud_test.cpp: guard makeDepthGradient against h==1 division-by-zero - PointCloudConfigBindings.cpp: add deprecated setSparse() binding - PointCloudDataBindings.cpp: replace duplicate setInstanceNum with getInstanceNum - PointCloudBindings.cpp: fix HousingCoordinateSystem overload default useSpecTranslation=true (matches C++ API)
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
include/depthai/pipeline/node/PointCloud.hpp (1)
88-90:⚠️ Potential issue | 🔴 CriticalInitialize
Implnumeric state to avoid undefined reads.
fx,fy,cx,cy,width,height, andsizeare uninitialized and can be observed before intrinsics are set on early paths.Suggested patch
- float fx, fy, cx, cy; - unsigned int width, height; - size_t size; + float fx = 0.f, fy = 0.f, cx = 0.f, cy = 0.f; + unsigned int width = 0, height = 0; + size_t size = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/depthai/pipeline/node/PointCloud.hpp` around lines 88 - 90, The Impl struct in PointCloud.hpp leaves numeric fields fx, fy, cx, cy, width, height, and size uninitialized; update Impl by providing sensible defaults (e.g., fx=0.0f, fy=0.0f, cx=0.0f, cy=0.0f and width=0, height=0, size=0) using in-class member initializers or a default Impl() constructor that sets them, so any early reads before intrinsics are set will see defined values; locate the Impl definition within PointCloud.hpp and add these initializers to the corresponding member declarations or constructor.examples/python/PointCloud/PointCloudVisualizer.py (1)
24-31:⚠️ Potential issue | 🟡 MinorGuard against collapsed depth normalization range.
minDandmaxDcan still become equal on low-variance valid frames, which makes the preview unstable/flat. Add a clamp before interpolation (and sample both axes when downscaling).Suggested patch
- downscaled = frame[::4] + downscaled = frame[::4, ::4] nonZero = downscaled[downscaled != 0] if nonZero.size == 0: minD, maxD = 0, 1 else: minD = np.percentile(nonZero, 1) maxD = np.percentile(nonZero, 99) + if maxD <= minD: + maxD = minD + 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudVisualizer.py` around lines 24 - 31, The depth normalization can collapse when minD == maxD or the range is too small; update the downsampling to sample both axes (use a 2D slice like downscaled = frame[::4, ::4]) and after computing minD and maxD (from nonZero percentiles) clamp the range by checking delta = maxD - minD and if delta < a small epsilon (e.g., 1e-3 or a fraction of minD) expand the bounds (for example set minD -= eps/2 and maxD += eps/2 or set maxD = minD + eps) before calling np.interp to compute colored; adjust variables downscaled, nonZero, minD, maxD, and colored accordingly in PointCloudVisualizer.py.examples/python/PointCloud/PointCloudShowcase.py (1)
76-89:⚠️ Potential issue | 🟠 Major
pcCamandpcCustomneed host execution for these settings to take effect.In
src/pipeline/node/PointCloud.cpp,setTargetCoordinateSystem()anduseCPU()only mutate host-side state; they are not written into node properties. WithsetRunOnHost(False), this showcase still prints "camera transform" / "custom transform" demos, but those two nodes won't actually apply the requested behavior.♻️ Proposed fix
- pcCam.setRunOnHost(False) + pcCam.setRunOnHost(True) pcCam.initialConfig.setLengthUnit(dai.LengthUnit.MILLIMETER) pcCam.setTargetCoordinateSystem(dai.CameraBoardSocket.CAM_A) @@ - pcCustom.setRunOnHost(False) + pcCustom.setRunOnHost(True) pcCustom.initialConfig.setLengthUnit(dai.LengthUnit.MILLIMETER) pcCustom.useCPU()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudShowcase.py` around lines 76 - 89, The PointCloud nodes pcCam and pcCustom are configured with host-only settings (setTargetCoordinateSystem and useCPU) but are created with setRunOnHost(False), so those host-side options never take effect; update the PointCloud node creation for pcCam and pcCustom to run on host (remove or change setRunOnHost(False) to setRunOnHost(True) / omit the call) so that setTargetCoordinateSystem(...) on pcCam and useCPU() on pcCustom actually apply; verify the demo still queues outputs via pcCam.outputPointCloud and pcCustom.outputPointCloud as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cpp`:
- Around line 66-72: The deprecated Python binding for
PointCloudConfig::setSparse currently returns None and breaks fluent chaining;
modify the lambda bound to "setSparse" so it still warns but returns the
modified cfg (i.e., after calling cfg.setOrganized(!sparse) return cfg) to
preserve setter-style chaining semantics; locate the binding for "setSparse" in
PointCloudConfigBindings.cpp and change the lambda to return the
PointCloudConfig& (or cfg) instead of void while keeping the PyErr_WarnEx call
and py::arg("sparse")/docstring intact.
In `@examples/cpp/PointCloud/PointCloudVisualizer.cpp`:
- Line 95: The preview uses stereo->depth which can desynchronize from the
published point cloud when frames drop; replace the depth output queue creation
to read from the point cloud node's passthrough depth stream
(pc->passthroughDepth) instead of stereo->depth so preview frames are
synchronized with the published cloud; locate the line creating qDepth from
stereo->depth (auto qDepth = stereo->depth.createOutputQueue(...)) and change it
to create the output queue from pc->passthroughDepth (using the same
createOutputQueue settings) so frame correspondence is maintained.
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 65-99: The output queues qSparse, qOrganized, qCam, qCustom and
qDepth are created as default (potentially blocking/unbounded) which can fill
during the 1s warm-up and cause backpressure; change each createOutputQueue()
call to use bounded non-blocking queues by calling createOutputQueue(maxSize=4,
blocking=False) for qSparse, qOrganized, qCam, qCustom and qDepth so they behave
like PointCloudVisualizer.py and avoid pausing StereoDepth/PointCloud during
warm-up.
In `@src/pipeline/node/PointCloud.cpp`:
- Around line 471-480: The node currently only reinitializes on config/EEPROM
changes; you must also detect changes in incoming frame geometry and force
initialize() when they differ: capture the cached geometry (width/height and
intrinsics fx/fy/cx/cy stored by the node or pimplPointCloud) and compare it to
the new depthFrame’s getWidth()/getHeight() and intrinsic values from inputDepth
or depthFrame; if any dimension or intrinsic component differs, set initialized
= false, update current cached geometry and call initialize(depthFrame,
latestConfig) (similar logic around the existing EEPROM check using
latestEepromId/currentEepromId and the block that sets
pimplPointCloud->setLengthUnit), and apply the same comparison/early-reinit
logic for the other branch referenced at lines 483-495.
- Around line 115-123: The setIntrinsics(float fx, float fy, float cx, float cy,
unsigned int width, unsigned int height) method updates resolution but does not
invalidate or reallocate GPU Kompute tensors, causing
computePointCloudDenseGPU() to reuse an old-sized xyzTensor while xyzOut is
resized, leading to out-of-bounds reads; modify setIntrinsics() to mark or clear
any GPU tensor allocations (e.g., reset/invalidate xyzTensor and related GPU
buffers used by computePointCloudDenseGPU(), such as any Kompute tensor handles
that back xyzOut/xyzTensor) and reset intrinsicsSet as appropriate so that the
next GPU path call will reallocate tensors to the new size before use.
- Around line 197-198: The code currently performs unaligned reads by casting
(depthData + i * 2) to const uint16_t* and dereferencing it (seen in the
PointCloud.cpp depth-reading code), which is undefined on strict-alignment
platforms; change both occurrences (the CPU and GPU paths where depthValue is
obtained, including the second site around the other read) to load the 2-byte
sample via std::memcpy into a local uint16_t variable before converting to
float, and add `#include` <cstring> to the translation unit so memcpy is
available; preserve existing scaling logic (the static_cast<float>(depthValue) *
scale) after the memcpy.
In `@tests/src/onhost_tests/point_cloud_test.cpp`:
- Around line 430-444: The test currently computes min/max locally and then
overwrites the PointCloudData (pcd) bounds directly, bypassing the production
logic; instead stop writing min/max into pcd, call pcd->updateBoundingBox() (the
production method in PointCloudData / src/pipeline/node/PointCloud.cpp) after
populating pcd, then assert pcd's reported minX/minY/minZ/maxX/maxY/maxZ against
the independently computed expected bounds from the loop over out to verify the
real updateBoundingBox() behavior.
---
Duplicate comments:
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 76-89: The PointCloud nodes pcCam and pcCustom are configured with
host-only settings (setTargetCoordinateSystem and useCPU) but are created with
setRunOnHost(False), so those host-side options never take effect; update the
PointCloud node creation for pcCam and pcCustom to run on host (remove or change
setRunOnHost(False) to setRunOnHost(True) / omit the call) so that
setTargetCoordinateSystem(...) on pcCam and useCPU() on pcCustom actually apply;
verify the demo still queues outputs via pcCam.outputPointCloud and
pcCustom.outputPointCloud as before.
In `@examples/python/PointCloud/PointCloudVisualizer.py`:
- Around line 24-31: The depth normalization can collapse when minD == maxD or
the range is too small; update the downsampling to sample both axes (use a 2D
slice like downscaled = frame[::4, ::4]) and after computing minD and maxD (from
nonZero percentiles) clamp the range by checking delta = maxD - minD and if
delta < a small epsilon (e.g., 1e-3 or a fraction of minD) expand the bounds
(for example set minD -= eps/2 and maxD += eps/2 or set maxD = minD + eps)
before calling np.interp to compute colored; adjust variables downscaled,
nonZero, minD, maxD, and colored accordingly in PointCloudVisualizer.py.
In `@include/depthai/pipeline/node/PointCloud.hpp`:
- Around line 88-90: The Impl struct in PointCloud.hpp leaves numeric fields fx,
fy, cx, cy, width, height, and size uninitialized; update Impl by providing
sensible defaults (e.g., fx=0.0f, fy=0.0f, cx=0.0f, cy=0.0f and width=0,
height=0, size=0) using in-class member initializers or a default Impl()
constructor that sets them, so any early reads before intrinsics are set will
see defined values; locate the Impl definition within PointCloud.hpp and add
these initializers to the corresponding member declarations or constructor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ddd2b72b-dde7-45ac-a69b-6caf736520e9
📒 Files selected for processing (11)
bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cppbindings/python/src/pipeline/datatype/PointCloudDataBindings.cppbindings/python/src/pipeline/node/PointCloudBindings.cppexamples/cpp/PointCloud/PointCloudVisualizer.cppexamples/python/PointCloud/PointCloudShowcase.pyexamples/python/PointCloud/PointCloudVisualizer.pyinclude/depthai/pipeline/datatype/PointCloudConfig.hppinclude/depthai/pipeline/datatype/PointCloudData.hppinclude/depthai/pipeline/node/PointCloud.hppsrc/pipeline/node/PointCloud.cpptests/src/onhost_tests/point_cloud_test.cpp
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
examples/python/PointCloud/PointCloudShowcase.py (1)
65-99:⚠️ Potential issue | 🟠 MajorUse bounded non-blocking output queues to avoid warm-up backpressure.
At Line 65, Line 73, Line 83, Line 98, and Line 99, queues are created with defaults while Line 116 intentionally pauses consumption. This can cause avoidable buffering/backpressure and occasional stale-frame/hang behavior in demos.
♻️ Proposed fix
- qSparse = pcSparse.outputPointCloud.createOutputQueue() + qSparse = pcSparse.outputPointCloud.createOutputQueue(maxSize=4, blocking=False) @@ - qOrganized = pcOrganized.outputPointCloud.createOutputQueue() + qOrganized = pcOrganized.outputPointCloud.createOutputQueue(maxSize=4, blocking=False) @@ - qCam = pcCam.outputPointCloud.createOutputQueue() + qCam = pcCam.outputPointCloud.createOutputQueue(maxSize=4, blocking=False) @@ - qCustom = pcCustom.outputPointCloud.createOutputQueue() - qDepth = pcCustom.passthroughDepth.createOutputQueue() + qCustom = pcCustom.outputPointCloud.createOutputQueue(maxSize=4, blocking=False) + qDepth = pcCustom.passthroughDepth.createOutputQueue(maxSize=4, blocking=False)#!/bin/bash set -euo pipefail # Verify queue-creation patterns in PointCloud examples rg -n "createOutputQueue\\(" examples/python/PointCloud -C2 || true # Verify whether API signatures/defaults are documented in repository headers/bindings rg -n "createOutputQueue\\s*\\(" include src bindings -C3 || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudShowcase.py` around lines 65 - 99, The output queues for point clouds are created with default (unbounded/blocking) settings which can cause warm-up backpressure; update the five creations (qSparse from pcSparse.outputPointCloud.createOutputQueue, qOrganized from pcOrganized.outputPointCloud.createOutputQueue, qCam from pcCam.outputPointCloud.createOutputQueue, qCustom from pcCustom.outputPointCloud.createOutputQueue, and qDepth from pcCustom.passthroughDepth.createOutputQueue) to use bounded, non-blocking queues — either by passing an explicit max size and non-blocking flag to createOutputQueue (e.g., maxSize=1, blocking=False) or by calling the queue API methods to set a small max size and setBlocking(False) — so the demo avoids buffering/backpressure when consumers pause.examples/cpp/PointCloud/PointCloudShowcase.cpp (1)
102-105: 🧹 Nitpick | 🔵 TrivialRemove redundant comment block.
This comment duplicates the information already present at lines 86-87. Consider removing it to reduce clutter.
♻️ Proposed fix
auto qCustom = pcCustom->outputPointCloud.createOutputQueue(); auto qDepth = pcCustom->passthroughDepth.createOutputQueue(); - // Note: Housing coordinate system transform is also available, e.g.: - // pc->setTargetCoordinateSystem(dai::HousingCoordinateSystem::VESA_A); - // See the file-level comment at the top for all available - // CameraBoardSocket and HousingCoordinateSystem values. - // ==============================================================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cpp/PointCloud/PointCloudShowcase.cpp` around lines 102 - 105, Remove the redundant comment block that repeats the note about setTargetCoordinateSystem (dai::HousingCoordinateSystem::VESA_A) and the available CameraBoardSocket and HousingCoordinateSystem values; locate the duplicate comment mentioning pc->setTargetCoordinateSystem and delete it so the earlier comment (lines 86–87) remains as the single source of that information, keeping surrounding comments intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/depthai/pipeline/datatype/PointCloudData.hpp`:
- Around line 205-209: Update the doc comment for
PointCloudData::updateBoundingBox to explain different behavior for sparse vs
organized clouds: state that for organized/structured point clouds the method
only considers valid points (z > 0) when recomputing min/max X,Y,Z and sets
bounds to 0 if none are valid, whereas for sparse clouds (e.g., when the
instance's sparse/organized flag indicates sparse) all stored points are treated
as valid regardless of their z value; keep the note about setting bounds to 0
when no valid points exist and reference the PointCloudData::updateBoundingBox
method and the PointCloudData sparse/organized flag so readers can locate the
relevant logic.
In `@src/pipeline/datatype/PointCloudData.cpp`:
- Around line 154-157: PointCloudData::setSparse is intentionally a deprecated
no-op; add a runtime warning inside that method to inform callers when
deprecation warnings are suppressed. Update the function body to emit a one-time
or lightweight log message (using the project's logging facility, e.g.,
spdlog::warn or existing logger) indicating that setSparse is deprecated and
that organization is now determined by height/width, then return *this; keep
behavior unchanged otherwise. Ensure you reference PointCloudData::setSparse so
callers see the message without altering API semantics.
---
Duplicate comments:
In `@examples/cpp/PointCloud/PointCloudShowcase.cpp`:
- Around line 102-105: Remove the redundant comment block that repeats the note
about setTargetCoordinateSystem (dai::HousingCoordinateSystem::VESA_A) and the
available CameraBoardSocket and HousingCoordinateSystem values; locate the
duplicate comment mentioning pc->setTargetCoordinateSystem and delete it so the
earlier comment (lines 86–87) remains as the single source of that information,
keeping surrounding comments intact.
In `@examples/python/PointCloud/PointCloudShowcase.py`:
- Around line 65-99: The output queues for point clouds are created with default
(unbounded/blocking) settings which can cause warm-up backpressure; update the
five creations (qSparse from pcSparse.outputPointCloud.createOutputQueue,
qOrganized from pcOrganized.outputPointCloud.createOutputQueue, qCam from
pcCam.outputPointCloud.createOutputQueue, qCustom from
pcCustom.outputPointCloud.createOutputQueue, and qDepth from
pcCustom.passthroughDepth.createOutputQueue) to use bounded, non-blocking queues
— either by passing an explicit max size and non-blocking flag to
createOutputQueue (e.g., maxSize=1, blocking=False) or by calling the queue API
methods to set a small max size and setBlocking(False) — so the demo avoids
buffering/backpressure when consumers pause.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d2d65961-723f-405c-813a-99cbae8993ad
📒 Files selected for processing (6)
examples/cpp/PointCloud/PointCloudShowcase.cppexamples/python/PointCloud/PointCloudShowcase.pyinclude/depthai/pipeline/datatype/PointCloudData.hppsrc/pipeline/datatype/PointCloudData.cpptests/src/ondevice_tests/pointcloud_test.cpptests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: pheec
Repo: luxonis/depthai-core PR: 1715
File: include/depthai/pipeline/node/PointCloud.hpp:34-98
Timestamp: 2026-03-16T11:17:08.485Z
Learning: In `include/depthai/pipeline/node/PointCloud.hpp`, the nested `Impl` class inside `PointCloud` is intentionally public (not private/forward-declared) because the on-host unit tests in `tests/src/onhost_tests/point_cloud_test.cpp` directly access `Impl` methods such as `setIntrinsics`, `setExtrinsics`, `useCPU`, `useCPUMT`, `computePointCloudDense`, `applyTransformation`, and `filterValidPoints`. Do not suggest making `Impl` private or forward-declaring it.
📚 Learning: 2026-03-16T11:17:08.485Z
Learnt from: pheec
Repo: luxonis/depthai-core PR: 1715
File: include/depthai/pipeline/node/PointCloud.hpp:34-98
Timestamp: 2026-03-16T11:17:08.485Z
Learning: In `include/depthai/pipeline/node/PointCloud.hpp`, the nested `Impl` class inside `PointCloud` is intentionally public (not private/forward-declared) because the on-host unit tests in `tests/src/onhost_tests/point_cloud_test.cpp` directly access `Impl` methods such as `setIntrinsics`, `setExtrinsics`, `useCPU`, `useCPUMT`, `computePointCloudDense`, `applyTransformation`, and `filterValidPoints`. Do not suggest making `Impl` private or forward-declaring it.
Applied to files:
examples/cpp/PointCloud/PointCloudShowcase.cpptests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cppsrc/pipeline/datatype/PointCloudData.cppexamples/python/PointCloud/PointCloudShowcase.pyinclude/depthai/pipeline/datatype/PointCloudData.hpptests/src/ondevice_tests/pointcloud_test.cpp
📚 Learning: 2026-03-13T19:40:47.231Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1718
File: src/pipeline/node/AutoCalibration.cpp:327-330
Timestamp: 2026-03-13T19:40:47.231Z
Learning: In `src/pipeline/node/AutoCalibration.cpp`, the hard-coded 1280x800 resolution check in `AutoCalibration::validateIncomingData()` is intentional. The AutoCalibration node in the depthai-core (C++) project is designed to support only 1280x800 sensor resolution and this constraint should not be flagged as a usability issue in future reviews.
Applied to files:
tests/src/ondevice_tests/pointcloud_test.cpp
🧬 Code graph analysis (2)
src/pipeline/datatype/PointCloudData.cpp (1)
include/depthai/pipeline/datatype/PointCloudData.hpp (5)
points(44-44)points(45-45)width(116-116)width(131-131)height(123-123)
tests/src/ondevice_tests/pointcloud_test.cpp (1)
include/depthai/device/CrashDump.hpp (1)
RVC2(200-202)
🪛 Cppcheck (2.20.0)
examples/cpp/PointCloud/PointCloudShowcase.cpp
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingInclude)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 1-1: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
[information] 19-19: Include file
(missingInclude)
[information] 20-20: Include file
(missingInclude)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[error] 23-23: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingInclude)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 9-9: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
tests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpp
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 7-7: Include file
(missingIncludeSystem)
[information] 8-8: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 2-2: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 6-6: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 5-5: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[error] 95-95: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
src/pipeline/datatype/PointCloudData.cpp
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingInclude)
tests/src/ondevice_tests/pointcloud_test.cpp
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
[information] 19-19: Include file
(missingInclude)
[information] 20-20: Include file
(missingInclude)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 14-14: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 11-11: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[error] 23-23: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 10-10: Include file
(missingIncludeSystem)
[information] 12-12: Include file
(missingIncludeSystem)
[information] 13-13: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingInclude)
[information] 10-10: Include file
(missingIncludeSystem)
🪛 Ruff (0.15.5)
examples/python/PointCloud/PointCloudShowcase.py
[warning] 28-28: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
[warning] 28-28: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
[warning] 41-41: Too many statements (69 > 50)
(PLR0915)
[warning] 50-50: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
[warning] 85-85: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF003)
[warning] 159-159: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
🔇 Additional comments (13)
examples/python/PointCloud/PointCloudShowcase.py (1)
157-157: Good defensive pairing withzip(..., strict=True).This is a solid correctness safeguard for synchronized point-cloud/depth frame reporting.
examples/cpp/PointCloud/PointCloudShowcase.cpp (1)
1-174: Well-structured showcase example.The example effectively demonstrates the PointCloud node's key features: organized vs filtered output, length units, camera-to-camera transforms, and custom 4×4 matrix transforms. The frame draining approach (lines 119-129) appropriately handles back-pressure during warm-up.
tests/src/ondevice_tests/pointcloud_test.cpp (3)
10-24: LGTM: Clear parameter semantics.The renamed
organizedparameter and conditional configuration logic (setOrganized(true)when organized) align well with the new API semantics.
26-43: LGTM: Organized test with platform guard.The RVC2 skip guard is appropriate since PointCloud node is not supported on that platform. The assertions correctly validate organized cloud invariants (width×height structure with all points present).
46-63: LGTM: Sparse test with corrected assertions.The sparse test now correctly validates the N×1 invariant:
height == 1,width == points.size(), andpoints.size() <= WIDTH * HEIGHT. This addresses the previous concern about incorrectly requiring sparse output to lose points.tests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpp (3)
1-24: LGTM: Well-organized test file with helper function.Good use of an anonymous namespace for the
makePointGridhelper. The test file provides comprehensive coverage of the PointCloudData API.
117-161: Thorough bounding box edge case coverage.These tests effectively validate the distinct behavior between sparse (height==1) and organized (height>1) point clouds:
- Sparse clouds include all points regardless of z sign (lines 117-146)
- Organized clouds skip z≤0 points as invalid (lines 148-161)
This aligns with the implementation's rationale that sparse clouds have already been filtered at creation time.
309-317: LGTM: Compile-time size assertions.The
static_assertchecks ensurePoint3f(12 bytes) andPoint3fRGBA(16 bytes) maintain their expected memory layouts, which is critical for correctreinterpret_castoperations in the implementation.src/pipeline/datatype/PointCloudData.cpp (2)
19-37: LGTM: Const-correct getPoints with dual-path handling.The method correctly handles both color (extracting XYZ from Point3fRGBA) and non-color data paths. The assertions at lines 33-34 enforce the organized/sparse invariants at runtime.
159-212: LGTM: updateBoundingBox with appropriate validity logic.The implementation correctly distinguishes between sparse and organized clouds:
- Sparse (
allPointsValid = true): All stored points are included since they passed validity filtering during creation- Organized (
allPointsValid = false): Usesz > 0heuristic since original depth validity info is not preservedThe comment at lines 167-170 clearly documents this rationale.
include/depthai/pipeline/datatype/PointCloudData.hpp (3)
23-28: LGTM: Proper member initialization.All numeric members are now initialized to safe defaults, preventing undefined behavior if accessed before being set.
92-104: LGTM: Clear deprecation and new API.The deprecation message for
isSparse()clearly guides users to the replacement (isOrganized()), and the relationship is documented (isSparse == !isOrganized()). TheisOrganized()documentation correctly explains the distinction between organized (width×height structure) and sparse (height==1, valid points only) clouds.
255-255: LGTM: Serialization excludes removed sparse member.The
DEPTHAI_SERIALIZEmacro correctly lists only the relevant fields, excluding the removedsparsemember.
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pipeline/datatype/PointCloudData.cpp (1)
21-29:⚠️ Potential issue | 🟡 MinorColor
getPoints()path misses shape invariant checks.Line 21–Line 29 returns converted XYZ points without the width/height assertions used in other getters, so malformed metadata is not caught consistently in debug builds.
Suggested fix
if(isColor()) { span<const Point3fRGBA> pointData(reinterpret_cast<Point3fRGBA*>(data->getData().data()), data->getData().size() / sizeof(Point3fRGBA)); std::vector<Point3fRGBA> points(pointData.begin(), pointData.end()); std::vector<Point3f> points3f; for(const auto& p : points) { points3f.push_back({p.x, p.y, p.z}); } + assert(isOrganized() || points3f.size() <= width); + assert(!isOrganized() || points3f.size() == width * height); return points3f; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipeline/datatype/PointCloudData.cpp` around lines 21 - 29, The color branch in getPoints() (guarded by isColor()) converts Point3fRGBA payload to Point3f but omits the width/height shape invariant checks used elsewhere; add the same metadata validation/assertions (the width/height shape checks used by the non-color getters) before interpreting data->getData(), so malformed buffer/metadata is rejected consistently. Locate the isColor() block and insert the identical shape-invariant validation call(s)/assert(s) (the same functions or checks other getters use), then proceed to reinterpret_cast<Point3fRGBA*> and convert to Point3f as shown.
♻️ Duplicate comments (3)
examples/python/PointCloud/PointCloudVisualizer.py (1)
22-32:⚠️ Potential issue | 🟡 MinorIncomplete 2D downscaling - only first axis is sliced.
Line 24 uses
frame[::4]which only takes every 4th row but keeps all columns. For proper downscaling of a 2D depth frame, both dimensions should be sliced:- downscaled = frame[::4] + downscaled = frame[::4, ::4]Without this, the percentile calculation still processes significantly more data than intended, and on images with sparse valid pixels concentrated in certain columns, the sampling may be less representative.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/python/PointCloud/PointCloudVisualizer.py` around lines 22 - 32, In colorize_depth, the downscaling currently uses frame[::4] which only samples rows; change the sampling to both axes (e.g., use frame[::4, ::4]) so that downscaled and consequently nonZero reflect a true 2D subsample of the depth frame; update the downscaled variable and percentile computation (minD/maxD) to use this 2D subsample so the interpolation and colormap reflect representative depth statistics.examples/cpp/PointCloud/PointCloudShowcase.cpp (1)
102-105: 🧹 Nitpick | 🔵 TrivialRemove duplicated housing-transform guidance block.
This repeats the note already present near Line 86–Line 88 and can be dropped for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cpp/PointCloud/PointCloudShowcase.cpp` around lines 102 - 105, Remove the duplicated housing-transform guidance comment block that repeats the note about setting the target coordinate system; specifically delete the second comment referencing pc->setTargetCoordinateSystem(dai::HousingCoordinateSystem::VESA_A) and the surrounding lines mentioning CameraBoardSocket and HousingCoordinateSystem so only the original guidance remains (leave the earlier occurrence intact).tests/src/onhost_tests/point_cloud_test.cpp (1)
430-444:⚠️ Potential issue | 🟠 MajorBounding-box assertions here bypass the production path.
Line 443 and Line 444 write computed bounds back into
pcd, so Line 447–Line 450 only validate the local loop, notPointCloudData::updateBoundingBox()behavior.Suggested fix
- // Compute bounding box ourselves and set it on the message + // Compute expected bounding box independently float minX = std::numeric_limits<float>::max(), maxX = std::numeric_limits<float>::lowest(); float minY = minX, maxY = maxX, minZ = minX, maxZ = maxX; for(const auto& p : out) { if(p.z > 0.f) { minX = std::min(minX, p.x); maxX = std::max(maxX, p.x); minY = std::min(minY, p.y); maxY = std::max(maxY, p.y); minZ = std::min(minZ, p.z); maxZ = std::max(maxZ, p.z); } } - pcd->setMinX(minX).setMinY(minY).setMinZ(minZ); - pcd->setMaxX(maxX).setMaxY(maxY).setMaxZ(maxZ); + pcd->updateBoundingBox(); + REQUIRE(pcd->getMinX() == Catch::Approx(minX)); + REQUIRE(pcd->getMaxX() == Catch::Approx(maxX)); + REQUIRE(pcd->getMinY() == Catch::Approx(minY)); + REQUIRE(pcd->getMaxY() == Catch::Approx(maxY)); + REQUIRE(pcd->getMinZ() == Catch::Approx(minZ)); + REQUIRE(pcd->getMaxZ() == Catch::Approx(maxZ));Also applies to: 447-450
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/onhost_tests/point_cloud_test.cpp` around lines 430 - 444, The test currently computes bounds itself and writes them back into the test subject (pcd), bypassing the production logic; remove the lines that setMinX/setMinY/setMinZ/setMaxX/setMaxY/setMaxZ on pcd and instead exercise the production code by calling PointCloudData::updateBoundingBox() (or the public method on pcd that triggers that computation) using the generated points (out), then assert the resulting pcd bounds; reference pcd, PointCloudData::updateBoundingBox(), and out to locate where to replace the manual bounds assignment with a call to the production update method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cpp`:
- Around line 58-72: The deprecated wrappers for getSparse and setSparse must
propagate failures from PyErr_WarnEx: in the lambdas passed to def("getSparse",
...) and def("setSparse", ...) call PyErr_WarnEx and if it returns < 0 throw
py::error_already_set() before continuing; do this for both the getter lambda
(returning !cfg.getOrganized()) and the setter lambda (returning
cfg.setOrganized(!sparse)) so a promoted DeprecationWarning becomes a Python
exception instead of leaving a pending error.
In `@bindings/python/src/pipeline/datatype/PointCloudDataBindings.cpp`:
- Around line 114-116: The Python binding for PointCloudData::isSparse currently
uses a lambda that bypasses the C++ deprecation; change the shim so it first
emits a Python deprecation warning via PyErr_WarnEx(PyExc_DeprecationWarning,
"...", 1) and if PyErr_WarnEx returns < 0 throw py::error_already_set(); then
return the existing semantics (either call PointCloudData::isSparse() or compute
!self.isOrganized()). Update the binding for "isSparse" to perform the warning +
error check before returning the boolean so Python callers see the deprecation
just like PointCloudConfig.getSparse().
In `@examples/cpp/PointCloud/CMakeLists.txt`:
- Around line 1-2: Swap the two CMake commands so cmake_minimum_required(VERSION
3.10) is called before project(pointcloud_examples); update the order around the
cmake_minimum_required() and project() invocations to ensure policy defaults
(e.g., CMP0048) are set prior to project() being evaluated.
In `@examples/cpp/PointCloud/PointCloudVisualizer.cpp`:
- Around line 23-26: Replace the signal-handler flag type from std::atomic<bool>
to a signal-safe type: declare the shutdown flag as volatile std::sig_atomic_t
(or std::sig_atomic_t) instead of std::atomic<bool>, set it to 1 inside the
signal handler, and test it as non-zero elsewhere; update both the flag
declaration and all places that write/read it (the signal handler function and
the code that polls the flag) to use numeric 0/1 checks so the behavior is
signal-safe and portable.
In `@examples/cpp/PointCloud/README.md`:
- Around line 32-49: Add explicit examples showing the optional
useSpecTranslation boolean when calling setTargetCoordinateSystem so readers can
see how to override defaults; update the Camera board socket snippet to call
setTargetCoordinateSystem(dai::CameraBoardSocket::CAM_A, false) to demonstrate
per-unit calibration vs spec translation and update the Housing coordinate
system snippet to call
setTargetCoordinateSystem(dai::HousingCoordinateSystem::VESA_A, true) to show
using spec translation explicitly, referencing the setTargetCoordinateSystem
method and the CameraBoardSocket and HousingCoordinateSystem enums in the README
text.
In `@examples/python/CMakeLists.txt`:
- Around line 380-385: The pointcloud_visualizer example uses GUI/windowing
(PointCloud/PointCloudVisualizer.py and functions like
o3d.visualization.Visualizer and cv2.imshow), so update the test labels for the
pointcloud_visualizer target by adding the nowindows label to its
dai_set_example_test_labels call (i.e., change
dai_set_example_test_labels(pointcloud_visualizer ondevice rvc4 ci) to include
nowindows) so the CI treats it as non-windowed.
In `@src/pipeline/node/PointCloud.cpp`:
- Around line 514-522: Before treating depthFrame->getData() as packed uint16_t
pixels, validate the inputDepth layout and buffer size: in PointCloud.cpp just
before taking depthData (the block that uses cameraInstanceNum, width, height
and depthFrame->getData()), check that depthFrame indicates a packed 16-bit
depth pixel format (e.g. pixel format /bits-per-pixel API or an enum on
depthFrame) and that depthFrame->getData().size() == width * height *
sizeof(uint16_t); if either check fails, short-circuit (return error/throw/log
and skip the compute path) rather than passing the pointer into the projection
code that expects packed uint16_t data. Ensure you reference the same symbols
(inputDepth handling, depthFrame, width, height, depthFrame->getData()) so the
validation sits immediately before using depthData.
- Around line 161-169: initializeGPU currently replaces mgr but leaves Kompute
state (algo, tensors, tensor handles, and GPU-initialized flags) bound to the
old manager, causing cross-manager conflicts; in PointCloud::Impl::initializeGPU
reset/clear any existing Kompute state before creating a new kp::Manager: null
out or reset the shared_ptr algo, clear vectors like tensors and tensorHandles,
and set any GPU initialization flags (e.g., gpuInitialized or similar) to false
(do this prior to assigning mgr = std::make_shared<kp::Manager>(device)) so
subsequent initialization will recreate tensors/algorithms under the new manager
and avoid stale bindings.
In `@tests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpp`:
- Around line 212-254: Add tests to cover the new LengthUnit field on
dai::PointCloudConfig: verify the default value via
PointCloudConfig::getLengthUnit(), test the setter/getter pair using
PointCloudConfig::setLengthUnit(...) and getLengthUnit(), and include LengthUnit
in the serialization roundtrip by calling cfg.serialize(meta, dt), checking dt
== dai::DatatypeEnum::PointCloudConfig and that after
dai::utility::deserialize(meta, cfg2) cfg2.getLengthUnit() equals the originally
set unit; update both the "defaults and setters" and "serialization roundtrip"
TEST_CASEs to include these assertions alongside existing
organized/transformation checks.
---
Outside diff comments:
In `@src/pipeline/datatype/PointCloudData.cpp`:
- Around line 21-29: The color branch in getPoints() (guarded by isColor())
converts Point3fRGBA payload to Point3f but omits the width/height shape
invariant checks used elsewhere; add the same metadata validation/assertions
(the width/height shape checks used by the non-color getters) before
interpreting data->getData(), so malformed buffer/metadata is rejected
consistently. Locate the isColor() block and insert the identical
shape-invariant validation call(s)/assert(s) (the same functions or checks other
getters use), then proceed to reinterpret_cast<Point3fRGBA*> and convert to
Point3f as shown.
---
Duplicate comments:
In `@examples/cpp/PointCloud/PointCloudShowcase.cpp`:
- Around line 102-105: Remove the duplicated housing-transform guidance comment
block that repeats the note about setting the target coordinate system;
specifically delete the second comment referencing
pc->setTargetCoordinateSystem(dai::HousingCoordinateSystem::VESA_A) and the
surrounding lines mentioning CameraBoardSocket and HousingCoordinateSystem so
only the original guidance remains (leave the earlier occurrence intact).
In `@examples/python/PointCloud/PointCloudVisualizer.py`:
- Around line 22-32: In colorize_depth, the downscaling currently uses
frame[::4] which only samples rows; change the sampling to both axes (e.g., use
frame[::4, ::4]) so that downscaled and consequently nonZero reflect a true 2D
subsample of the depth frame; update the downscaled variable and percentile
computation (minD/maxD) to use this 2D subsample so the interpolation and
colormap reflect representative depth statistics.
In `@tests/src/onhost_tests/point_cloud_test.cpp`:
- Around line 430-444: The test currently computes bounds itself and writes them
back into the test subject (pcd), bypassing the production logic; remove the
lines that setMinX/setMinY/setMinZ/setMaxX/setMaxY/setMaxZ on pcd and instead
exercise the production code by calling PointCloudData::updateBoundingBox() (or
the public method on pcd that triggers that computation) using the generated
points (out), then assert the resulting pcd bounds; reference pcd,
PointCloudData::updateBoundingBox(), and out to locate where to replace the
manual bounds assignment with a call to the production update method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18a4f89a-2b21-4f58-ab1b-b406b412be5f
📒 Files selected for processing (25)
bindings/python/src/pipeline/datatype/PointCloudConfigBindings.cppbindings/python/src/pipeline/datatype/PointCloudDataBindings.cppbindings/python/src/pipeline/node/PointCloudBindings.cppcmake/Depthai/DepthaiDeviceRVC4Config.cmakeexamples/cpp/CMakeLists.txtexamples/cpp/PointCloud/CMakeLists.txtexamples/cpp/PointCloud/PointCloudShowcase.cppexamples/cpp/PointCloud/PointCloudVisualizer.cppexamples/cpp/PointCloud/README.mdexamples/python/CMakeLists.txtexamples/python/PointCloud/PointCloudShowcase.pyexamples/python/PointCloud/PointCloudVisualizer.pyinclude/depthai/pipeline/datatype/PointCloudConfig.hppinclude/depthai/pipeline/datatype/PointCloudData.hppinclude/depthai/pipeline/node/PointCloud.hppsrc/pcl/PointCloudData.cppsrc/pipeline/datatype/PointCloudConfig.cppsrc/pipeline/datatype/PointCloudData.cppsrc/pipeline/node/PointCloud.cppsrc/rtabmap/RTABMapSLAM.cppsrc/utility/ProtoSerialize.cpptests/CMakeLists.txttests/src/ondevice_tests/pointcloud_test.cpptests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cpptests/src/onhost_tests/point_cloud_test.cpp
💤 Files with no reviewable changes (1)
- src/rtabmap/RTABMapSLAM.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: pheec
Repo: luxonis/depthai-core PR: 1715
File: include/depthai/pipeline/node/PointCloud.hpp:34-98
Timestamp: 2026-03-16T11:17:08.485Z
Learning: In `include/depthai/pipeline/node/PointCloud.hpp`, the nested `Impl` class inside `PointCloud` is intentionally public (not private/forward-declared) because the on-host unit tests in `tests/src/onhost_tests/point_cloud_test.cpp` directly access `Impl` methods such as `setIntrinsics`, `setExtrinsics`, `useCPU`, `useCPUMT`, `computePointCloudDense`, `applyTransformation`, and `filterValidPoints`. Do not suggest making `Impl` private or forward-declaring it.
📚 Learning: 2026-03-16T11:17:08.485Z
Learnt from: pheec
Repo: luxonis/depthai-core PR: 1715
File: include/depthai/pipeline/node/PointCloud.hpp:34-98
Timestamp: 2026-03-16T11:17:08.485Z
Learning: In `include/depthai/pipeline/node/PointCloud.hpp`, the nested `Impl` class inside `PointCloud` is intentionally public (not private/forward-declared) because the on-host unit tests in `tests/src/onhost_tests/point_cloud_test.cpp` directly access `Impl` methods such as `setIntrinsics`, `setExtrinsics`, `useCPU`, `useCPUMT`, `computePointCloudDense`, `applyTransformation`, and `filterValidPoints`. Do not suggest making `Impl` private or forward-declaring it.
Applied to files:
tests/src/ondevice_tests/pointcloud_test.cpptests/src/onhost_tests/point_cloud_test.cppexamples/cpp/PointCloud/README.mdsrc/pipeline/datatype/PointCloudData.cpptests/CMakeLists.txtexamples/python/PointCloud/PointCloudShowcase.pytests/src/onhost_tests/pipeline/datatype/pointclouddata_test.cppexamples/cpp/PointCloud/PointCloudVisualizer.cppexamples/cpp/PointCloud/PointCloudShowcase.cppinclude/depthai/pipeline/datatype/PointCloudConfig.hppinclude/depthai/pipeline/datatype/PointCloudData.hppbindings/python/src/pipeline/node/PointCloudBindings.cppsrc/pipeline/node/PointCloud.cppbindings/python/src/pipeline/datatype/PointCloudDataBindings.cppinclude/depthai/pipeline/node/PointCloud.hpp
📚 Learning: 2026-03-13T19:40:47.231Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1718
File: src/pipeline/node/AutoCalibration.cpp:327-330
Timestamp: 2026-03-13T19:40:47.231Z
Learning: In `src/pipeline/node/AutoCalibration.cpp`, the hard-coded 1280x800 resolution check in `AutoCalibration::validateIncomingData()` is intentional. The AutoCalibration node in the depthai-core (C++) project is designed to support only 1280x800 sensor resolution and this constraint should not be flagged as a usability issue in future reviews.
Applied to files:
tests/src/ondevice_tests/pointcloud_test.cppsrc/pipeline/node/PointCloud.cpp
🔇 Additional comments (15)
tests/CMakeLists.txt (1)
464-470: Good CI wiring for new PointCloud tests.The added
pointclouddata_testandpoint_cloud_testregistrations and labels are consistent with the on-host test pattern used in this file.src/pipeline/datatype/PointCloudConfig.cpp (1)
12-42: API transition to organized/length-unit semantics looks clean.Getters/setters and matrix-overload behavior are coherent, and the 3×3 overload now correctly funnels through the 4×4 path.
examples/cpp/CMakeLists.txt (1)
167-167: PointCloud examples are integrated correctly into the C++ examples tree.examples/cpp/PointCloud/CMakeLists.txt (1)
7-11: Example target registration and remote-connection guard look good.src/utility/ProtoSerialize.cpp (1)
695-697: Backward-compat sparse emission is consistent with new layout semantics.Computing sparse from
height == 1keeps protobuf compatibility while matching organized-vs-filtered behavior.src/pcl/PointCloudData.cpp (1)
10-10: Consistentis_densemapping across both PCL export paths.Both XYZ and XYZRGB conversion paths now apply the same organized-based density flag logic.
Also applies to: 47-47
cmake/Depthai/DepthaiDeviceRVC4Config.cmake (1)
6-6: RVC2 device config does not exist in this repository. This review comment is based on incorrect assumptions. Only KB, RVC4, and Side devices have separate firmware configurations. RVC2 compatibility is not a concern for this file since RVC2 does not have a separate device config that requires version bumping.> Likely an incorrect or invalid review comment.examples/cpp/PointCloud/README.md (1)
1-64: LGTM!The documentation is well-structured with a clear feature table, build instructions, and comprehensive coverage of the three coordinate-system transform modes. The code samples effectively illustrate each capability.
examples/python/PointCloud/PointCloudVisualizer.py (3)
24-31: Variable naming uses camelCase instead of snake_case.Variables like
nonZero,minD,maxDuse camelCase. Python convention (PEP 8) recommends snake_case for variable names:non_zero,min_d,max_d.
35-73: LGTM!The pipeline setup is well-structured:
- Camera and StereoDepth configuration is clear
- PointCloud node correctly uses
passthroughDepthfor synchronized depth frames- Open3D visualizer initialization with coordinate frame is appropriate
- Auto-exposure settling and queue draining prevents stale initial frames
77-106: LGTM!The streaming loop correctly:
- Uses non-blocking
tryGet()for both queues- Handles the first geometry addition vs. subsequent updates
- Provides multiple exit paths (Q key, window close, Ctrl-C)
- Properly cleans up both visualization windows
examples/cpp/PointCloud/PointCloudVisualizer.cpp (1)
43-50: Nice robustness and stream-sync improvements.The percentile guards prevent invalid access in edge cases, and using
pc->passthroughDepthkeeps preview frames synchronized with the published cloud.Also applies to: 94-96
tests/src/ondevice_tests/pointcloud_test.cpp (1)
57-59: Sparse layout assertions are now correct.This enforces the right N×1 invariant (
height == 1,width == points.size(), bounded byWIDTH * HEIGHT) without assuming points must be strictly fewer than organized output.include/depthai/pipeline/datatype/PointCloudConfig.hpp (1)
34-36: Transform layout documentation is now accurate.The row-major wording aligns the public contract with how transforms are applied.
bindings/python/src/pipeline/node/PointCloudBindings.cpp (1)
50-54: Verify housing overload default foruseSpecTranslation.Python currently defaults housing transforms to
true(Line 53), while camera overload defaults tofalse(Line 48). Please confirm this matches C++ API defaults and intended behavior parity.#!/bin/bash set -euo pipefail echo "== C++ API declarations ==" rg -n "setTargetCoordinateSystem\\(" include/depthai/pipeline/node/PointCloud.hpp -C2 echo echo "== Python binding defaults ==" rg -n "setTargetCoordinateSystem|useSpecTranslation" bindings/python/src/pipeline/node/PointCloudBindings.cpp -C2If the C++ housing default is
false, update Line 53 topy::arg("useSpecTranslation") = false.
3 participants
Purpose
Adds the PointCloud node
Specification
PointCloudnode implementation (src/pipeline/node/PointCloud.cpp) with:keepPointCloudOrganized()setTargetCoordinateSystem()(camera socket or housing coordinate system)setLengthUnit()getEepromId()pattern (same asImageAlign) — automatically reinitializes when calibration is updated during runtimePointCloudDatato support organized (width×height) and filtered (N×1) layouts with bounding box metadatapointcloud_test.cppto reflect the current API (keepPointCloudOrganized()replaces the removedinitialConfig->setSparse())Dependencies & Potential Impact
StereoDepthnode output for depth inputPointCloudConfig::setSparse()is no longer the control mechanism for output layout, replaced bykeepPointCloudOrganized()on the node.Deployment Plan
Testing & Validation
pointcloud_test: covers organized and sparse output modes (10 frames each), validates dimensions, point count, and bounding box correctnesspointclouddata_test: validatesPointCloudDataserialization and accessorspoint_cloud_test: unit tests for the compute backend (intrinsics, extrinsics, length units)Summary by CodeRabbit
New Features
Breaking Changes
Tests / Docs