Fix delayed subscription missing first message and change topic name #52
Conversation
…orrect subscriber on topic_name change
behaviortree_ros2/include/behaviortree_ros2/bt_topic_sub_node.hpp
Outdated
Show resolved
Hide resolved
| * | ||
| * @return true will clear the message after ticking/processing. | ||
| */ | ||
| virtual bool clearProcessedMessage() { return true; } |
There was a problem hiding this comment.
I don't understand why by default this should return true. Can you elaborate? When should a user override this?
There was a problem hiding this comment.
That retains current behaviour of the node. Anything else and current default implementation behaviour will change
behaviortree_ros2/include/behaviortree_ros2/bt_topic_sub_node.hpp
Outdated
Show resolved
Hide resolved
|
Thanks, but I would not customize the retention policy through inheritance. Also, it is not very clear what the intent is. What about adding a parameter in the constructor, such as |
style: update from review Co-authored-by: Davide Faconti <davide.faconti@gmail.com>
It is more flexible (some more discussion in #42). For example a colleague wanted to clear it based on a port value, this can easily be added in the overridden definition, although for those particular use cases I think it was better to just define 2 actions Maybe there are other uses cases basing it on the content of the message itself ... but haven't hit that myself yet Definitely open to better naming |
Fixes issue #51