[20543] Examples refactor: Hello World by JesusPoderoso · Pull Request #4547 · eProsima/Fast-DDS

JesusPoderoso · 2024-03-11T15:25:55Z

Description

This PR should be merge right after:

[20543] Create Participant with default profile (use environment XML configuration) #4534

This PR is the first of a suite of PR which would make a refactor in the repository examples.
It is intended to apply to most of the examples, by making them homogeneous, more understandable, and more specific to the case they were meant to be.

In this hello world example, the key changes are:

remove --env option and set that environment behavior as default
add subscriber waitsets class and its usage
provide XML profiles examples that target several scenarios (other tests as SampleConfig_Controller / Events / Multimedia)

The following changes apply to this and the remain examples:

remove the DDS prefix from the binaries
snake case in binary names and directory names
launch only one endpoint per binary (always running them in different shells)
unify args names and utility
write proper README.md with the purpose of the example and expected inputs, outputs, behavior… (do not include options in readme → enhance user to run --help )
implement listener callback in the same class (inheritance)
include initialization ( init() ) methods in the constructors
improve parsing options to be a black box to the user

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
Any new/modified methods have been properly documented using Doxygen.
Changes are ABI compatible.
Changes are API compatible.
New feature has been added to the versions.md file (if applicable).
N/A New feature has been documented/Current behavior is correctly described in the documentation.
N/A Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

examples/cpp/hello_world/cli_options.hpp

test/examples/hello_world.compose.yml

examples/cpp/hello_world/HelloWorldPublisher.cpp

test/examples/hello_world.compose.yml

test/examples/test_examples.py

examples/cpp/hello_world/CMakeLists.txt

examples/cpp/hello_world/HelloWorld.idl

examples/cpp/hello_world/CLIParser.hpp

examples/cpp/hello_world/Subscriber.cpp

examples/cpp/hello_world/SubscriberWaitset.hpp

examples/cpp/hello_world/SubscriberWaitset.cpp

test/examples/test_examples.py

examples/cpp/hello_world/CLIParser.hpp

examples/cpp/hello_world/CMakeLists.txt

EduPonz

We're definitely getting there! I like the idea of the classes not having any static members.

Now, I have a "crazy" idea to simplify the main and to also group some of the subscribers' code. Right now, all three entities have similar public API:

A ctor
A run member function with no args
A stop member function with no args

We can do some hierarchy of classes:

classDiagram
class Entity {
    +run() void
    +stop() void
   +create_entity(const hello_world_config& config)$ std::shared_ptr~Entity~
}
Entity <|-- Publisher
Entity <|-- AbstractSubscriber : Optional grouping of subscriber code
AbstractSubscriber <|-- Subscriber
AbstractSubscriber <|-- WaitSubscriber

Entity ..> Publisher : creates
Entity ..> Subscriber : creates
Entity ..> WaitSubscriber : creates

We would have a base class for the entities which can also act as a factory of the specializations via an static method. This way you don't need all those pointers, just the one, nor do you need any switches. You'd just create the entity passing the config and then spawn the thread to run it's run method. Since the entity is a shared_ptr, you wouldn't need to delete it either. You can see a PoC here.

examples/cpp/hello_world/HelloWorld_main.cpp

examples/cpp/hello_world/Publisher.cpp

examples/cpp/hello_world/HelloWorld_main.cpp

Mario-DL · 2024-03-18T15:10:14Z

We're definitely getting there! I like the idea of the classes not having any static members.

Now, I have a "crazy" idea to simplify the main and to also group some of the subscribers' code. Right now, all three entities have similar public API:

A ctor

A run member function with no args

A stop member function with no args

We can do some hierarchy of classes:
classDiagram
class Entity {
    +run() void
    +stop() void
   +create_entity(const hello_world_config& config)$ std::shared_ptr~Entity~
}
Entity <|-- Publisher
Entity <|-- AbstractSubscriber : Optional grouping of subscriber code
AbstractSubscriber <|-- Subscriber
AbstractSubscriber <|-- WaitSubscriber

Entity ..> Publisher : creates
Entity ..> Subscriber : creates
Entity ..> WaitSubscriber : creates
Loading
We would have a base class for the entities which can also act as a factory of the specializations via an static method. This way you don't need all those pointers, just the one, nor do you need any switches. You'd just create the entity passing the config and then spawn the thread to run it's run method. Since the entity is a shared_ptr, you wouldn't need to delete it either. You can see a PoC here.

(NIT) Apart from the internally agreed things I would call the creation method (or global free function) make_app() instead of create 😃

examples/cpp/hello_world/main.cpp

examples/cpp/hello_world/CMakeLists.txt

examples/cpp/hello_world/main.cpp

examples/cpp/hello_world/CLIParser.hpp

EduPonz

This is turning out nicely, great job! 🤖

examples/cpp/hello_world/CLIParser.hpp

test/examples/test_examples.py

examples/cpp/hello_world/Application.hpp

examples/cpp/hello_world/WaitsetSubscriberApp.cpp

examples/cpp/hello_world/ListenerSubscriberApp.cpp

examples/cpp/hello_world/PublisherApp.cpp

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> Refs #20543: Fix installation path Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> A Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

JesusPoderoso added the ci-pending PR which CI is running label Mar 11, 2024

JesusPoderoso added this to the v3.0.0 milestone Mar 11, 2024

EduPonz suggested changes Mar 12, 2024

View reviewed changes

EduPonz added the temporarily-blocked PR must be merged after another one label Mar 12, 2024

JesusPoderoso force-pushed the feature/consider_profile_domain_id branch from 48f5bad to bb375a4 Compare March 12, 2024 11:06

JesusPoderoso force-pushed the feature/example_refactor/hello_world branch from c366a69 to 6f2a1e0 Compare March 12, 2024 16:20

JesusPoderoso force-pushed the feature/consider_profile_domain_id branch 2 times, most recently from 7af5a31 to d73f963 Compare March 13, 2024 07:33

JesusPoderoso force-pushed the feature/example_refactor/hello_world branch from 6f2a1e0 to c6e0741 Compare March 13, 2024 09:29

JesusPoderoso force-pushed the feature/consider_profile_domain_id branch from d73f963 to 2dc171a Compare March 14, 2024 08:04

JesusPoderoso force-pushed the feature/example_refactor/hello_world branch from 3cd6aa0 to 30317aa Compare March 14, 2024 08:11

EduPonz suggested changes Mar 14, 2024

View reviewed changes

JesusPoderoso force-pushed the feature/example_refactor/hello_world branch from 813cb7d to fe33b04 Compare March 14, 2024 15:39

EduPonz reviewed Mar 16, 2024

View reviewed changes

EduPonz suggested changes Mar 16, 2024

View reviewed changes

JesusPoderoso force-pushed the feature/example_refactor/hello_world branch 5 times, most recently from b2a2839 to 5064ab8 Compare March 20, 2024 15:22

EduPonz added temporarily-blocked PR must be merged after another one and removed temporarily-blocked PR must be merged after another one labels Mar 20, 2024

EduPonz reviewed Mar 20, 2024

View reviewed changes

examples/cpp/hello_world/main.cpp Outdated Show resolved Hide resolved

examples/cpp/hello_world/CMakeLists.txt Show resolved Hide resolved

examples/cpp/hello_world/main.cpp Outdated Show resolved Hide resolved

examples/cpp/hello_world/CLIParser.hpp Outdated Show resolved Hide resolved

EduPonz suggested changes Mar 20, 2024

View reviewed changes

JesusPoderoso added a commit that referenced this pull request Mar 21, 2024

Refs #4547: [ARS] Install XML profile

6ff2753

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

JesusPoderoso added a commit that referenced this pull request Mar 21, 2024

Refs #4547: [ARS] Set EntityKind as enum class

0a6b63a

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

JesusPoderoso added a commit that referenced this pull request Mar 21, 2024

Refs #4547: [ARS] Remove ternary operator

d8d4973

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

JesusPoderoso added a commit that referenced this pull request Mar 21, 2024

Refs #4547: [ARS] Simplify running message

3e0d744

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

JesusPoderoso added a commit that referenced this pull request Mar 21, 2024

Refs #4547: [ARS] Improve CLI usage message

8c87ce7

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

JesusPoderoso added 28 commits May 20, 2024 16:30

Refs #20543: Rename cli_options.hpp to CLIParser.hpp

15ac5f8

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Improve usage message

5c0b65e

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (2)

b789923

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (3)

223cf8c

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Include namespaces

ba30e0d

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Fix example installation

1cae90a

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> Refs #20543: Fix installation path Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (4)

79d960f

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> A Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (5)

866f593

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (6)

3a15e8f

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Refactor entities

99d8c45

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Install XML profile

71fdf6d

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Set EntityKind as enum class

02e9a0b

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Remove ternary operator

1678470

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Simplify running message

9699cba

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Improve CLI usage message

a32fa54

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Improve CLI parse

617a7f3

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: [ARS] Apply remain NITs

96ff8a2

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (7)

19e557a

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (8)

ec3e442

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions (9)

6e282fb

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Fix listener loop

cf1dcf8

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Add example in versions.md

83bd757

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Fix windows and mac errors and warnings

5a723d7

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Fix mac warning

1033d09

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Fix example test run without THIRDPARTY flags

ecd10b6

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply internal review suggestions

85eaadf

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply configuration example suggestions

4b4bf33

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #20543: Apply rev suggestions

416fce2

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

EduPonz approved these changes May 21, 2024

View reviewed changes

JesusPoderoso mentioned this pull request Jun 10, 2024

Bugfix: LifespanQoSExample Using the api incorrectly #4912

Closed

15 tasks

Conversation

JesusPoderoso commented Mar 11, 2024 • edited by EduPonz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Checklist

Reviewer Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EduPonz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mario-DL commented Mar 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EduPonz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JesusPoderoso commented Mar 11, 2024 •

edited by EduPonz

Loading