-
Notifications
You must be signed in to change notification settings - Fork 57
[#429] add response channel #680
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
base: main
Are you sure you want to change the base?
[#429] add response channel #680
Conversation
4ede017
to
4b0f3b9
Compare
…the request id in request
… this is unnecessary
… already out of scope
baef1a0
to
b1f003e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new response channel mechanism and augments the request/response API to better support both zero-copy and copy-based responses. Key changes include:
- Adding new configuration fields in the config module to control client buffer size and fire-and-forget request behavior.
- Extending the ActiveRequest type with new fields (e.g. shared_state, request_id, channel_id, connection_id) and methods (e.g. loan_uninit, send_copy, loan) to manage response channels.
- Removing obsolete FFI API functions and updating related tests and zero-copy connection components to support enhanced channel state management.
Reviewed Changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
iceoryx2/src/config.rs | Added new configuration fields supporting response channel behavior. |
iceoryx2/src/active_request.rs | Extended ActiveRequest with new fields and response loan/send APIs. |
iceoryx2-ffi/ffi/src/api/subscriber.rs & subscriber.hpp | Removed the update_connections API from the FFI layer. |
iceoryx2-cal/* | Updated tests and internal components to support channel state handling. |
iceoryx2-bb/* & iceoryx2-cal/* | Adjustments in dynamic storage and container implementations. |
examples/rust/request_response/* | Updated examples to demonstrate usage of both response APIs. |
Files not reviewed (1)
- iceoryx2-cal/BUILD.bazel: Language not supported
Comments suppressed due to low confidence (4)
iceoryx2/src/config.rs:358
- [nitpick] The flag 'allow_fire_and_forget_requests' might be ambiguous; consider renaming it to 'enable_fire_and_forget_requests' to clearly indicate that this flag enables the behavior.
pub allow_fire_and_forget_requests: bool,
iceoryx2-ffi/ffi/src/api/subscriber.rs:390
- [nitpick] The removal of the update_connections function from the FFI API should be accompanied by updated documentation and tests to ensure that consumers of the API are adequately informed of this change.
/* removed update_connections function */
iceoryx2/src/active_request.rs:158
- Replacing fatal_panic! with error! changes critical error handling semantics. Verify that logging the error without panicking is the intended behavior when the client's retrieval channel is full.
error!(from self, "This should never happen! The clients retrieve channel is full and the request cannot be returned.");
iceoryx2-bb/container/src/vec.rs:302
- Ensure that the memory move in remove_impl, which uses core::ptr::copy, correctly handles overlapping memory regions. Although core::ptr::copy allows for overlap, verify that the logic is safe in all scenarios.
unsafe fn remove_impl(&mut self, index: usize) -> T {
Notes for Reviewer
Pre-Review Checklist for the PR Author
Convert to draft
)SPDX-License-Identifier: Apache-2.0 OR MIT
iox2-123-introduce-posix-ipc-example
)[#123] Add posix ipc example
)task-list-completed
)Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Relates to #429