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.

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

added device for synchronization #2772

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

ste93
Copy link
Contributor

@ste93 ste93 commented Dec 6, 2021

systemReady {#master}

yarp

devices

  • added systemReady device that opens some ports when called. It is useful for synchronize processes opened with yarprobotinterface and yarpmanager to avoid crashes when some processes are not started

@update-docs
Copy link

update-docs bot commented Dec 6, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update the release notes by adding a file in doc/release/<target_branch>, based on your changes.

@ste93 ste93 temporarily deployed to code-analysis December 6, 2021 16:07 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 6, 2021 16:07 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 6, 2021 16:07 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 7, 2021 09:43 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 7, 2021 09:43 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 7, 2021 09:43 Inactive
close();
return false;
}
port_pointers_list.push_back(current_port);
Copy link
Member

Choose a reason for hiding this comment

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

This should go before the if clause, otherwise the newly constructed current_port will not be accessible by the close handler for deletion, thus causing a memory leak.

bool SystemReady_nws_yarp::close()
{
for (auto elem: port_pointers_list){
elem->close();
Copy link
Member

Choose a reason for hiding this comment

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

There used to be a delete here in previous revisions, it needs to be kept to avoid memory leaks.

@ste93 ste93 temporarily deployed to code-analysis December 7, 2021 13:42 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 7, 2021 13:42 Inactive
@ste93 ste93 temporarily deployed to code-analysis December 7, 2021 13:42 Inactive
@ste93 ste93 marked this pull request as draft December 7, 2021 14:45
@traversaro traversaro removed their request for review May 30, 2022 08:27
@randaz81 randaz81 force-pushed the master branch 3 times, most recently from 71900d2 to ead1caf Compare November 30, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants