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

Loaned functions returning rcl_ret_t use rmw_ret_t #735

Closed
Blast545 opened this issue Aug 7, 2020 · 1 comment · Fixed by #738
Closed

Loaned functions returning rcl_ret_t use rmw_ret_t #735

Blast545 opened this issue Aug 7, 2020 · 1 comment · Fixed by #738
Assignees
Labels
enhancement New feature or request

Comments

@Blast545
Copy link
Contributor

Blast545 commented Aug 7, 2020

Loaned functions as rcl_borrow_loaned_message:

rcl_ret_t
rcl_borrow_loaned_message(
  const rcl_publisher_t * publisher,
  const rosidl_message_type_support_t * type_support,
  void ** ros_message)
{
  if (!rcl_publisher_is_valid(publisher)) {
    return RCL_RET_PUBLISHER_INVALID;  // error already set
  }
  return rmw_borrow_loaned_message(publisher->impl->rmw_handle, type_support, ros_message);
}

Make a call to a rmw function and the result is returned as a rcl_ret_t. I think for those it's needed an additional rcl_convert_rmw_ret_to_rcl_ret call. Let me know your thoughts and I'll open a PR accordingly.

@Blast545 Blast545 added the enhancement New feature or request label Aug 7, 2020
@Blast545 Blast545 self-assigned this Aug 7, 2020
Blast545 added a commit that referenced this issue Aug 7, 2020
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@clalancette
Copy link
Contributor

I think for those it's needed an additional rcl_convert_rmw_ret_to_rcl_ret call. Let me know your thoughts and I'll open a PR accordingly.

Yep, looks like it. Seems reasonable to me to open a PR to fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants