Use shared transport SHM provider instead of own instance of SHM provider #857
Conversation
evshary
left a comment
evshary
left a comment
There was a problem hiding this comment.
Some coding styles is changed, which is not related to the PR. I think it would be better to keep the same style.
| enabled: true, | ||
| /// SHM memory size in bytes used for transport optimization (default `16 * 1024 * 1024`). | ||
| pool_size: 16777216, | ||
| pool_size: 50331648, |
There was a problem hiding this comment.
It would be better if we could add ROS setting: in the comment like this:
zenoh_cpp_vendor/CMakeLists.txt
Outdated
| ament_vendor(zenoh_c_vendor | ||
| VCS_URL https://github.com/eclipse-zenoh/zenoh-c.git | ||
| VCS_VERSION f376456ccf75ed837a21a186bdf5191cba50eb3b | ||
| VCS_URL https://github.com/ZettaScaleLabs/zenoh-c.git |
There was a problem hiding this comment.
I think you will switch back to eclipse-zenoh after the corresponding PR is merged, right?
| #define SAMPLE_MISS_DETECTION_HEARTBEAT_PERIOD 500 | ||
|
|
||
| ///============================================================================= | ||
| ///============================================================================= |
There was a problem hiding this comment.
Just out of curiosity, do you leave the indent here intentionally?
There was a problem hiding this comment.
I think this is auto-formatter artifact.
There was a problem hiding this comment.
I think it would be better to keep the same coding style. I see many style changes in rmw_publisher_data.cpp are not related to the PR.
…SSION_CONFIG.json5
JEnoch
left a comment
JEnoch
left a comment
There was a problem hiding this comment.
I think that for backward compatibility we need to keep support of both ZENOH_SHM_ALLOC_SIZE and ZENOH_SHM_MESSAGE_SIZE_THRESHOLD environment variable. They should override the config values transport_optimization/pool_size and transport_optimization/message_size_threshold respectively.
We could add a deprecation warning log when set, explaining those variables will no longer be supported in Lyrical Luth distro and that the transport_optimization config shall be used instead.
Then before Lyrical code branch (03/2026) we remove those environment variables from the rolling branch only.
The README.md has to be updated also with regard to this change.
3 participants
No description provided.