Support actions / Add communication benchmarks directory #137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
April 29, 2024 11:16
mauropasse
changed the title
alsora
reviewed
Nov 18, 2024
irobot_benchmark/comms_benchmark/scripts/actions/run_mix_process_benchmarks.sh
Outdated
Show resolved
Hide resolved
alsora
reviewed
Nov 18, 2024
irobot_benchmark/comms_benchmark/scripts/actions/run_mix_process_benchmarks.sh
Outdated
Show resolved
Hide resolved
alsora
reviewed
Nov 18, 2024
irobot_benchmark/comms_benchmark/scripts/actions/run_mix_process_benchmarks.sh
Outdated
Show resolved
Hide resolved
alsora
reviewed
Nov 18, 2024
| topology_basename.substr(0, topology_basename.length() - 5) + "_log"; | ||
| std::string result_dir_name; | ||
|
|
||
| if (result_dir != "") { |
There was a problem hiding this comment.
Suggested change
| if (result_dir != "") { | |
| if (!result_dir.empty()) { |
alsora
reviewed
Nov 18, 2024
| @@ -219,6 +252,7 @@ class PerformanceNodeBase | |||
| // Client blocking call does not work with timers | |||
| // Use a lock variable to avoid calling when you are already waiting | |||
| std::atomic<bool> m_client_lock {false}; | |||
| std::atomic<bool> m_action_client_lock {false}; | |||
There was a problem hiding this comment.
if i recall correctly, this assumes that you are only doing 1 request from each node.
i think that each client should have their own "lock", rather than 1 per node
alsora
reviewed
Nov 18, 2024
| // A service-name indexed map to store the client pointers with their | ||
| // trackers. | ||
| std::map<std::string, ClientsTuple> m_clients; | ||
| std::map<std::string, ActionClientsTuple> m_action_clients; | ||
| std::map<std::string, ActionServersTuple> m_action_servers; |
There was a problem hiding this comment.
minor: these could probably be unordered maps
alsora
reviewed
Nov 18, 2024
| auto client = std::static_pointer_cast<rclcpp_action::Client<Action>>(std::get<0>(client_tuple)); | ||
|
|
||
| // Wait for action server to become available | ||
| if (!client->wait_for_action_server(std::chrono::seconds(1))) { |
There was a problem hiding this comment.
this will block the whole node (e.g. maybe there are pubs-subs also that should be running).
I wonder if we should wait 0 here and just skip the request if not discovered.
then the test validation should check how many goals were sent.
maybe the same comment applies also to services
alsora
reviewed
Nov 18, 2024
|
|
||
| size_t non_sep_pos = split_right.find_first_not_of(sep); | ||
| if (non_sep_pos == std::string::npos) { | ||
| std::cerr << "Non-separator character not found after separator in line: " << line << std::endl; |
There was a problem hiding this comment.
what can cause this?
should we assert?
alsora
reviewed
Nov 18, 2024
| } else if (action_client_json.find("period_ms") != action_client_json.end()) { | ||
| period_ms = action_client_json["period_ms"]; | ||
| } else { | ||
| std::cout << "Error! Action Clients must set period_ms or freq_hz in json file" << std::endl; |
alsora
reviewed
Nov 18, 2024
| @@ -39,7 +39,7 @@ | |||
| {"topic_name":"parana", "msg_type":"stamped3_float32"} | |||
| ], | |||
| "publishers": [ | |||
| {"topic_name": "salween", "msg_type": "stamped12_float32", "period_ms": 100, "msg_pass_by":"loaned_msg"} | |||
| {"topic_name": "salween", "msg_type": "stamped12_float32", "period_ms": 100, "msg_pass_by":"shared_ptr"} | |||
mauropasse
force-pushed
the
mauro/comms-benchmark-iron
branch
2 times, most recently
from
1fb00f4 to
d9fdf5c
Compare
mauropasse
force-pushed
the
mauro/comms-benchmark-iron
branch
from
d9fdf5c to
5a939f0
Compare
mauropasse
force-pushed
the
mauro/comms-benchmark-iron
branch
from
5a939f0 to
08f967b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commits:
d50eebd Support Actions and add comms_benchmark dir
comms_benchmarkdirectory contains scripts to run benchmarks plus topologies:10B/100KB/1MB/4MBirobot_benchmark/comms_benchmark/scripts/run_all_benchmarks.shis the script to run all tests.irobot_benchmark/comms_benchmark/scripts/parse-resultscontains scripts to parse the results.jsontopology files.e67baa4 Improve error handling in data parsing and file operations
2fdd9bc warning if negative latency for multi-host benchmarks
5d4e998 Show latency even if msg arrived late
0f646cf Add option to create results directory
9def392 Remove irobot_events_executor dependency
24cf6b2 Remove option and always name threads