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

Extra options for benchmark #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Dec 11, 2019

This PR adds three command line arguments to the benchmark application

  • parameter-server: Enable/disable parameter server. The parameter server is by default activated, and therefore some services are created when creating a ROS 2 node. This has an impact in the memory consumption that is not representative of the use case, since the benchmark does not use these services.
  • wait-pdp: Enable/disable waiting for pdp discovery. Whether or not to wait for participant discovery before starting sampling. This flag is for instance necessary when testing for Fast-RTPS with participant whilisting, because in that case, each participant only matches with the ones in its whitelist, resulting in less "matches" than what performance_test::System::wait_pdp_discovery() is expecting.
  • wait-epd: Enable/disable waiting for edp discovery.

   * Enable/disable parameter server (since the application does not use it)
   * Enable/disable waiting for pdp discovery
   * Enable/disable waiting for edp discovery

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@alsora
Copy link
Collaborator

alsora commented Dec 11, 2019

Thank you for the contribution!

What about merging wait-pdp and wait-edp into a single option wait-discovery?
I don't see cases where it may be useful to enable only one but not the other (moreover, if wait-pdp is for any reason set to false, wait-edp does not make a lot of sense)

@EduPonz
Copy link
Author

EduPonz commented Dec 12, 2019

Yeah, actually spin() was taking a wait-discovery boolean that was always passed as true. The only reason to separate them is because in our try-outs with the participant white-list (we are filtering participants in the participant discovery protocol), we were actually commenting out wait_pdp_discovery() (as I describe in #8) and were only using wait_edp_discovery(). In this sense, if the feature request #8 is tackled, then it wouldn't make sense to separate wait-pdp and wait-edp. That would in my opinion be the best option.

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.

None yet

2 participants