Skip to content

Conversation

@nicolapiccinelli
Copy link

@nicolapiccinelli nicolapiccinelli commented Dec 12, 2025

Hello @adeguet1,

This pull request is part of a larger project aimed at adding a hardware simulation mode to the dVRK. I have added a new IO port PORT_SIMULATION, which is currently simulating part of the firmware revision 9. The simulation port already features a dynamic simulation of each joint separately. Of course, it lacks the entire kinematics structure, but if you would appreciate the idea, I can work on creating an external connector that allows us to offload the simulation of the hardware to existing simulation software, providing tight integration with the dVRK stack.

See #38
See jhu-saw/sawRobotIO1394#17
See jhu-dvrk/sawIntuitiveResearchKit#240

…ting the QLA hardware. At the current stage there is a basic single joint simulation
@pkazanzides
Copy link
Contributor

This is nice. I have been interested in a simulation mode like this for quite some time. I support adding the hooks for PORT_SIMULATION, at a minimum. The actual simulation code could be here, or in a separate repository. Our approach with mechatronics-software was to minimize dependencies -- we do not even have a dependency on cisst (in retrospect, I wonder if that was a good idea). This proposed change only adds a pthread dependency, but this would break the build on some platforms, notably Windows. A quick solution (if we keep the simulation code here) is to add a CMake option BUILD_SIMULATION, or to add a CMake condition to only build it on Linux. As a minor point, it would be more convenient to use a shorter string, such as "sim" rather than "simulation".

@nicolapiccinelli
Copy link
Author

This is nice. I have been interested in a simulation mode like this for quite some time. I support adding the hooks for PORT_SIMULATION, at a minimum. The actual simulation code could be here, or in a separate repository. Our approach with mechatronics-software was to minimize dependencies -- we do not even have a dependency on cisst (in retrospect, I wonder if that was a good idea). This proposed change only adds a pthread dependency, but this would break the build on some platforms, notably Windows. A quick solution (if we keep the simulation code here) is to add a CMake option BUILD_SIMULATION, or to add a CMake condition to only build it on Linux. As a minor point, it would be more convenient to use a shorter string, such as "sim" rather than "simulation".

Hello @pkazanzides, I agree "sim" is much better than "simulation". I think it would be best to have a separate repository for implementing the simulation clients and keep the server endpoints here. I did not commit to that already because the proposal for which communication protocol to adopt is quite critical and can lead to different dependencies. But I can investigate that.

@pkazanzides
Copy link
Contributor

I don't want to over-complicate things, especially by introducing another communication protocol or supporting dynamic loading of simulation plugins. Perhaps the best solution is to make the simulation a library, in a separate repository, and then have a CMake option to use it. If the CMake option is on, it would define a conditional compilation flag. This would be similar to how the Firewire port is handled -- it requires conditional compilation because the library is only available on Linux. In the case of the simulation (unlike Firewire), the library would also need to be built, which could be handled by a CMake ExternalProject.

@nicolapiccinelli
Copy link
Author

nicolapiccinelli commented Dec 16, 2025

I don't want to over-complicate things, especially by introducing another communication protocol or supporting dynamic loading of simulation plugins. Perhaps the best solution is to make the simulation a library, in a separate repository, and then have a CMake option to use it. If the CMake option is on, it would define a conditional compilation flag. This would be similar to how the Firewire port is handled -- it requires conditional compilation because the library is only available on Linux. In the case of the simulation (unlike Firewire), the library would also need to be built, which could be handled by a CMake ExternalProject.

Understood, I will change the structure as you suggested. We can also move the threading dependency to this new simulation library. Can the external CMake project be located within this repository and then moved to the appropriate repository at a later stage? @pkazanzides

@pkazanzides
Copy link
Contributor

It would be even easier to make the dynamic simulation a library within this repository. If it is later moved to a separate repository, the CMake external project setup can be done at that time.

Following is what I would recommend:

  1. In CMakeLists.txt (probably the one in the lib sub-directory, since sim would likely be a sub-directory there):
option (Amp1394_HAS_SIM "Build Amp1394 with dynamic simulation" OFF)
if  (Amp1394_HAS_SIM)
   add_subdirectory(sim)
endif ()
  1. In Amp1394Config.cmake.in:
set (Amp1394_HAS_SIM    "@Amp1394_HAS_SIM@")
  1. In AmpIORevision.h.in:
#cmakedefine01 Amp1394_HAS_SIM
  1. This latter change will define the preprocessor symbol Amp1394_HAS_SIM, which can then be used in PortFactory.cpp.

@nicolapiccinelli
Copy link
Author

Hi @pkazanzides, I've separated the compilation and made the simulation a dynamic library. To enable the compilation, we have to set Amp1394_HAS_SIM in the sawRobotIO1394. I saw in the CMake that Amp1394_BUILD_SWIG is set via another variable CISST_HAS_SWIG_PYTHON. Should we also take the same path for the hardware simulation mode?

@pkazanzides
Copy link
Contributor

These changes look good. I would request to first merge to the devel branch, rather than main.

I don't think we need to set any additional CMake variables in sawRobotIO1394, since that should load the CMake options from Amp1394Config.cmake and (if needed) C++ preprocessor definitions from AmpIORevision.h (both of which are created in the Amp1394 build tree).

@nicolapiccinelli nicolapiccinelli changed the base branch from main to devel January 9, 2026 10:52
@nicolapiccinelli
Copy link
Author

@pkazanzides I've changed the base branch. To make this simulated port actually usable with the dVRK stack, there are a few other changes to be made here and there:

See jhu-saw/sawRobotIO1394#17
See jhu-dvrk/sawIntuitiveResearchKit#240

But at least here, I think we have everything ready.

@pkazanzides pkazanzides merged commit 3b6aeb0 into jhu-cisst:devel Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants