-
Notifications
You must be signed in to change notification settings - Fork 0
Offline run #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: cam-streams
Are you sure you want to change the base?
Offline run #2
Conversation
screechingviolet
commented
Oct 24, 2025
- When run with both IPs set to 0, it runs the vision pipeline in 'dummy mode' on the images saved to disk
- Still need to separate front right and front left cameras for easier viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces an "offline run" feature that enables running the vision pipeline in dummy mode when both robot IPs are set to "0", processing saved images from disk instead of connecting to actual Spot robots.
Key changes:
- Added global dummy mode flag with getter/setter functions
- Modified connection logic to create dummy connections when enabled
- Implemented image loading from disk in dummy mode instead of live camera feeds
Reviewed changes
Copilot reviewed 5 out of 105 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integ-test/integ-test.cpp | Implements dummy mode detection logic and conditional connection handling for offline image processing |
| src/spot-observer.cpp | Adds dummy mode state variables and modifies connection logic to support dummy connections with ID 0 |
| src/spot-connection.cpp | Implements disk-based image loading in dummy mode, reading saved RGB and depth images instead of streaming from cameras |
| include/spot-observer.h | Exposes dummy mode state variables and getter/setter functions in the public API |
| CMakeLists.txt | Updates dependency versions for OpenCV (4110→4120), ONNX Runtime (1.22.0→1.23.0), and Spot SDK path |
Comments suppressed due to low confidence (13)
tests/integ-test/integ-test.cpp:1
- [nitpick] The else clause should use braces for consistency and readability, especially since the if clause uses braces.
tests/integ-test/integ-test.cpp:1 - The commented code '// rm hand for vision? // FRONTRIGHT | FRONTLEFT' should be removed or clarified. These ambiguous comments make the intent unclear.
tests/integ-test/integ-test.cpp:1 - The commented-out variable 'run_on_dummy_images' should be removed if it's no longer needed.
tests/integ-test/integ-test.cpp:1 - This large block of commented-out code should be removed. If this logic might be needed later, consider using version control instead of leaving dead code in the codebase.
tests/integ-test/integ-test.cpp:1 - This commented-out image-saving logic should either be removed or implemented. Dead code reduces readability and maintainability.
src/spot-observer.cpp:1 - These TODO comments with 'AATASK' prefix should either be addressed or converted to proper TODO/FIXME comments with context about what needs to be done.
src/spot-observer.cpp:1 - This large block of commented-out code should be removed. The logic is already handled by the earlier return statement at line 107.
src/spot-observer.cpp:1 - Remove trailing whitespace at the end of the line.
src/spot-observer.cpp:1 - This TODO comment with 'AATASK' prefix should either be addressed or converted to a proper TODO/FIXME comment with clear context.
src/spot-observer.cpp:1 - The log message at line 487 is a duplicate of line 479. Remove the redundant log statement.
src/spot-observer.cpp:1 - This TODO comment with 'AATASK' prefix should either be addressed or converted to a proper TODO/FIXME comment with clear requirements.
src/spot-connection.cpp:1 - Remove these commented-out lines. The logic is already implemented at lines 292-293, and the incomplete comment '// cv::Mat' adds no value.
src/spot-connection.cpp:1 - This TODO comment with 'AATASK' prefix should either be addressed or converted to a proper TODO/FIXME comment. The code following this comment appears to already implement the disk image reading functionality.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Make avaiable to integ-test | ||
|
|
||
| UNITY_INTERFACE_EXPORT | ||
| bool UNITY_INTERFACE_API getDummy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a more descriptive name (and make it follow the naming convention of external-facing APIs)
| #endif | ||
|
|
||
| // Make dummy available to other files in namespace | ||
| // change so all files use getter func? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are internal state variables. They shouldn't go in an externally-facing file.
| cv::imshow("SPOT " + std::to_string(spot) + " Stream " + std::to_string(stream) + " RGB" + std::to_string(i), image); | ||
| cv::imshow("SPOT " + std::to_string(spot) + " Stream " + std::to_string(stream) + " Depth" + std::to_string(i), depth); | ||
|
|
||
| // save images to disk for dummy usage with vision pipeline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a useful bit of functionality. We should uncomment it and put it behind a flag.
| LogMessage("Dummy mode: reset taken_from_dummy counter"); | ||
| } | ||
|
|
||
| cv::Mat cv_img = cv::imread(std::format("..\\..\\saved_images\\spot_rgb{}.png", 1 + (taken_from_dummy/2)), cv::IMREAD_UNCHANGED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break if the code isn't launched from the right place
| } | ||
|
|
||
| cv::Mat cv_img = cv::imread(std::format("..\\..\\saved_images\\spot_rgb{}.png", 1 + (taken_from_dummy/2)), cv::IMREAD_UNCHANGED); | ||
| cv::cvtColor(cv_img, cv_img, cv::COLOR_BGR2RGB); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use cvtColor(img_rgba, cv2.COLOR_BGR2BGRA) instead of manually appending an alpha channel
| message(STATUS "CUDAToolkit found: ${CUDAToolkit_VERSION}") | ||
|
|
||
| set(SPOT_SDK_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/extern/spot-sdk-install) | ||
| set(SPOT_SDK_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/extern/spot-cpp-sdk) # changed from sdk-install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>