-
Notifications
You must be signed in to change notification settings - Fork 5
Telesurgery Workflow #149
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
Telesurgery Workflow #149
Conversation
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
workflows/telesurgery/README.md
Outdated
| ## Quick Start | ||
|
|
||
| ### X86 | ||
| 1. Install NVIDIA driver (>= 555) and CUDA (>= 12.6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line can be removed and merge the x86 setup and aarch63 setup together.
Thanks,
Bin
workflows/telesurgery/README.md
Outdated
| export PATIENT_IP=10.137.145.163 | ||
| export SURGEON_IP=10.111.66.170 | ||
| ``` | ||
| > Make sure MIRA API Server is up and running (port: 8081) in case of Physical World. |
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.
May need more details or the link of set up MIRA here.
Thanks,
Bin
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.
ping @wendell-hom
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.
@SachidanandAlle @Nic-Ma is the MIRA api server needed for simulation side? If not, then I suggest removing this line for now for v0.2
workflows/telesurgery/README.md
Outdated
| cd <path-to-i4h-workflows> | ||
| bash tools/env_setup_telesurgery.sh | ||
| ``` | ||
| 4. Set environment variables: |
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.
Some duplication between line 51-55 and line 79-91.
Thanks,
Bin
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 the full telesurgery workflow, adding camera operators, environment setup scripts, DDS configuration, Docker support, and documentation.
- Added RealSense and OpenCV camera operators under
holohub/operators/camera. - Included DDS QoS profile, buffer tuning, and utility scripts for environment setup.
- Provided Docker setup and comprehensive README updates for running simulation and physical workflows.
Reviewed Changes
Copilot reviewed 69 out of 69 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/telesurgery/scripts/holohub/operators/camera/realsense.py | RealSense camera operator implementation |
| workflows/telesurgery/scripts/holohub/operators/camera/cv2.py | OpenCV video capture operator implementation |
| workflows/telesurgery/scripts/holohub/operators/camera/init.py | Package initialization for camera operators |
| workflows/telesurgery/scripts/holohub/operators/init.py | Package initialization for holohub operators |
| workflows/telesurgery/scripts/holohub/init.py | Package initialization for holohub module |
| workflows/telesurgery/scripts/holohub/README.md | Holoscan operator folder description |
| workflows/telesurgery/scripts/env.sh | Environment variable setup script |
| workflows/telesurgery/scripts/dds/qos_profile.xml | DDS QoS profile for telesurgery |
| workflows/telesurgery/scripts/dds/buffers.sh | UDP buffer inspection and tuning script |
| workflows/telesurgery/scripts/dds/README.md | DDS troubleshooting guide |
| workflows/telesurgery/scripts/dds/.gitignore | Ignored DDS license file |
| workflows/telesurgery/scripts/common/utils.py | NTP offset and utility functions |
| workflows/telesurgery/scripts/common/init.py | Package initialization for common utilities |
| workflows/telesurgery/scripts/init.py | Root scripts package initialization |
| workflows/telesurgery/requirements.txt | Python dependencies for telesurgery workflow |
| workflows/telesurgery/docker/setup.sh | Docker helper script for launching the container |
| workflows/telesurgery/docker/README.md | Docker usage guide |
| workflows/telesurgery/docker/Dockerfile | Dockerfile for building telesurgery image |
| workflows/telesurgery/README.md | Top-level telesurgery workflow documentation |
| tools/env_setup_telesurgery.sh | Automated environment setup script |
Comments suppressed due to low confidence (1)
workflows/telesurgery/scripts/holohub/operators/camera/cv2.py:25
- [nitpick] Add unit or integration tests for
CV2VideoCaptureOpto verify successful initialization, frame capture, conversion, and error handling.
class CV2VideoCaptureOp(Operator):
|
|
||
| def __init__(self, fragment, width: int, height: int, device_idx: int, framerate: int, *args, **kwargs): | ||
| """ | ||
| Initialize the RealSense operator. |
Copilot
AI
May 29, 2025
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.
[nitpick] Update the constructor docstring to accurately describe CV2VideoCaptureOp and its parameters instead of referencing RealSense and unused args.
| self.device_idx = device_idx | ||
| self.framerate = framerate | ||
|
|
||
| self.stream_type = 2 # color |
Copilot
AI
May 29, 2025
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.
[nitpick] Replace the magic number 2 with a descriptive constant or enum (e.g., cv2.CAP_PROP_CONVERT_RGB) for clarity.
| self.stream_type = 2 # color | |
| self.stream_type = STREAM_TYPE_COLOR |
| self.framerate = framerate | ||
|
|
||
| self.stream_type = 2 # color | ||
| self.stream_format = 5 # rgb |
Copilot
AI
May 29, 2025
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.
[nitpick] Replace the magic number 5 with a descriptive constant or enum (e.g., cv2.COLOR_BGR2RGB) for consistency and readability.
| self.stream_format = 5 # rgb | |
| self.stream_format = STREAM_FORMAT_RGB # rgb |
Fix a few minor issues in the README file. --------- Signed-off-by: binliu <[email protected]>
tools/env_setup_telesurgery.sh
Outdated
| # ---- Install build tools (Common) ---- | ||
| echo "Installing build tools..." | ||
| if [ "$EUID" -ne 0 ]; then | ||
| sudo apt-get install -y git cmake build-essential pybind11-dev v4l-utils |
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.
We may need to add v4l-utils in the NOTICE.txt
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.
Added the following
v4l-utils (https://github.com/gjasny/v4l-utils)
Copyright (C) 1989, 1991 Free Software Foundation, Inc.
Licensed under GPL-2.0 (https://github.com/gjasny/v4l-utils/blob/master/COPYING)
| # Fix livestream public endpoint address issue in 2.0.2/2.1.0 | ||
| RUN sed -i '/--\/app\/livestream\/publicEndpointAddress=/d' /workspace/isaaclab/source/isaaclab/isaaclab/app/app_launcher.py | ||
|
|
||
| # Install uv using curl for openpi |
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.
It seems we copied this comment from robot us docker which is no longer valid. We should remove this line
|
|
||
| WORKDIR /tmp | ||
|
|
||
| RUN apt-get update && \ |
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.
CUDA 12.8 Installation is heavy (~2GB) CUDA toolkit installation might be unnecessary for telesurgery.
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.
we need cuda toolkit for nvjpeg
| WORKDIR /workspace | ||
|
|
||
| # Fix livestream public endpoint address issue in 2.0.2/2.1.0 | ||
| RUN sed -i '/--\/app\/livestream\/publicEndpointAddress=/d' /workspace/isaaclab/source/isaaclab/isaaclab/app/app_launcher.py |
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.
If webrtc livestream is not used, this patch may not be needed.
| class DelayOp(Operator): | ||
| """A delay operator that takes input and waits for few milliseconds.""" | ||
|
|
||
| def __init__(self, fragment, deplay_ms, *args, **kwargs): |
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.
Typo in variable name: deplay_ms should be delay_ms
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.
Fixed.
| from holoscan.core._core import OperatorSpec | ||
|
|
||
|
|
||
| class HaplyOp(Operator): |
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 seems to be a skeleton with empty methods
| """Parse command-line arguments and run the application.""" | ||
| parser = argparse.ArgumentParser(description="Run the hid application") | ||
| parser.add_argument("--device_idx", type=int, default=0, help="device index case of multiple joysticks") | ||
| parser.add_argument("--api_host", type=str, default="10.137.145.163", help="api server host") |
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.
We have serveral places that is harded to 10.137.145.163. We may need to find a way to make it more flexible
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.
lets fetch default values from env variables instead.. that reduces the direct ip exploration
| @@ -0,0 +1,138 @@ | |||
| # Telesurgery Workflow | |||
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.
Please also update the project root main project README.md. It mentions only two workflows (robotic_surgery and robotic_ultrasound) but doesn't include telesurgery. Optionally, we can also add a FIXME there to remind the team to add that later
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.
Added a link in the top-level README.
|
|
||
| import time | ||
|
|
||
| import cv2 |
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.
Also need to include this in the NOTICE.txt
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.
Added
opencv-python (https://github.com/opencv/opencv-python)
Copyright (c) Olli-Pekka Heinisuo
License under MIT (https://github.com/opencv/opencv-python/blob/4.x/LICENSE.txt)
| @@ -0,0 +1,14 @@ | |||
| # SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to construct any tests without the hardware? Thanks!
workflows/telesurgery/README.md
Outdated
| ### [Option-2] Patient in Simulation World _(x86)_ | ||
| ```bash | ||
| # download the assets | ||
| export ISAAC_ASSET_SHA256_HASH=8e80faed126c533243f50bb01dca3dcf035e86b5bf567d622878866a8ef7f12d |
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.
@mingxin-zheng Can we remove this line now?
Thanks.
| @@ -0,0 +1,72 @@ | |||
| __TODO::__ This document needs update | |||
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.
@SachidanandAlle will you update the doc in this PR or future?
Thanks.
| @@ -0,0 +1 @@ | |||
| rti_license.dat | |||
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.
@mingxin-zheng Shoud we move this file to the root?
Thanks.
|
|
||
| # Run the asynchronous main function | ||
| if __name__ == "__main__": | ||
| asyncio.run(main("ws://localhost:10001", "10.137.145.163", 8081)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not hardcode the IP address here, can we pass it in the CLI command?
Thanks.
Add a new encoder/decoder option: `nvc` using NVIDIA Video Codec. The encoder and decoder operators are in C++ with Python wrappers. ### Known Issue The simulated surgeon app prints the following error when attempting to access the Tensor in the `CameraStreamStats` operators: ``` [error] [entity.hpp:90] Unable to find component from the name '' (error: GXF_ENTITY_COMPONENT_NOT_FOUND) ``` --------- Signed-off-by: Victor Chang <[email protected]>
Signed-off-by: Victor Chang <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Update env_setup_telesurgery.sh to install the Haply Inverse Service Co-authored-by: Wendell Hom <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
| def _data_acquisition_callback(self, event: carb.events.IEvent): | ||
| super()._data_acquisition_callback(event) | ||
|
|
||
| if self.callback and self._current_frame["rendering_frame"] != self.prev_rendering_frame: |
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.
Hi @SachidanandAlle ,
I've added a few output here for the self._current_frame["rendering_frame"] and self.prev_rendering_frame. And here is the result. I think this is the reason why I stuck at the camera window. I am not sure if this is a bug or we need to trigger something to make the app work.
NFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: -1
INFO:root:[CameraEx] Sending frame 0 to callback
INFO:root:[CameraEx] RGB shape: (1920, 1080, 3)
INFO:root:[CameraApp] Frame 0 received, data shape: (1920, 1080, 3)
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
|
Did you click the play button in the Isaac SIM.. looks like there is no
change in the frame.. hence sim callback is not sending any new frames to
the client..
…On Sun, Jun 1, 2025, 9:33 PM binliunls ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In workflows/telesurgery/scripts/patient/simulation/camera/sensor.py
<#149 (comment)>
:
> +class CameraEx(Camera):
+ def __init__(self, channels=3, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+
+ self.channels = channels
+ self.prev_rendering_frame = -1
+ self.frame_counter = 0
+ self.callback = None
+
+ def set_callback(self, callback):
+ self.callback = callback
+
+ def _data_acquisition_callback(self, event: carb.events.IEvent):
+ super()._data_acquisition_callback(event)
+
+ if self.callback and self._current_frame["rendering_frame"] != self.prev_rendering_frame:
Hi @SachidanandAlle <https://github.com/SachidanandAlle> ,
I've added a few output here for the
self._current_frame["rendering_frame"] and self.prev_rendering_frame. And
here is the result. I think this is the reason why I stuck at the camera
window. I am not sure if this is a bug or we need to trigger something to
make the app work.
NFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: -1
INFO:root:[CameraEx] Sending frame 0 to callback
INFO:root:[CameraEx] RGB shape: (1920, 1080, 3)
INFO:root:[CameraApp] Frame 0 received, data shape: (1920, 1080, 3)
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
INFO:root:[CameraEx] Data acquisition event received current frame: 0, prev_rendering_frame: 0
—
Reply to this email directly, view it on GitHub
<#149 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABX7YK2D3QOBQ3H7ALSBTLL3BPHZPAVCNFSM6AAAAAB6CW7XO2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQOBWGY4DCOJZGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thanks, this works. I think it would be better if there is a way to config the app to run automatically or we can start it through code, so that it would be easy for testing. Will try to figure it out. |
|
Check with Yiheng.. he might have a solution to this. Most of the logic is
coming from his MIRA robot example
*Thanks & Regards*
*SACHI*
…On Sun, Jun 1, 2025, 10:14 PM binliunls ***@***.***> wrote:
*binliunls* left a comment (isaac-for-healthcare/i4h-workflows#149)
<#149 (comment)>
Thanks, this works. I think it would be better if there is a way to config
the app to run automatically or we can start it through code, so that it
would be easy for testing. Will try to figure it out.
—
Reply to this email directly, view it on GitHub
<#149 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABX7YK75GOQOM45JEK5QXAL3BPMSZAVCNFSM6AAAAAB6CW7XO2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMRYG44TOMZRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Update instructions to run the applications in a containerized environment. - Improve OOBE on a single system setup - Reduce the number of manual steps wherever possible Tested with the following configurations - [x] Run Surgeon app - [x] Run Patient simulated app --------- Signed-off-by: Victor Chang <[email protected]>
Signed-off-by: Sachidanand Alle <[email protected]>
Add the integration test --------- Signed-off-by: binliu <[email protected]> Signed-off-by: binliunls <[email protected]>
Signed-off-by: binliu <[email protected]>
|
/build |
Signed-off-by: binliu <[email protected]>
|
/build |
Uh oh!
There was an error while loading. Please reload this page.