-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add topologies for testing #125
Conversation
irobot_benchmark/CMakeLists.txt
Outdated
@@ -33,12 +33,89 @@ set(TOPOLOGY_FILES | |||
topology/debug_sierra_nevada_best_effort.json | |||
topology/debug_mont_blanc_reliable.json | |||
topology/debug_mont_blanc_best_effort.json | |||
topology/debug_white_mountain_best_effort.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what is the purpose/goal of the debug_*_best_effort.json
topologies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to have the debug node of the white_mountain.json
topology.
There's also the white_mountain_fixed_size_debug.json
but that's the debug of the "fixed size" version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a topology with a single node that subscribes to all topics published by the white-mountain.
it simulates a "debug station" connecting to the robot for logging or visualization.
FWIW, these debug topologies could be automatically generated
irobot_benchmark/CMakeLists.txt
Outdated
) | ||
|
||
install(FILES | ||
${TOPOLOGY_FILES} | ||
DESTINATION lib/${PROJECT_NAME}/topology) | ||
|
||
set(SP topology/tests/single_process) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about removing the tests/
subdirectory? The new topologies are already "identified" via the subdirectories that contain them, like full_topologies
, multi_process
, and single_process
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5530b70
Like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Another question though, the white_mountain_fixed_size_debug.json specifies "qos_reliability":"best_effort"
. But the analogous sierra_nevada_fixed_size_debug.json does not. Intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 389c1d7 I've renamed the topologies specifying the reliability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing a duplicate debug_white_mountain_best_effort.json in the top-level topology
directory. But it is really (correctly, in my view) beneath the full_topologies/
directory. See
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, just removed it.
Add topologies for testing: