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

Clear the queue when consumer reads from it #96

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

corycrean
Copy link
Contributor

Currently, getLatestProduct calls queue_.waitDequeTimed to retrieve a package from the queue, which means that it always retrieves the oldest package from the queue (when the queue contains more than one package). The additional packages remain in the queue, which can cause the queue to fill up over time and lead to Pipeline producer overflowed! errors:

if (!queue_.tryEnqueue(std::move(p)))
{
URCL_LOG_ERROR("Pipeline producer overflowed! <%s>", name_.c_str());
}

This PR modifies getLatestProduct so that it always returns the most recent package from the queue, and discards older ones.

Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! This looks good to me!

However, I'd like to extend the function's documentation a little. @corycrean would you agree with my suggestion?

include/ur_client_library/comm/pipeline.h Outdated Show resolved Hide resolved
Co-authored-by: Felix Exner <[email protected]>
@fmauch fmauch self-assigned this Mar 24, 2022
@fmauch fmauch requested a review from t-schnell April 5, 2022 09:31
Copy link
Collaborator

@t-schnell t-schnell left a comment

Choose a reason for hiding this comment

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

Looks good overall, and I don't see any problems with merging this as I don't think there are any other foreseeable plans to utilize the queued packages in a more useful way.

}

// If the queue is empty, wait for a package.
return res || queue_.waitDequeTimed(product, timeout);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally not a fan of this type of return structure, but I can accept that as subjective.

@fmauch fmauch merged commit 6ea3d3a into UniversalRobots:master Apr 7, 2022
@fmauch
Copy link
Collaborator

fmauch commented Apr 7, 2022

Thanks again @corycrean

@fmauch fmauch mentioned this pull request Apr 12, 2022
urmahp pushed a commit to urmahp/Universal_Robots_Client_Library that referenced this pull request Sep 5, 2022
Get the latest package each time the consumer reads from the queue.
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.

3 participants