provide option to not create logging publisher

[wjwwood]

Feature request

Feature description

Currently the node unconditionally creates a publisher for logging:

rcl/rcl/src/rcl/node.c

Line 358 in d655147

ret = rcl_logging_rosout_init_publisher_for_node(node);

This is potentially undesirable in systems which do not need this and have an rmw layer that aggressively preallocates storage for publishers and subscriptions.

There should be some way to control whether or not this occurs when creating the node, or possibly globally.

Implementation considerations

We should look for places where we assume this publisher exists for each node and ensure it falls back gracefully when it is disabled.

[fujitatomoya]

fundamentally it looks like following flag is true,

rcl/rcl/src/rcl/logging.c

Line 43 in d655147

static bool g_rcl_logging_rosout_enabled = false;

but we do not want to make publisher instance, am i understanding correct?

[fujitatomoya]

@wjwwood

could you take a look at #514 ? when you got time.

[fujitatomoya]

@wjwwood @ivanpauno

i believe that we can close this thread.

[ivanpauno]

I think that we should keep this open.
IMO, having the ability of enabling/disabling rosout logging in each node individually is desired.

Now, we can only globally disable it.

[wjwwood]

IMO, having the ability of enabling/disabling rosout logging in each node individually is desired.

Definitely, and so I would be fine with leaving this open and/or closing it but opening a new issue.

[fujitatomoya]

@ivanpauno @wjwwood

okay, got it.

what about,

• add rosout enable/disable flag in NodeOption as each node setting.
• and we can also keep the global switch g_rcl_logging_rosout_enabled.

or you already have proposal?

[ivanpauno]

@ivanpauno @wjwwood

okay, got it.

what about,

• add rosout enable/disable flag in NodeOption as each node setting.
• and we can also keep the global switch g_rcl_logging_rosout_enabled.

or you already have proposal?

Sounds like a good plan!

[Barry-Xu-2018]

@ivanpauno @fujitatomoya @wjwwood

what about,
add rosout enable/disable flag in NodeOption as each node setting.
and we can also keep the global switch g_rcl_logging_rosout_enabled.

I implemented them.
Could you review my patches ?
#532 and ros2/rclcpp#900

[fujitatomoya]

@Barry-Xu-2018

requesting fix for #532 (review)

[Barry-Xu-2018]

@fujitatomoya

requesting fix for #532 (review)

Thanks for your review.
But I have a question on your comments.
Could you check it #532 (comment)

[fujitatomoya]

@wjwwood @ivanpauno

no pressure, but could you take a look at it when you got time?

thanks

[fujitatomoya]

@wjwwood @ivanpauno

is there anything we could help you via this thread? if not, could you close this issue?

Best,

[ivanpauno]

is there anything we could help you via this thread? if not, could you close this issue?

Thanks for the reminder. I think that everything has been done.
Thanks for the contributions @fujitatomoya @Barry-Xu-2018 !