Skip to content

[23432] Pass value of TransportPriorityQosPolicy to transport layer #5933

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

[23432] Pass value of TransportPriorityQosPolicy to transport layer #5933

wants to merge 24 commits into from

Conversation

MiguelCompany
Copy link
Member

@MiguelCompany MiguelCompany commented Jul 14, 2025 •
edited by Mario-DL
Loading

Description

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.
  • Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • NO: Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • Changes are API compatible.
  • 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.
  • 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.
  • If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@MiguelCompany MiguelCompany added this to the v3.4.0 milestone Jul 14, 2025
@MiguelCompany MiguelCompany force-pushed the feature/23432 branch 3 times, most recently from 63f06ce to ebafca3 Compare July 14, 2025 10:19
@MiguelCompany MiguelCompany requested a review from Mario-DL July 14, 2025 10:20
@github-actions github-actions bot added the ci-pending PR which CI is running label Jul 14, 2025
@MiguelCompany MiguelCompany added the needs-review PR that is ready to be reviewed label Jul 14, 2025
@MiguelCompany MiguelCompany requested review from richiprosima and removed request for richiprosima July 14, 2025 12:14
@MiguelCompany MiguelCompany requested review from richiprosima and removed request for richiprosima July 15, 2025 06:07
@MiguelCompany MiguelCompany requested review from Mario-DL and removed request for Mario-DL July 15, 2025 06:27
Mario-DL
Mario-DL requested changes Jul 15, 2025
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the comments below, I was wondering whether we should include a logic in the transport_priority within TopicQoS i.e should we keep it updated to the same value as the one in the writer ? (I think that the standard does not mention nothing, nor any requirement in terms of compatibility rule)
What if the value is changed in runtime in the Topic QoS ? If we are just "ignoring" it I think we could document that.

@Mario-DL Mario-DL added doc-pending Issue or PR which is pending to be documented versions-pending labels Jul 15, 2025
@MiguelCompany MiguelCompany requested a review from Mario-DL July 15, 2025 12:57
Mario-DL
Mario-DL previously approved these changes Jul 16, 2025
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Mario-DL Mario-DL removed the needs-review PR that is ready to be reviewed label Jul 16, 2025
@MiguelCompany MiguelCompany mentioned this pull request Jul 17, 2025
Open
7 tasks
@MiguelCompany MiguelCompany removed the doc-pending Issue or PR which is pending to be documented label Jul 17, 2025
@MiguelCompany MiguelCompany requested a review from Mario-DL July 18, 2025 11:22
Mario-DL
Mario-DL previously approved these changes Jul 21, 2025
View reviewed changes
MiguelCompany
MiguelCompany commented Jul 21, 2025
View reviewed changes
@MiguelCompany
Refs #23432. Add transport_priority to SenderResource::send
fde4a23 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add new lambda that receives the priority to `SenderReso…
9500517 
…urce`

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Leave old method to avoid API break.
62443cc 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Update chaining transport interfaces.
0669899 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Deprecate old version.
34c48c3 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add transport_priority to `RTPSParticipantImpl::sendSy…
9f5036b 
…nc`.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add transport_priority to WriterAttributes.
773ec5d 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Fix sign of value in TransportPriorityQosPolicy.
7616d65 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Set field in WriterAttributes when enabling DataWriter.
6603b28 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Update field when updating QoS.
3666b01 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Pass transport_priority to UDPTransportInterface::send().
9be7521 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Pass transport_priority on locator filter in test_UDPv4T…
e4b8a35 
…ransport.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add blackbox tests.
763a8f5 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add XML support for transport_priority.
883728f 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Uncrustify.
1733dd8 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Fix comments.
4bb6cfa 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add const qualifier to priority argument.
e4e3341 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Add transport_priority to TCP and SHM.
a6bbd18 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Remove unnecessary lambda in ChainingSenderResource.
d27454f 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Update versions.md.
7bed167 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Move transport_priority from PublisherAttributes to Writ…
ceb844d 
…erQos.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Move XML for transport_priority inside qos tag.
e9829af 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Improve TRP-FUN-03 test.
5607bcd 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #23432. Make transport_priority updates be transmitted.
a3e1cce 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany MiguelCompany requested a review from Mario-DL July 22, 2025 08:19
Mario-DL
Mario-DL approved these changes Jul 22, 2025
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mario-DL Mario-DL Mario-DL approved these changes

Assignees
No one assigned
Labels
ci-pending PR which CI is running
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@MiguelCompany @Mario-DL