Skip to content
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

Updates #15

Merged
merged 3 commits into from
Jun 27, 2023
Merged

Updates #15

merged 3 commits into from
Jun 27, 2023

Conversation

marip8
Copy link
Member

@marip8 marip8 commented Jun 27, 2023

This PR updates the ROS interface singleton pattern to allow the plugin library from this library to be loaded dynamically from within Python and for downstream custom plugins to be able to link against the interface implementations in this repo. This PR partially addresses #13. My guess is that the primary issue was that the extern declaration of the node singleton. Since the node was provided in the c++ reach study executable (and not in the library itself), the node singleton looked like an undefined reference to any other executable that tried to load it.

Although this PR allows ROS2-enabled plugins to be loaded in Python, they won't actually work in a Python script because rclcpp::init needs to be called first with the contents of argv (i.e. parameters, etc.). I tried calling rclpy.init() in the Python script instead (see below), but that doesn't seem to do the trick. I think we'll need to add a small Python interface to this repo for invoking rclcpp::init from Python. I propose that be done in a separate PR

import yaml
from reach import runReachStudy, PluginLoader
import rclpy
import sys


def main():
    rclpy.init(args=sys.argv)
    if not rclpy.ok():
        raise RuntimeError('ROS is not running')

    yaml_path = 'config/reach_study.yaml'
    with open(yaml_path) as f:
        config = yaml.safe_load(f)

    loader = PluginLoader()
    loader.search_libraries_env = 'REACH_PLUGINS'

    # Works correctly now
    solver = loader.createIKSolverFactoryInstance('MoveItIKSolver')

    # Fails because `rclcpp::ok()` returns false in the c++ plugin library, even though `rclpy.init` was called in this executable
    res = runReachStudy(config)


if __name__ == '__main__':
    main()

@marip8
Copy link
Member Author

marip8 commented Jun 27, 2023

@SammyRamone can you review and test on your machine?

@SammyRamone
Copy link
Contributor

SammyRamone commented Jun 27, 2023

@SammyRamone can you review and test on your machine?

Thanks a lot for the effort you put into fixing this weird issue!
I will test it in a few minutes and get back to you.
About the rclcpp::init: You are correct, it is not enough to call the Python version. I encountered the same issue in a different project. We also fixed it by having a small method that just calls the C++ init. It is also possible to check if the init method was already called an just call it in this case (see here for an example, although in this case the parameters were given directly and we did not use argv)

@SammyRamone
Copy link
Contributor

SammyRamone commented Jun 27, 2023

The code works (both for the Python interface as well as through normal ROS 2 calls) 🥳

For the init problem, I think that just having a small C++ method with Python interface is the best solution. There are some methods to optain argv from any part of the code (in Linux there is the pseudo file /proc/self/cmdline, in Windows there is GetCommandLineA()), however not in all use cases the argv of the process are the argv that you want to pass to ROS. Having a method makes things more flexible.
I would also add anoher method to this Python interface that allows manual setting of parameters to the C++ ROS node object (that was what I planned to do in my code as I basically want to change IK parameters during runtime).

However, I agree that this (+ some dokumentation how to run the ROS 2 plugins from Python) can be resolved in a second PR. If you want to, I can programm it.

@marip8
Copy link
Member Author

marip8 commented Jun 27, 2023

The code works (both for the Python interface as well as through normal ROS 2 calls)

Excellent; I'll go ahead and merge then

I would also add anoher method to this Python interface that allows manual setting of parameters to the C++ ROS node object (that was what I planned to do in my code as I basically want to change IK parameters during runtime).

Good point. I've used the Python interface in the past to run a series of reach studies where I changed some ROS parameters in between runs, so I think that would be a useful addition

However, I agree that this (+ some dokumentation how to run the ROS 2 plugins from Python) can be resolved in a second PR. If you want to, I can programm it.

Sure, that would be helpful. I'll open an issue about the features we want to expose in the Python interface for reference

@marip8 marip8 merged commit d3ac7bb into ros-industrial:master Jun 27, 2023
@marip8 marip8 deleted the updates branch June 27, 2023 16:52
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