Skip to content

Make sick safetyscanners2 compatible with composition#59

Closed
JinyongJeong-Riibotics wants to merge 11 commits intoSICKAG:masterfrom
Riibotics:master
Closed

Make sick safetyscanners2 compatible with composition#59
JinyongJeong-Riibotics wants to merge 11 commits intoSICKAG:masterfrom
Riibotics:master

Conversation

@JinyongJeong-Riibotics
Copy link

Purpose of this PR

The current driver does not support zero-copy intra-process communication (IPC) when used as a composed component.
This pull request introduces the required modifications to enable composition-friendly, zero-copy operation.

Key Changes

  1. Constructor Signature
    The primary constructor now accepts rclcpp::NodeOptions, ensuring the driver can be instantiated as a composable node.

  2. Component Registration
    The node is registered with rclcpp_components_register_node, allowing it to be loaded dynamically into a component container.

  3. Subscriber Count Check
    Before each publish, the driver verifies that at least one subscriber is connected, eliminating unnecessary work when no consumers are present.

  4. Zero-Copy Publishing
    Message data are created and published as std::unique_ptr objects.
    In environments where IPC is enabled, this enables true zero-copy transfer; when IPC is disabled, behavior remains identical to standard publishing.

  5. Reduced Copy Overhead
    Internal data structures are generated directly into unique_ptr instances, minimizing intermediate copies and improving performance.

Maintainer Guidance

These changes, particularly the revised constructor, introduce breaking API differences.
If the new interface or zero-copy assumptions conflict with the project’s roadmap, please let me know. I will close this PR rather than disrupt the main branch.

@chriseichmann
Copy link
Collaborator

Thank you for the contribution! I will try to have a more detailed look into this in the coming days. In general, I like the idea to support composition, but need to be careful to not disrupt existing user applications.

@JinyongJeong-Riibotics
Copy link
Author

@chriseichmann — thank you for the quick response.

I share your view that we must avoid disrupting existing projects. Unfortunately, this PR introduces many changes to enable composition and could break current applications. For that reason, the final decision on whether to merge rests with the repository’s maintenance team.

If the team feels the risk to existing projects is too high, please let me know and I will close this PR.

@xaru8145
Copy link

This is a very nice feature and we are really interested, thanks for the work 🙌 Is there anything I can do from my side to help this move forward?

@chriseichmann
Copy link
Collaborator

I will push the improve/composition branch from the Riibotics repo to this repo to amend some changes:

  • provide both new and old constructor signatures to ensure backward compatibility
  • add documentation on component
  • add example launch file

@chriseichmann chriseichmann mentioned this pull request Nov 11, 2025
3 tasks
@chriseichmann
Copy link
Collaborator

Closing this in favor of #65

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.

4 participants