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

[#490] Enable loan_slice APIs in CXX bindings #492

Merged

Conversation

orecham
Copy link
Contributor

@orecham orecham commented Oct 28, 2024

Notes for Reviewer

  1. Implement iox::Slice
  2. Implement loan_slice APIs in C++ binding
  3. Expose properties payload_alignment and max_slice_len in C++ binding

Pre-Review Checklist for the PR Author

  1. Add sensible notes for the reviewer
  2. PR title is short, expressive and meaningful
  3. Relevant issues are linked in the References section
  4. Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
  5. Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  6. Commits messages are according to this guideline
  7. Tests follow the best practice for testing
  8. Changelog updated in the unreleased section including API breaking changes
  9. Assign PR to reviewer
  10. All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

Closes #490

@orecham orecham self-assigned this Oct 28, 2024
@orecham orecham changed the title WIP: [#490] Enable loan_slice_uninit in CXX bindings WIP: [#490] Enable loan_slice APIs in CXX bindings Oct 28, 2024
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch 4 times, most recently from 515c63a to a970c4d Compare October 31, 2024 15:23
@orecham orecham changed the title WIP: [#490] Enable loan_slice APIs in CXX bindings [#490] Enable loan_slice APIs in CXX bindings Oct 31, 2024
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch 2 times, most recently from a4d05bb to a450cc8 Compare October 31, 2024 15:45
iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp Outdated Show resolved Hide resolved
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch 2 times, most recently from 1da7cbd to 1468900 Compare November 2, 2024 19:28
@orecham orecham changed the title [#490] Enable loan_slice APIs in CXX bindings BASED ON !499: [#490] Enable loan_slice APIs in CXX bindings Nov 2, 2024
@orecham orecham changed the title BASED ON !499: [#490] Enable loan_slice APIs in CXX bindings BASED ON !499: [#499] Enable loan_slice APIs in CXX bindings Nov 4, 2024
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch 9 times, most recently from 7617565 to 0cf2c2b Compare November 6, 2024 10:47
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch 2 times, most recently from 594f3bb to 7d4daa7 Compare November 7, 2024 00:33
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch from 7d4daa7 to 91f3cf2 Compare November 7, 2024 00:43
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch from 91f3cf2 to 1a8a87e Compare November 7, 2024 00:51
Copy link
Contributor

@elfenpiff elfenpiff left a comment

Choose a reason for hiding this comment

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

I found another issue with the Slice I overlooked in the previous reviews. We need to introduce a ConstIterator so that the user does not change the const MutableSlice content by accident when they iterate over the slice elements.

iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
iceoryx2-ffi/cxx/include/iox/slice.hpp Outdated Show resolved Hide resolved
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch 2 times, most recently from 52a2a4b to 23b5be2 Compare November 7, 2024 10:43
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch from 23b5be2 to b353e0a Compare November 7, 2024 10:47
@orecham orecham force-pushed the iox2-490-loan-slices-from-cxx-api branch from 7ae92fd to 636e199 Compare November 7, 2024 11:34
Copy link
Contributor

@elfenpiff elfenpiff left a comment

Choose a reason for hiding this comment

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

When the Rust example is adjusted we can merge it

@orecham orecham merged commit 810fb87 into eclipse-iceoryx:main Nov 7, 2024
47 checks passed
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.

Loan slices from C/C++ API
3 participants