Iox 2408 multi domain id req rep

[jmyvalour]

Notes for Reviewer

Added support for Server, Client multi domain ID, as part of the experimental Node (already supporting Subscribe/Publisher)

Added setDefaultRuntime to overload the RuntimeFactory in order for discovery service to use by default the poshruntime interface provided by the node.

Pre-Review Checklist for the PR Author

1. Code follows the coding style of CONTRIBUTING.md
2. Tests follow the best practice for testing
3. Changelog updated in the unreleased section including API breaking changes
4. Branch follows the naming format (iox-123-this-is-a-branch)
5. Commits messages are according to this guideline
   • Commit messages have the issue ID (iox-#123 commit text)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
6. Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
7. Relevant issues are linked
8. Add sensible notes for the reviewer
9. All checks have passed (except task-list-completed)
10. Assign PR to reviewer

Checklist for the PR Reviewer

• Consider a second reviewer for complex new features or larger refactorings
• Commits are properly organized and messages are according to the guideline
• Code according to our coding style and naming conventions
• Unit tests have been written for new behavior
• Public API changes are documented via doxygen
• Copyright owner are updated in the changed files
• All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

Related tickets:
#1129
#2408

[mossmaurice]

@jmyvalour Thanks for this PR!

A few organizational topics, could you please:

1. Add iox-#2408 Foobar to each of your commit messages (more info see here)
2. Run clang-format (there are some failures)
3. Use this pull request template, we can then tick the checkboxes and make sure not to forget anything

[mossmaurice]

Thanks for the PR! I left a few remarks and questions. @elBoberido should have a look as well.

[jmyvalour]

hey @mossmaurice ,

Thank you for your review, I added the Listener support too and the required tests to have a more extensive coverage. Exhaust, Typed / Untyped server, client, and a listener creation through the Node.

Open question would be about the setDefaultRuntime and how you want to have it internally, migh be a question for @elBoberido

[codecov]

Codecov Report

Attention: Patch coverage is 8.65385% with 95 lines in your changes missing coverage. Please review.

Project coverage is 78.25%. Comparing base (4449fcf) to head (4bc2454).

Files with missing lines	Patch %	Lines
...al/include/iox/posh/experimental/detail/client.inl	0.00%	26 Missing ⚠️
...al/include/iox/posh/experimental/detail/server.inl	0.00%	25 Missing ⚠️
.../include/iox/posh/experimental/detail/listener.inl	0.00%	10 Missing ⚠️
iceoryx_posh/experimental/source/node.cpp	0.00%	6 Missing ⚠️
...include/iceoryx_posh/internal/popo/client_impl.inl	0.00%	3 Missing ⚠️
...include/iceoryx_posh/internal/popo/server_impl.inl	0.00%	3 Missing ⚠️
...iceoryx_posh/internal/popo/untyped_client_impl.inl	0.00%	3 Missing ⚠️
...iceoryx_posh/internal/popo/untyped_server_impl.inl	0.00%	3 Missing ⚠️
iceoryx_posh/include/iceoryx_posh/popo/client.hpp	0.00%	3 Missing ⚠️
iceoryx_posh/include/iceoryx_posh/popo/server.hpp	0.00%	3 Missing ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2425      +/-   ##
==========================================
- Coverage   78.68%   78.25%   -0.43%     
==========================================
  Files         440      445       +5     
  Lines       16988    17091     +103     
  Branches     2361     2373      +12     
==========================================
+ Hits        13367    13375       +8     
- Misses       2740     2835      +95     
  Partials      881      881              

Flag	Coverage Δ
unittests	78.05% <8.65%> (-0.43%)	⬇️
unittests_timing	15.31% <7.69%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...xperimental/include/iox/posh/experimental/node.hpp	0.00% <ø> (ø)
...include/iceoryx_posh/internal/popo/base_client.inl	89.18% <100.00%> (+0.45%)	⬆️
...include/iceoryx_posh/internal/popo/base_server.inl	89.47% <100.00%> (+0.43%)	⬆️
...nternal/popo/building_blocks/chunk_distributor.inl	95.62% <100.00%> (ø)
...ceoryx_posh/include/iceoryx_posh/popo/listener.hpp	0.00% <ø> (ø)
...posh/include/iceoryx_posh/runtime/posh_runtime.hpp	100.00% <ø> (ø)
...erimental/include/iox/posh/experimental/client.hpp	0.00% <0.00%> (ø)
...erimental/include/iox/posh/experimental/server.hpp	0.00% <0.00%> (ø)
...include/iceoryx_posh/internal/popo/client_impl.inl	92.10% <0.00%> (-7.90%)	⬇️
...include/iceoryx_posh/internal/popo/server_impl.inl	92.30% <0.00%> (-7.70%)	⬇️
... and 10 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jmyvalour]

Hello @elBoberido @mossmaurice and thank you for your time reviewing this PR,
Regarding the windows build, it looks like this is not related to the PR, let me know if I need to provide anything else,

D:\a\iceoryx\iceoryx\iceoryx_hoofs\test\moduletests\test_time_unit_duration.cpp(1119,44): error C2039: 'system_clock': is not a member of 'std::chrono'

[elBoberido]

@jmyvalour you might need to rebase to main. There was a fix for this error some time ago. You most probably have branched away before that fix was merged.

[jmyvalour]

@elBoberido just rebased remote main into the PR and retrieved the window build fix indeed, thx for the info

[elBoberido]

Can you also please add an example to iceoryx_examples/experimental/node. You can copy iceoryx_examples/request_response/client_cxx_waitset.cpp to iox_cpp_node_client.cpp and iceoryx_examples/request_response/server_cxx_listener.cpp to iox_cpp_node_server.cpp.

When done, please also add that example to the integrationtests:

• copy iceoryx_integrationtests/iceoryx_integrationtests/test_experimental_node.py to e.g. test_experimental_node_request_response.py
• rename test_experimental_node.py to e.g. `test_experimental_node_publish_subscribe.py``
• add/adjust the test names in iceoryx_integrationtests/CMakeLists.txt
• add/adjust the test names in iceoryx_integrationtests/package.xml

[elBoberido]

Looks good. Just one minor nitpik. Once that is removed and the CI is green, this can be merged

[elBoberido]

The code can be merged. I noticed you used 1295 as issue number for the last commits. Can you please change that to 2408, like the initial commits.

[jmyvalour]

@elBoberido apologies for the mistake, I rewrote the commit messages with the correct ticket numbers,

[elBoberido]

@jmyvalour the git commit history is a bit messy. Could you please execute the following commands on your local checkout

git checkout main
git remote add upstream [email protected]:eclipse-iceoryx/iceoryx.git
git fetch --all
git branch --set-upstream-to=upstream/main
git pull
git checkout iox-2408-multi-domain-id-req-rep
git rebase main

This should remove the unrelated commits from this PR

[elBoberido]

Thanks very much. Now it's just waiting for the CI and hoping for the best

[jmyvalour]

Thank you for your help cleaning the PR and the commits history,
Looks like the ubuntu bazel build refused to get the ncurse package, does a new run generally solve this issue ?

[elBoberido]

@jmyvalour we did not yet encounter this problem but I just tried a local build and it works. I triggered a new run for the bazel job. Let's see if we are lucky.

[elBoberido]

@jmyvalour congratulations to your first iceoryx PR. Thanks for your patience and looking forward to the next PR.

[jmyvalour]

Thank you @elBoberido , @mossmaurice for your help and support through this PR, will try to be more effective for the next one following your comments, not sure when I will get the time to go into the service discovery PR but I keep it in mind and prioritize.