Add YARP interop plugin and support for opening YARP devices via dinrail::Device factory - #36
Conversation
a994296 to
ea2f37a
Compare
ea2f37a to
88b2207
Compare
There was a problem hiding this comment.
Pull request overview
Adds an interop framework allowing dinrail::Device to open YARP devices and expose native YARP interfaces.
Changes:
- Adds interop plugin discovery and fallback device creation.
- Implements the YARP interop plugin and runtime interface casting.
- Extends CLI discovery, tests, and documentation.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/yarp/test/YarpDropInCompatibilityTest.cpp |
Tests opening and viewing YARP devices. |
src/yarp/test/CMakeLists.txt |
Registers the YARP compatibility test. |
src/yarp/dinrail/YarpInteropPlugin.h |
Declares the YARP interop plugin. |
src/yarp/dinrail/YarpInteropPlugin.cpp |
Implements YARP device creation and discovery. |
src/yarp/dinrail/YarpDeviceWrapper.h |
Declares the YARP device wrapper. |
src/yarp/dinrail/YarpDeviceWrapper.cpp |
Implements native YARP interface resolution. |
src/yarp/CMakeLists.txt |
Builds and installs the YARP interop module. |
src/tools/dinrail.cpp |
Adds interop and expanded device listing commands. |
src/core/test/RuntimeDynamicCastTest.cpp |
Tests runtime casting behavior. |
src/core/test/InteropPluginTest.cpp |
Tests interop discovery and fallback. |
src/core/test/interop/TestInteropCommon.h |
Provides shared interop test fixtures. |
src/core/test/interop/TestInteropBeta.cpp |
Adds the beta test plugin. |
src/core/test/interop/TestInteropAlpha.cpp |
Adds the alpha test plugin. |
src/core/test/interop/IFooTest.h |
Defines a test-only foreign interface. |
src/core/test/CMakeLists.txt |
Builds interop plugins and tests. |
src/core/dinrail/RuntimeDynamicCast.h |
Declares ABI-level runtime casting. |
src/core/dinrail/RuntimeDynamicCast.cpp |
Implements MSVC and Itanium casting paths. |
src/core/dinrail/RuntimeContext.h |
Exposes interop discovery APIs. |
src/core/dinrail/RuntimeContext.cpp |
Adds interop loading, caching, and fallback. |
src/core/dinrail/PluginUtils.h |
Defines discovery metadata and APIs. |
src/core/dinrail/PluginUtils.cpp |
Implements native and interop discovery. |
src/core/dinrail/Interfaces.cpp |
Defines new interface destructors. |
src/core/dinrail/IInteropPlugin.h |
Defines the interop plugin contract. |
src/core/dinrail/IInterfaceView.h |
Defines dynamic interface lookup. |
src/core/dinrail/Device.h |
Extends view() for wrapped interfaces. |
src/core/dinrail/Device.cpp |
Updates implementation dependencies. |
src/core/CMakeLists.txt |
Adds new core sources and tests. |
docs/yarp-migration.md |
Documents migration from PolyDriver. |
docs/plugin-discovery.md |
Documents interop discovery conventions. |
docs/interop-plugins.md |
Documents authoring interop plugins. |
Suppressed comments (1)
src/core/dinrail/RuntimeDynamicCast.h:95
Sourceis already unqualified by the preceding static assertions, sosourceandstatic_cast<UnqualifiedSource*>(source)are the same pointer and this calculation is guaranteed to produce zero. Therefore the MSVC path cannot supply the nonzero vfptr displacement it was designed to support; derive the displacement using the actual MSVC object-layout mechanism and add a hierarchy test that requires it.
const auto sourceAddress = reinterpret_cast<std::intptr_t>(static_cast<void*>(source));
const auto rttiAddress = reinterpret_cast<std::intptr_t>(
static_cast<void*>(static_cast<UnqualifiedSource*>(source)));
const auto vfDelta = static_cast<std::ptrdiff_t>(rttiAddress - sourceAddress);
return PolymorphicView{static_cast<void*>(source), typeid(UnqualifiedSource), vfDelta};
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
205161e to
cd533b9
Compare
|
Brilliant! Maybe I am missing something, but can't the |
How you would envision the signature of such a method? |
I thought the interface should be like dinrail/src/core/dinrail/Device.h Line 72 in 01af08f Just for brainstorming, to avoid the use the templates, we could have instead a |
If I got this correctly, I think I experimented a bit with this idea. The problem is that the only way of doing dynamic_cast in standard C++ (without the compiler specific bits introduced here) is to compile somewhere something similar to the following code: yarp::dev::DeviceDriver* ptr = ..;
Interface* interface{nullptr};
interface = dynamic_cast<Interface*>(ptr);This requires to know (at compile time) both the types |
|
Ok, I see. Thanks for the explanation! |
|
fyi @isorrentino if there is anyone interested in this on @gbionics/team-hermes side, feel free to check it out! |
|
I did not add @RiccardoGrieco to the long list of reviewers, my bad! |
| auto interop = interopPlugin->allocate(); | ||
| if (!interop) | ||
| { | ||
| std::cerr << "dinrail::Device: impossible to create instance for interop plugin '" |
There was a problem hiding this comment.
Is it possible to move these messages to spdlog?
There was a problem hiding this comment.
Yep, but I would do that in a separate PR where we migrate the whole src/core directory to spdlog, if that is ok for you: #45 .
…n of interop plugins support
8ef8838 to
e41b040
Compare
This PR adds the required code to open any YARP device as a dinrail device (i.e. via
dinrail::Device::open), and view any interface of the opened YARP device viadinrail::Device::view, without adding any YARP dependency in the core dinrail library, but only a runtime dependency on thedinrail-yarplibrary.In a nutshell, it permits to write code like:
How this is achieved? The PR is unfortunately large, but the key points are the following.
Dinrail Interop Plugins
A new kind of dinrail plugin (beside the dinrail device) is introduced, the dinrail interop plugin, that is a class that inherits from the
dinrail::IInteropPlugin, i.e. that defines this two methods:i.e., it provides an arbitrary way to open devices, given a
dinrail::Parametersobject.This is used to implement without adding any dep on YARP on the dinrail core, the possibility of opening in dinrail non-dinrail devices, such as YARP devices. The
listDevicespart is a minor features, useful to make sure thatdinrail dev --listcan list YARP devices in case the YARP interop plugin is installed.YARP Interop Plugin
The main class that implements the
dinrail::IInteropPluginis thedinrail::YarpInteropPluginclass, that implements the aforementioned methods, and in particularstd::unique_ptr<dinrail::IDevice> createDevice. This is implemented by creating adinrail::YarpDeviceWrapperthat is a proper dinrail device (i.e. it inherits fromdinrail::IDevice), but under the hood, it opens a YARP device via theyarp::dev::PolyDriver.However, the big problem here is the following: you can easily open YARP devices and pass them as dinrail devics using
dinrail::YarpDeviceWrapper, but then theDevice::viewwill fail toviewany interface implemented by the underlying YARP device, as thedynamic_castwill be calleddinrail::YarpDeviceWrapper, not on the underlying YARP device. To solve this, the next point is introduced.Runtime Dynamic Cast
The
Device::viewmethod has been extended as in the following:template <class T> bool view(T*& x) { x = nullptr; IDevice* impl = getImplementation(); if (!impl) { return false; } T* v = dynamic_cast<T*>(impl); if (v != nullptr) { x = v; return true; } + // Devices wrapping a foreign implementation (e.g. interop plugins) can + // resolve interfaces that are not reachable through a direct cast. + auto* interfaceView = dynamic_cast<IInterfaceView*>(impl); + if (interfaceView != nullptr) + { + void* resolved = interfaceView->viewInterface(typeid(T)); + if (resolved != nullptr) + { + x = static_cast<T*>(resolved); + return true; + } + } return false; }The
dinrail::IInterfaceViewis an interface implemented by thedinrail::YarpDeviceWrapperclass, and it permits basically to do a dynamic cast, without knowing the base polymorphic class of the pointer, that in the case of YARP devices isyarp::dev::DeviceDriver. This is not possible to do in stock C++, so a bit of platform-specific code (targeting Itanium ABI for Clang and GCC, and MSVC ABI for Visual Studio, that both provide some platform-specific way of doing that) has been added in theRuntimeDynamicCasthelper functions. In this way, theviewfunctionality can be extended in a plugin-base way, without the need to recompile the code, as instead it happens in the regular templated view for each pair of base polymorphic interface (likedinrail::IDeviceoryarp::dev::DeviceDriver) and interfaceTbeing tested.Follow up work
The work necessary to complete the YARP device drop-in replacement support is not completed, as it is missing #39 . However, this PR is already quite big, so it was decidec to keep #39 in a future PR.