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

ROS_DISABLE_LOAN_MSG flag to disable LoanedMessage from rmw #945

Closed
fujitatomoya opened this issue Oct 26, 2021 · 5 comments
Closed

ROS_DISABLE_LOAN_MSG flag to disable LoanedMessage from rmw #945

fujitatomoya opened this issue Oct 26, 2021 · 5 comments
Assignees

Comments

@fujitatomoya
Copy link
Collaborator

we'd like to open discussion, any comments are welcome.

Feature request

Feature description

adding environmental flag to disable LoanedMessage in rcl even if rmw implementation supports it. this is ROS general way to do that is agnostic from rmw implementation.
once user set environmental variable ROS_DISABLE_LOAN_MSG=1 in the process space, LoanedMessage is disable gracefully. this allows user to disable LoanedMessage and fallback to normal publisher / subscription w/o any code change or rmw implementation specific configuration.

Implementation considerations

since this is ROS general switch to the middleware, rcl would be the best place to support.
rcl_publisher_can_loan_messages and rcl_subscription_can_loan_messages checks environmental variable ROS_DISABLE_LOAN_MSG and return false immediately. this can conceal implementation feature for any frontend library such as rclcpp.

(just idea) thinking about LoanedMessage as rmw feature, we could have ROS_DISABLE_FEATURE_FLAG=0xBEEF which bit fields map to the specific feature to disable. so that we can avoid having redundant flag in the future.

@aprotyas
Copy link
Member

I might be missing context, but what is the motivation in disabling loaned messages?

@fujitatomoya
Copy link
Collaborator Author

@aprotyas fallback to normal publisher / subscription w/o any code change or rmw implementation specific configuration, if any issues happen to implementation.

@fujitatomoya
Copy link
Collaborator Author

@ivanpauno @wjwwood @jacobperron what do you think about this?

@wjwwood
Copy link
Member

wjwwood commented Feb 16, 2022

@fujitatomoya can this be closed now?

@fujitatomoya
Copy link
Collaborator Author

we still need to work on docs but implementation has been merged. close this issue.

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

No branches or pull requests

3 participants