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

feat: add TF listener and lookup TF helpers in BaseControllerInterface #173

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

bpapaspyros
Copy link
Member

Description

This PR solves the issue by replicating the TF listener functions from ComponentInterface.

Review guidelines

Estimated Time of Review: 8 minutes

Checklist before merging:

  • Confirm that the relevant changelog(s) are up-to-date in case of any user-facing changes

@bpapaspyros bpapaspyros self-assigned this Dec 5, 2024
@bpapaspyros bpapaspyros marked this pull request as ready for review December 5, 2024 09:43
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

Is there even a modification compared to modulo component interface?

@bpapaspyros
Copy link
Member Author

Is there even a modification compared to modulo component interface?

no, just get_node()-> instead of an actual variable holding the node or clock

@domire8
Copy link
Member

domire8 commented Dec 5, 2024

no, just get_node()-> instead of an actual variable holding the node or clock

nice, that triggers a problem actually. If someone tried add_tf_listener() on construction, this would give a segmentation error because there is no node on construction. Can you verify that and check that the node is actually available?

@bpapaspyros
Copy link
Member Author

bpapaspyros commented Dec 5, 2024

no, just get_node()-> instead of an actual variable holding the node or clock

nice, that triggers a problem actually. If someone tried add_tf_listener() on construction, this would give a segmentation error because there is no node on construction. Can you verify that and check that the node is actually available?

yes, sure enough that gives a seg. fault

@bpapaspyros bpapaspyros requested a review from domire8 December 5, 2024 12:19
Copy link
Member

@domire8 domire8 left a comment

Choose a reason for hiding this comment

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

Looks good!

source/modulo_controllers/src/BaseControllerInterface.cpp Outdated Show resolved Hide resolved
@bpapaspyros bpapaspyros merged commit b748d8b into feat/tf-devel Dec 5, 2024
4 checks passed
@bpapaspyros bpapaspyros deleted the feat/tf-listener branch December 5, 2024 12:28
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2024
@bpapaspyros bpapaspyros linked an issue Dec 5, 2024 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add TF listener in BaseControllerInterface
2 participants