Skip to content

MessageFilter Generalizated Nodes#661

Closed
CursedRock17 wants to merge 4 commits intoros2:rollingfrom
CursedRock17:message_filter_node
Closed

MessageFilter Generalizated Nodes#661
CursedRock17 wants to merge 4 commits intoros2:rollingfrom
CursedRock17:message_filter_node

Conversation

@CursedRock17
Copy link
Copy Markdown
Contributor

This PR should entirely resolve #95 as pull request #96 didn't use a templated Node in the MessageFilter constructors. There were concerns of:

Retain[ing] the existing node-based interface for backwards compatibility.

which we can do with defaulted templated Nodes and node_interface methods.

Signed-off-by: CursedRock17 <mtglucas1@gmail.com>
Signed-off-by: CursedRock17 <mtglucas1@gmail.com>
@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Mar 15, 2024

@clalancette what do you think about this?

which we can do with defaulted templated Nodes and node_interface methods.

I can see both strategies in some other packages, any recomendation ?

Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conflicts

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented Aug 29, 2024

@CursedRock17 friendly ping to rebase the PR 🧇

@sloretz sloretz added the more-information-needed Further information is required label Aug 29, 2024
@CursedRock17
Copy link
Copy Markdown
Contributor Author

@sloretz after I saw issue #698, I began writing a fix which would essentially make this PR obsolete. Is it even still worth cleaning up and merging this in, if it's going to be deprecated in the future, I would feel it to be a dead feature.

@clalancette
Copy link
Copy Markdown
Contributor

@sloretz after I saw issue #698, I began writing a fix which would essentially make this PR obsolete. Is it even still worth cleaning up and merging this in, if it's going to be deprecated in the future, I would feel it to be a dead feature.

Agreed with your reasoning. In that case, I'm going to close this out. But feel free to reopen if you think we should pursue this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

more-information-needed Further information is required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lifecycle node support for tf2_ros::MessageFilter

4 participants