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.

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

[12371] Waitset implementation #2128

Merged
merged 7 commits into from
Aug 31, 2021
Merged

[12371] Waitset implementation #2128

merged 7 commits into from
Aug 31, 2021

Conversation

MiguelCompany
Copy link
Member

Add WaitSet, GuardCondition, and StatusCondition implementation and tests.

Documentation PR: eProsima/Fast-DDS-docs#279

@MiguelCompany
Copy link
Member Author

@richiprosima Please test windows

@EduPonz EduPonz added this to the v2.4.0 milestone Aug 12, 2021
@IkerLuengo
Copy link
Contributor

@richiprosima Please test linux

IkerLuengo
IkerLuengo suggested changes Aug 18, 2021
View reviewed changes
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Entity::get_statuscondition() is returning a const reference. We should be returning a non-const reference so that we can apply modifications, like StatusCondition::set_enabled_statuses(), which is a mutator method. Although Condition itself has no mutating methods, we cannot assume that its derivatives have none.

This was found during the review of the examples in the documentation

And while we are at it, we could also remove the logWarning stating that get_statuscondition is not implemented

@IkerLuengo IkerLuengo changed the title Waitset implementation [12371] Waitset implementation Aug 23, 2021
MiguelCompany and others added 7 commits August 30, 2021 12:37
@MiguelCompany
WaitSet detail classes (#1989)
8bd41ba 
* Refs 11608. Required changes on Condition API.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. ConditionNotifier with empty implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl with empty implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. ConditionNotifier unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Added unordered_vector.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. ConditionNotifier implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Condition management on WaitSetImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Implenting wait and wake_up on WaitSetImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Solving link issues.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Doxygen improvements.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Check vector size on test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
WaitSet, Condition and GuardCondition implementation (#1991)
57653be 
* Refs 11608. Add ConditionNotifier to condition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl attaches to notifier on attach_condition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl detaches from notifier on detach_condition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Adding expectations to WaitSetImplTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Waiting on an empty WaitSet should timeout.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Add WaitSetImpl to WaitSet.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Adapting ConditionTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Added GuardCondition implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608.Unregister conditions on WaitSetImpl destruction.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Fixing WaitSetImplTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Explicit memory order on GuardCondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Includes alpha-sort.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11609. Explicitly setting GuardCondition initial value.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
StatusCondition unit tests (#2026)
97fc571 
* Refs 11823. Added StatusConditionImpl header.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added StatusConditionImpl empty implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added unit test infrastructure.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added StatusConditionImplTests tests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Entity returns non-const StatusCondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added StatusCondition::get_impl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added basic test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added trigger test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Fixed warnings.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Using StrictMock.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Improved doxydoc.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
StatusCondition implementation (#2029)
b949d0c 
* Refs 11824. Added attributes to StatusConditionImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Compositing StatusConditionImpl inside StatusCondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Fixed link issues.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Adding implementation to StatusConditionImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Fixed Entity::get_statuscondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Added StatusCondition::get_trigger_value.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Changed EntityTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Notify status changes (#2030)
1e959ab 
* Refs 11835. Fixed listener calls for deadline statuses.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Basic notifications on DataWriterImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Basic notifications on DataReaderImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Implementing DataReader::get_subscription_matched_status.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Fixed unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Implementing DataWriter::get_publication_matched_status.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Fixed unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Implement Entity::get_status_changes through StatusConditionImpl::get_raw_status.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Added StatusConditionImpl::get_raw_status to unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@IkerLuengo @MiguelCompany
Status condition blackbox (#2052)
04b6110 
* Refs 11834. Linux compilation errors with gmock 1.10

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. pubsub reader/writer with waitsets instead of listeners

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Waitset blackbox

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Uncrustify

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Methods to check timeouts in waitsets

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Check that when no data is received, the waitset times out

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. signed-unsigned comparison

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Suggested changes

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@MiguelCompany
Fix after rebase
64116d9 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany force-pushed the feature/waitsets/impl branch from b77d112 to 64116d9 Compare August 30, 2021 10:55
@MiguelCompany
Copy link
Member Author

Entity::get_statuscondition() is returning a const reference. We should be returning a non-const reference so that we can apply modifications, like StatusCondition::set_enabled_statuses(), which is a mutator method. Although Condition itself has no mutating methods, we cannot assume that its derivatives have none.

This was found during the review of the examples in the documentation

And while we are at it, we could also remove the logWarning stating that get_statuscondition is not implemented

This is part of the changes .... Were you looking at the code in master, perhaps?

IkerLuengo
IkerLuengo approved these changes Aug 31, 2021
View reviewed changes
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment

Choose a reason for hiding this comment

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

This is part of the changes .... Were you looking at the code in master, perhaps?

Sincerely, I do not know what I was looking 😥

@MiguelCompany MiguelCompany merged commit 930d786 into master Aug 31, 2021
@MiguelCompany MiguelCompany deleted the feature/waitsets/impl branch August 31, 2021 06:57
SamuelWHalodi pushed a commit to SamuelWHalodi/Fast-DDS that referenced this pull request Oct 4, 2021
@MiguelCompany @IkerLuengo @SamuelWHalodi
Waitset implementation (eProsima#2128)
385ea61 
* WaitSet detail classes (eProsima#1989)

* Refs 11608. Required changes on Condition API.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. ConditionNotifier with empty implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl with empty implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. ConditionNotifier unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Added unordered_vector.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. ConditionNotifier implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Condition management on WaitSetImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Implenting wait and wake_up on WaitSetImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Solving link issues.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Doxygen improvements.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11656. Check vector size on test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* WaitSet, Condition and GuardCondition implementation (eProsima#1991)

* Refs 11608. Add ConditionNotifier to condition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl attaches to notifier on attach_condition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. WaitSetImpl detaches from notifier on detach_condition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Adding expectations to WaitSetImplTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Waiting on an empty WaitSet should timeout.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Add WaitSetImpl to WaitSet.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Adapting ConditionTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Added GuardCondition implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608.Unregister conditions on WaitSetImpl destruction.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Fixing WaitSetImplTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Explicit memory order on GuardCondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Includes alpha-sort.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11608. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11609. Explicitly setting GuardCondition initial value.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* StatusCondition unit tests (eProsima#2026)

* Refs 11823. Added StatusConditionImpl header.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added StatusConditionImpl empty implementation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added unit test infrastructure.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added StatusConditionImplTests tests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Entity returns non-const StatusCondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added StatusCondition::get_impl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added basic test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Added trigger test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Fixed warnings.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Using StrictMock.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11823. Improved doxydoc.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* StatusCondition implementation (eProsima#2029)

* Refs 11824. Added attributes to StatusConditionImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Compositing StatusConditionImpl inside StatusCondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Fixed link issues.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Adding implementation to StatusConditionImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Fixed Entity::get_statuscondition.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Added StatusCondition::get_trigger_value.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Linters.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11824. Changed EntityTests.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Notify status changes (eProsima#2030)

* Refs 11835. Fixed listener calls for deadline statuses.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Basic notifications on DataWriterImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Basic notifications on DataReaderImpl.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Implementing DataReader::get_subscription_matched_status.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Fixed unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Implementing DataWriter::get_publication_matched_status.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Fixed unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Implement Entity::get_status_changes through StatusConditionImpl::get_raw_status.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 11835. Added StatusConditionImpl::get_raw_status to unit test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Status condition blackbox (eProsima#2052)

* Refs 11834. Linux compilation errors with gmock 1.10

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. pubsub reader/writer with waitsets instead of listeners

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Waitset blackbox

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Uncrustify

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Methods to check timeouts in waitsets

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Check that when no data is received, the waitset times out

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. signed-unsigned comparison

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Refs 11834. Suggested changes

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>

* Fix after rebase

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Co-authored-by: IkerLuengo <57146230+IkerLuengo@users.noreply.github.com>
Signed-off-by: Samuel Wilhelmsson <samuel@halodi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IkerLuengo IkerLuengo IkerLuengo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@MiguelCompany @IkerLuengo @EduPonz