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

Port class_loader or alternative into ROS 2 #60

Closed
1 task done
wjwwood opened this issue Jun 15, 2015 · 13 comments
Closed
1 task done

Port class_loader or alternative into ROS 2 #60

wjwwood opened this issue Jun 15, 2015 · 13 comments
Assignees

Comments

@wjwwood
Copy link
Member

wjwwood commented Jun 15, 2015

In order to implement the Node Components pattern, we need to be able to load shared libraries and instantiate classes from within those shared libraries. This is handled by class_loader in ROS 1, so we need to migrate it or do something similar for ROS 2.

AC:

  • Add package that provides the needed machinery to "register" classes into and "load" classes out of a shared library.
@gbiggs
Copy link
Member

gbiggs commented Jul 1, 2015

Looking over the code for class_loader, I didn't see anything that's ROS-specific (other than the build system). It mainly looks like it needs cleaning up to C++11.

@dirk-thomas
Copy link
Member

In ROS 1 class_loader contains the ROS agnostic part for loading plugins. pluginlib provides the ROS specific logic to find plugins within the ROS package infrastructure. I think we need two similar pieces in ROS 2.

@wjwwood
Copy link
Member Author

wjwwood commented Jul 1, 2015

@dirk-thomas That's a separate task in my outline (no github issue yet). Whereas, this task is just to port something like class loader, which could be used manually to load things until we have a pluginlib equivalent.

@gbiggs The other thing to consider in ROS 2 is how we could do plugins with C (class loader is C++ specific). That's out of scope for this issue, but else we have to think about.

@gbiggs
Copy link
Member

gbiggs commented Jul 16, 2015

A structure of callbacks might be the way to go there. Any plugin system must have a well-defined API that the plugin provides, so asking for a plugin writer to provide a structure of callbacks is not too much to ask, I think.

@dirk-thomas
Copy link
Member

It does not have to be a structure of callbacks. ROS 1 class_loader already supports loading libraries with an arbitrary but known API which is defined by the type used for the plugin registration / lookup.

@wjwwood
Copy link
Member Author

wjwwood commented Jul 17, 2015

@dirk-thomas I think he's referring to C, where, AFAIK, class_loader's techniques will not work since it relies on class constructors/destructors and anonymous namespaces.

@wjwwood
Copy link
Member Author

wjwwood commented Jul 17, 2015

Though I'm not sure the structure of callbacks really addresses the anonymous registration issue. At any rate, class_loader expects a class with methods as the plugin interface, and a struct of callbacks is the closest thing to that in C. Still, maybe that's best left to think about until we need a C plugin loading system.

@gbiggs
Copy link
Member

gbiggs commented Jul 28, 2015

I don't think it would be too much of a burden on developers of C plugins to require that they provide a known function that can be called by the plugin loader. We use "pluginname_initialise".

@dirk-thomas dirk-thomas self-assigned this Aug 4, 2015
@dirk-thomas dirk-thomas added ready Work is about to start (Kanban column) in progress Actively being worked on (Kanban column) and removed ready Work is about to start (Kanban column) labels Aug 4, 2015
@dirk-thomas
Copy link
Member

I have created a ros2 branch in the class_loader repo (https://github.com/ros/class_loader/tree/ros2).

The new branch:

  • uses ament
  • uses C++11
  • updates constness of signatures
  • removes the Boost dependency
  • general clean up

Please "review" if this is sufficient for this.

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Aug 11, 2015
@gbiggs
Copy link
Member

gbiggs commented Aug 11, 2015

+1. Looks like it does the job to me.

@esteve
Copy link
Member

esteve commented Aug 12, 2015

+1

1 similar comment
@wjwwood
Copy link
Member Author

wjwwood commented Aug 12, 2015

+1

@tfoote
Copy link
Contributor

tfoote commented Aug 12, 2015

Overall looks good. As this is a fork of the existing implementation currently without a rename we should not reset the version number, but bump it up following semantic versioning which would be a major version number up and not delete the old changelogs etc.

@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Aug 18, 2015
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

No branches or pull requests

5 participants