Skip to content

Convert QoS APIs to pybind11#736

Merged
hidmic merged 2 commits intomasterfrom
hidmic/pybind-qos-api
Mar 26, 2021
Merged

Convert QoS APIs to pybind11#736
hidmic merged 2 commits intomasterfrom
hidmic/pybind-qos-api

Conversation

@hidmic
Copy link
Copy Markdown
Contributor

@hidmic hidmic commented Mar 24, 2021

Part of #665. It's a shallow port (i.e. didn't migrate relevant common.c functions) in hopes we can eventually bind the entire qos.py module.

CI up to rclpy:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status
  • Windows Debug Build Status

@hidmic hidmic requested review from gbalke and sloretz March 24, 2021 20:58
@hidmic hidmic mentioned this pull request Mar 24, 2021
34 tasks
Copy link
Copy Markdown
Contributor

@gbalke gbalke left a comment

Choose a reason for hiding this comment

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

Looking good!

static_cast<rmw_qos_profile_t *>(
PyMem_Malloc(sizeof(rmw_qos_profile_t))), PyMem_Free);
if (!qos_profile) {
throw std::bad_alloc();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this isn't documented in the header.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. See e0d3644.


// Fill a given rmw_time_t from an rclpy.duration.Duration instance.
void
_convert_py_duration_to_rmw_time(py::capsule pyduration, rmw_time_t * out_time)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This not being a bool definitely made the code below a lot simpler!

Copy link
Copy Markdown
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

Just a suggestion to check the pycapsule names.

@hidmic
Copy link
Copy Markdown
Contributor Author

hidmic commented Mar 25, 2021

CI up to rclpy:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

hidmic added 2 commits March 25, 2021 18:58
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic force-pushed the hidmic/pybind-qos-api branch from e0d3644 to 1b922a5 Compare March 25, 2021 22:41
@hidmic
Copy link
Copy Markdown
Contributor Author

hidmic commented Mar 25, 2021

CI up to rclpy:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@hidmic
Copy link
Copy Markdown
Contributor Author

hidmic commented Mar 26, 2021

Alright, going in !

@hidmic hidmic merged commit d150942 into master Mar 26, 2021
@delete-merged-branch delete-merged-branch bot deleted the hidmic/pybind-qos-api branch March 26, 2021 12:56
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