Skip to content

Set sequences & handle spurious wakeups in RVC4 IMU #1727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
asahtik wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Set sequences & handle spurious wakeups in RVC4 IMU #1727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99072a2
Add test to check for updates in each imu package
asahtik Mar 19, 2026
fca5575
RVC4 FW: Set sequences & handle spurious wakeups in IMU
asahtik Mar 19, 2026
e755f01
Fix test
asahtik Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmake/Depthai/DepthaiDeviceRVC4Config.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
set(DEPTHAI_DEVICE_RVC4_MATURITY "snapshot")

# "version if applicable"
set(DEPTHAI_DEVICE_RVC4_VERSION "0.0.1+b96987e20d3e015dae8854b5fa8faa599c8d5ea8")
set(DEPTHAI_DEVICE_RVC4_VERSION "0.0.1+40ac229a57372612e96393b2a6bf595a51b8ae88")
37 changes: 37 additions & 0 deletions tests/src/ondevice_tests/pipeline/node/imu_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <catch2/catch_all.hpp>
#include <catch2/catch_test_macros.hpp>
#include <chrono>
#include <initializer_list>

#include "depthai/depthai.hpp"
Expand Down Expand Up @@ -40,3 +41,39 @@ TEST_CASE("Test IMU, all sensors") {
TEST_CASE("Test IMU, gyroscope 480 Hz") {
basicIMUTest(480.0f, {dai::IMUSensor::GYROSCOPE_RAW}, 0.8f); // TODO(Morato) - debug why some devices need so much tolerance
}

TEST_CASE("At least one measurement should be updated") {
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider a more descriptive test name.

The current name "At least one measurement should be updated" could be clearer. Consider renaming to something like "Test IMU sequence numbers strictly increase" or "Test IMU packets contain new data (no spurious wakeups)" to better reflect the test's purpose.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/src/ondevice_tests/pipeline/node/imu_test.cpp` at line 45, Rename the
test case string in the TEST_CASE invocation to a more descriptive name that
reflects its intent (e.g., "Test IMU sequence numbers strictly increase" or
"Test IMU packets contain new data (no spurious wakeups)"); locate the
TEST_CASE(...) macro in imu_test.cpp (the TEST_CASE symbol) and replace the
current literal "At least one measurement should be updated" with the chosen
clearer description, and update any test documentation/comments or related
assertions if they reference the old name.

dai::Pipeline pipeline;
auto imu = pipeline.create<dai::node::IMU>();
imu->enableIMUSensor(dai::IMUSensor::ACCELEROMETER_RAW, 400);
imu->enableIMUSensor(dai::IMUSensor::GYROSCOPE_RAW, 400);

imu->setBatchReportThreshold(10);
imu->setMaxBatchReports(20);
Comment on lines +51 to +52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify maxBatchReports constraint documentation and runtime handling

# Check the IMUProperties documentation
echo "=== IMUProperties maxBatchReports documentation ==="
rg -n -A2 -B2 'maxBatchReports' --type=cpp

# Check if there's any validation/clamping logic for maxBatchReports
echo ""
echo "=== Searching for validation of maxBatchReports ==="
rg -n -C5 'setMaxBatchReports' --type=cpp

Repository: luxonis/depthai-core

Length of output: 12251

setMaxBatchReports(20) exceeds the documented maximum of 5.

According to include/depthai/properties/IMUProperties.hpp:172-173, maxBatchReports is documented as "Maximum 5," yet the test sets this to 20. The setter in src/pipeline/node/IMU.cpp:42-43 does not validate or clamp this value, so undefined behavior or firmware-level constraint violations are possible.

🔧 Proposed fix 
     imu->setBatchReportThreshold(10);
-    imu->setMaxBatchReports(20);
+    imu->setMaxBatchReports(5);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
imu->setBatchReportThreshold(10);
imu->setMaxBatchReports(20);
imu->setBatchReportThreshold(10);
imu->setMaxBatchReports(5);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/src/ondevice_tests/pipeline/node/imu_test.cpp` around lines 51 - 52,
The test sets imu->setMaxBatchReports(20) which exceeds the documented maximum
of 5; update the IMU setter and/or the test: in the IMU class (function
IMU::setMaxBatchReports in src/pipeline/node/IMU.cpp) add validation/clamping to
enforce the documented limit (e.g., clamp value to 5 or return/error when >5)
and update the unit test in imu_test.cpp to use an allowed value (<=5) such as 5
so it matches include/depthai/properties/IMUProperties.hpp documentation and
avoids firmware violations.


auto imuQueue = imu->out.createOutputQueue(50, false);

pipeline.start();

auto start = std::chrono::steady_clock::now();

dai::IMUPacket previousPacket;

uint32_t numMessages = 0;

while(pipeline.isRunning() && std::chrono::steady_clock::now() - start <= std::chrono::seconds(10)) {
auto imuData = imuQueue->get<dai::IMUData>();
if(imuData == nullptr) continue;

++numMessages;

for(const auto& imuPacket : imuData->packets) {
Comment on lines +60 to +70
REQUIRE((imuPacket.acceleroMeter.sequence > previousPacket.acceleroMeter.sequence || imuPacket.gyroscope.sequence > previousPacket.gyroscope.sequence));
previousPacket = imuPacket;
}
Comment on lines +70 to +73
}

REQUIRE(numMessages > 0);

pipeline.stop();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.