[20401] Check History QoS inconsistencies

[JesusPoderoso]

Description

This PR introduces a check to ensure that the history QoS is consistent in both DataWriter and DataReader creation.
In this case, it checks that if history kind is set as KEEP_LAST, the history depth must be higher than zero.

Edit: Check between history depth and resource limits' max_samples_per_instance also included

@Mergifyio backport 2.12.x 2.11.x 2.10.x 2.6.x

Fixes #4365

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
• N/A Any new/modified methods have been properly documented using Doxygen.
• Changes are ABI compatible.
• Changes are API compatible.
• N/A New feature has been added to the versions.md file (if applicable).
• New feature has been documented/Current behavior is correctly described in the documentation.
  Related documentation PR: [20401] Improve History QoS documentation and compatibility rules Fast-DDS-docs#664
• 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.

[Mario-DL]

Changes makes sense to me and added tests assert the intended behavior.
Nonetheless there are a bunch of tests that needs to be fixed, mostly as a consequence of the consistency check between the max_samples_per_instance and history depth when KEEP_LAST.

For instance, the DDSDataWriter.HeartbeatWhileDestruction test would require setting the max_samples_per_instance datawriter qos to n_samples (1000) so that max_samples_per_instance >= history.depth

[Mario-DL]

In addition, it would be necessary to specify in the docs/doxygen that KEEP LAST with 0 is an inconsistency

[JesusPoderoso]

@richiprosima please test this

[Mario-DL]

@richiprosima please test mac

[elianalf]

@richiprosima please test windows

[JesusPoderoso]

@richiprosima please test mac

[JesusPoderoso]

I've launched a documentation CI manual run with the latest doc PR updates.

[Mario-DL]

@richiprosima please test mac

[JesusPoderoso]

CI issues unrelated to the PR. Ready to merge!

[Mario-DL]

LGTM

[EduPonz]

@Mergifyio backport 2.12.x 2.10.x 2.6.x

[mergify]

backport 2.12.x 2.10.x 2.6.x

✅ Backports have been created

• #4406 [20401] Check History QoS inconsistencies (backport #4375) has been created for branch 2.12.x
• #4407 [20401] Check History QoS inconsistencies (backport #4375) has been created for branch 2.10.x
• #4408 [20401] Check History QoS inconsistencies (backport #4375) has been created for branch 2.6.x but encountered conflicts

[michaelaeriksen]

This just broke my ros2 workspace. Now I get

[RTPS_QOS_CHECK Error] HISTORY DEPTH must be lower or equal to the max_samples_per_instance value. -> Function check_qos

when calling

rclcpp::Node::make_shared("Hello World");

[EduPonz]

This just broke my ros2 workspace. Now I get

[RTPS_QOS_CHECK Error] HISTORY DEPTH must be lower or equal to the max_samples_per_instance value. -> Function check_qos

when calling

rclcpp::Node::make_shared("Hello World");

Hi @michaelaeriksen, we realized this yesterday and have #4417 on the way to fix it, you can give it a try. In the mean time, the ROS 2 Rolling ros2.repos file was updated to pin Fast DDS to the commit previous to this one. In any case, it'd be fixed in some hours.