Conversation
edf4451 to
b6dfd2d
Compare
f1817aa to
efeb6dc
Compare
| def test_linktime_composition(): | ||
| if os.name == 'nt': | ||
| print('The link time registration of classes does not work on Windows') | ||
| return |
There was a problem hiding this comment.
The classes exported in libraries which are linked by the linker are not registered in class_loader on Windows. Therefore this test is being skipped on Windows.
There was a problem hiding this comment.
You can mark the test as skipped with something like this:
from unittest.case import SkipTest
# ...
def test_linktime_composition():
if os.name == 'nt':
print('The link time registration of classes does not work on Windows')
raise SkipTestIt looks something like this:
...............................S
----------------------------------------------------------------------
Ran 32 tests in 0.537s
OK (SKIP=1)
I think you can also import it from nose but it's just a forward to this exception.
There was a problem hiding this comment.
Since I don't see an easy way to address this that sounds like a good idea. Done: http://ci.ros2.org/job/ci_windows/1857/testReport/(root)/test_composition__rmw_fastrtps_cpp_Release/
8f79990 to
fe14590
Compare
1ffdd9b to
78d3c36
Compare
|
I have added the following:
Ready for review 🎉
|
There was a problem hiding this comment.
lgtm
I would like to see an example of a "header less" component. I recognize that the talker and listener components you have here need headers so that you can make the manual_composition.cpp demo work. However, if I think about the minimal equivalent to my_node.cpp with it's own main, then it would be a class that is defined and implemented in place without a header. This should work for all cases except manual composition, but I expect that "manual composition" to be a quite rare pattern for normal users. And many will likely choose to forego the header if possible. Those users will likely take the idea that a header is required (even if it is not true) as a drawback to this approach.
| def test_linktime_composition(): | ||
| if os.name == 'nt': | ||
| print('The link time registration of classes does not work on Windows') | ||
| return |
There was a problem hiding this comment.
You can mark the test as skipped with something like this:
from unittest.case import SkipTest
# ...
def test_linktime_composition():
if os.name == 'nt':
print('The link time registration of classes does not work on Windows')
raise SkipTestIt looks something like this:
...............................S
----------------------------------------------------------------------
Ran 32 tests in 0.537s
OK (SKIP=1)
I think you can also import it from nose but it's just a forward to this exception.
78d3c36 to
bee5cfd
Compare
|
I would rather not duplicate the talker / listener code just to make it header less. Our other demos / examples also lack the ability to serve as a copy-n-paste template at the moment. Therefore I would not aim for that goal in this PR. |
bee5cfd to
5d31a8e
Compare
5d31a8e to
b265539
Compare
|
I don't desire a copy and paste template, I was just suggesting that this is something else that could be demonstrated. Which is something we have duplicated code for in other demos, where appropriate. I don't think it's necessary so long as you are sure a headerless node works in the non manual composition cases. Though I think it would be nice to have. |
|
I will merge the current state. I further examples are desired they can be added in a new PR. |
This package contains multiple different ways how multiple nodes can be composed into a single process. The talker and listener nodes are compiled into shared libraries in all cases.
manual_composition:mainuses the headers of the nodes and instantiates concrete nodeslinktime_composition:class_loadermainusingclass_loader(on the library "")-Wl,--no-as-needed(since it doesn't directly require any symbols from the libraries)dlopen_composition:class_loadermainusingclass_loaderto load the shared libraries--