-
Notifications
You must be signed in to change notification settings - Fork 86
#130 add simple filter tutorial for cpp #239
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
#130 add simple filter tutorial for cpp #239
Conversation
fujitatomoya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm with minor comments.
@CursedRock17 can you take a look at this?
doc/Tutorials/Chain-Python.rst
Outdated
| This filter class will be a successor to the ``SimpleFilter`` class, but this is a topic for another tutorial. | ||
|
|
||
| .. TODO: @EsipovPA: Add message_filters.SimpleFilter tutorial reference, when ready | ||
| For more information on this topic, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/jazzy/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we point it to the rolling branch? (the same comment goes else where.)
| For more information on this topic, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/jazzy/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. | |
| For more information on this topic, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/rolling/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I would agree with changing this rolling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for noticing this. Also, it might be an issue when backporting this to other releases. This should be valid for every release, so it should be changed by hand before merging MRs created by the bot if will be used.
upd
Fixed here in this PR
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| 1. Create a Basic Node | ||
| ~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Let's assume, you've already created an empty ROS 2 package for C++. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to keep the consistency, probably we would want to quote it? or maybe unquote all of them?
| Let's assume, you've already created an empty ROS 2 package for C++. | |
| Let's assume, you've already created an empty ROS 2 package for ``C++``. |
doc/Tutorials/Chain-Python.rst
Outdated
|
|
||
| .. More on this succession mechanism should be in the corresponding tutorial | ||
| .. TODO: @EsipovPA Add link to the message_filters.SimpleFilter tutorial, when added. | ||
| For more information on this succession mechanism, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/jazzy/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| For more information on this succession mechanism, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/jazzy/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. | |
| For more information on this succession mechanism, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/rolling/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| SimpleFilter (C++): | ||
| ------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SimpleFilter (C++): | |
| ------------ | |
| SimpleFilter (C++): | |
| ------------------- |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| ament_auto_add_executable(simple_filter_tutorial src/simple_filter_tutorial.cpp) | ||
| Finally, add the install(TARGETS…) section so ros2 run can find your executable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Finally, add the install(TARGETS…) section so ros2 run can find your executable: | |
| Finally, add the ``install(TARGETS…)`` section so ros2 run can find your executable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed here and in another tutorials as well.
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| .. code-block:: console | ||
| [INFO] [1766958211.195602859] [SimpleFilterExampleNode]: No messages published yet | ||
| [INFO] [1766958212.195809044] [SimpleFilterExampleNode]: Published messages count: 1. Last message: Pub count: 1 | ||
| [INFO] [1766958213.195618995] [SimpleFilterExampleNode]: Published messages count: 2. Last message: Pub count: 2 | ||
| [INFO] [1766958214.195599466] [SimpleFilterExampleNode]: Published messages count: 3. Last message: Pub count: 3 | ||
| [INFO] [1766958215.195890964] [SimpleFilterExampleNode]: Published messages count: 4. Last message: Pub count: 4 | ||
| [INFO] [1766958216.195910443] [SimpleFilterExampleNode]: Published messages count: 5. Last message: Pub count: 5 | ||
| [INFO] [1766958217.195906785] [SimpleFilterExampleNode]: Published messages count: 6. Last message: Pub count: 6 | ||
| [INFO] [1766958218.195652168] [SimpleFilterExampleNode]: Published messages count: 7. Last message: Pub count: 7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .. code-block:: console | |
| [INFO] [1766958211.195602859] [SimpleFilterExampleNode]: No messages published yet | |
| [INFO] [1766958212.195809044] [SimpleFilterExampleNode]: Published messages count: 1. Last message: Pub count: 1 | |
| [INFO] [1766958213.195618995] [SimpleFilterExampleNode]: Published messages count: 2. Last message: Pub count: 2 | |
| [INFO] [1766958214.195599466] [SimpleFilterExampleNode]: Published messages count: 3. Last message: Pub count: 3 | |
| [INFO] [1766958215.195890964] [SimpleFilterExampleNode]: Published messages count: 4. Last message: Pub count: 4 | |
| [INFO] [1766958216.195910443] [SimpleFilterExampleNode]: Published messages count: 5. Last message: Pub count: 5 | |
| [INFO] [1766958217.195906785] [SimpleFilterExampleNode]: Published messages count: 6. Last message: Pub count: 6 | |
| [INFO] [1766958218.195652168] [SimpleFilterExampleNode]: Published messages count: 7. Last message: Pub count: 7 | |
| .. code-block:: console | |
| [INFO] [1766958211.195602859] [SimpleFilterExampleNode]: No messages published yet | |
| [INFO] [1766958212.195809044] [SimpleFilterExampleNode]: Published messages count: 1. Last message: Pub count: 1 | |
| [INFO] [1766958213.195618995] [SimpleFilterExampleNode]: Published messages count: 2. Last message: Pub count: 2 | |
| [INFO] [1766958214.195599466] [SimpleFilterExampleNode]: Published messages count: 3. Last message: Pub count: 3 | |
| [INFO] [1766958215.195890964] [SimpleFilterExampleNode]: Published messages count: 4. Last message: Pub count: 4 | |
| [INFO] [1766958216.195910443] [SimpleFilterExampleNode]: Published messages count: 5. Last message: Pub count: 5 | |
| [INFO] [1766958217.195906785] [SimpleFilterExampleNode]: Published messages count: 6. Last message: Pub count: 6 | |
| [INFO] [1766958218.195652168] [SimpleFilterExampleNode]: Published messages count: 7. Last message: Pub count: 7 |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| typedef MessageEvent<M const> EventType; | ||
|
|
||
| We start with a few ``typedef declarations`` for ``MConstPtr``, ``Callback`` and ``EventType``. | ||
| These will make the following code clearer and easyer to read. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| These will make the following code clearer and easyer to read. | |
| These will make the following code clearer and easier to read. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| ); | ||
| } | ||
|
|
||
| In order to create a connection with another filter, this method registeres the ``add`` method of this class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| In order to create a connection with another filter, this method registeres the ``add`` method of this class | |
| In order to create a connection with another filter, this method registers the ``add`` method of this class |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
|
|
||
| The private section in this case is rather simple. | ||
| It consists of the ``counter_`` and the ``last_message_cache_`` fields and the ``incoming_connection_`` field. | ||
| First two are the part of the buisness logic of this class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| First two are the part of the buisness logic of this class. | |
| First two are the part of the business logic of this class. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| [INFO] [1766958217.195906785] [SimpleFilterExampleNode]: Published messages count: 6. Last message: Pub count: 6 | ||
| [INFO] [1766958218.195652168] [SimpleFilterExampleNode]: Published messages count: 7. Last message: Pub count: 7 | ||
|
|
||
| Note that when the first query is executed, there were no messages that have pasesd the filter, as is indicated by the console output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Note that when the first query is executed, there were no messages that have pasesd the filter, as is indicated by the console output. | |
| Note that when the first query is executed, there were no messages that have passed the filter, as is indicated by the console output. |
|
Thank you all for the review. I'll definetely fix the code when I'm back home. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the most part LGTM, I just wanted to go through and build the node just to make sure it was followable. Found a slight error in a typo with the main CounterWilthLastMessageCache which has wilth instead of with. Other than that just nitpicking for readability.
Great work!
doc/Tutorials/Chain-Python.rst
Outdated
| This filter class will be a successor to the ``SimpleFilter`` class, but this is a topic for another tutorial. | ||
|
|
||
| .. TODO: @EsipovPA: Add message_filters.SimpleFilter tutorial reference, when ready | ||
| For more information on this topic, please refer to the `SimpleFilter for Python tutorial <https://docs.ros.org/en/jazzy/p/message_filters/doc/Tutorials/SimpleFilter-Python.html>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I would agree with changing this rolling
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| namespace message_filters | ||
| { | ||
| template<class M> | ||
| class CounterWilthLastMessageCache : public SimpleFilter<M> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo on Class
| class CounterWilthLastMessageCache : public SimpleFilter<M> { | |
| class CounterWithLastMessageCache : public SimpleFilter<M> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omg 🥲
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| virtual ~CounterWilthLastMessageCache() {} | ||
|
|
||
| CounterWilthLastMessageCache() {} | ||
|
|
||
| template<typename F> | ||
| explicit CounterWilthLastMessageCache(F & filter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo continued.
| virtual ~CounterWilthLastMessageCache() {} | |
| CounterWilthLastMessageCache() {} | |
| template<typename F> | |
| explicit CounterWilthLastMessageCache(F & filter) | |
| virtual ~CounterWithLastMessageCache() {} | |
| CounterWithLastMessageCache() {} | |
| template<typename F> | |
| explicit CounterWithLastMessageCache(F & filter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a typo in a first class declaration in the actual project where I've tested the code, and after that is was autocompletion in vscode. Should always remind myself not to trust autocompletion completely (this one is intended) 🤦♂️
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| incoming_connection_ = filter.registerCallback( | ||
| typename SimpleFilter<M>::EventCallback( | ||
| std::bind( | ||
| &CounterWilthLastMessageCache::add, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| &CounterWilthLastMessageCache::add, | |
| &CounterWithLastMessageCache::add, |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| This tutorial demonstrates how to create a custom filter that is going to be a successor to the ``SimpleFilter`` class. | ||
| The ``SimpleFilter`` is a base class for almost all of message filters implemented in ``C++``. | ||
| It provides the basic functionality for building filters. | ||
| To demonstrate the functionality of this filter we are going to create a ``CounterWilthLastMessageCache`` filter class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small Typo.
| To demonstrate the functionality of this filter we are going to create a ``CounterWilthLastMessageCache`` filter class. | |
| To demonstrate the functionality of this filter we are going to create a ``CounterWithLastMessageCache`` filter class. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| The private section in this case is rather simple. | ||
| It consists of the ``counter_`` and the ``last_message_cache_`` fields and the ``incoming_connection_`` field. | ||
| First two are the part of the buisness logic of this class. | ||
| The last one is required for managing the connection with a filter that does pass messages to this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The last one is required for managing the connection with a filter that does pass messages to this one. | |
| The last one manages the connection with a filter that passes messages to this one. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| The instance of the ``SubscriberFilter``is used to receive messages from the ``TUTORIAL_TOPIC``. | ||
| The instance of the ``CounterWilthLastMessageCache`` filter is immediately connected to the ``subscriber_filter_``'s output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline code block breaking up and typo change.
| The instance of the ``SubscriberFilter``is used to receive messages from the ``TUTORIAL_TOPIC``. | |
| The instance of the ``CounterWilthLastMessageCache`` filter is immediately connected to the ``subscriber_filter_``'s output. | |
| The instance of the ``SubscriberFilter`` receives messages from the ``TUTORIAL_TOPIC``. | |
| The instance of the ``CounterWithLastMessageCache`` filter is immediately connected to the ``subscriber_filter_``'s output. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| And the two timers are added to automate the work. | ||
| The ``publisher_timer_`` to automate message publishing. | ||
| And the ``query_timer_`` to automate the introspection of the node filters state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking up the section
| And the two timers are added to automate the work. | |
| The ``publisher_timer_`` to automate message publishing. | |
| And the ``query_timer_`` to automate the introspection of the node filters state. | |
| Two timers are added to automate the work: | |
| The ``publisher_timer_`` automates message publishing. | |
| The ``query_timer_`` automates the introspection of the node filter's state. |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
| incoming_connection_ = filter.registerCallback( | ||
| typename SimpleFilter<M>::EventCallback( | ||
| std::bind( | ||
| &CounterWilthLastMessageCache::add, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo continued.
| &CounterWilthLastMessageCache::add, | |
| &CounterWithLastMessageCache::add, |
doc/Tutorials/SimpleFilter-Cpp.rst
Outdated
|
|
||
| private: | ||
| message_filters::Subscriber<std_msgs::msg::String> subscriber_filter_; | ||
| message_filters::CounterWilthLastMessageCache<std_msgs::msg::String> counter_filter_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo Continued.
| message_filters::CounterWilthLastMessageCache<std_msgs::msg::String> counter_filter_; | |
| message_filters::CounterWithLastMessageCache<std_msgs::msg::String> counter_filter_; |
…ference. Fix minor typos in Chain and Cache tutorials Signed-off-by: EsipovPA <esipov.p@mail.ru>
Signed-off-by: EsipovPA <esipov.p@mail.ru>
4c6021e to
6459e47
Compare
|
Pulls: #239 |
|
Hello @ahcorde! Thanks for the review and for the approval. It seems like the 'Linux' job is not running. It was in a 'failing' state for some time, but now it is just in 'not run'. Is there anything to be done about it? Maybe there is something, I can do? Sorry for bothering, if I'm being inconvenient. |
|
https://github.com/Mergifyio backport kilted jazzy humble |
✅ Backports have been createdDetails
|
* #130 add simple filter tutorial for cpp (#239) Signed-off-by: EsipovPA <esipov.p@mail.ru> (cherry picked from commit 92af2b7) Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com> Co-authored-by: Pavel Esipov <38457822+EsipovPA@users.noreply.github.com> Co-authored-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
* #130 add simple filter tutorial for cpp (#239) Signed-off-by: EsipovPA <esipov.p@mail.ru> (cherry picked from commit 92af2b7) Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com> Co-authored-by: Pavel Esipov <38457822+EsipovPA@users.noreply.github.com> Co-authored-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
* #130 add simple filter tutorial for cpp (#239) Signed-off-by: EsipovPA <esipov.p@mail.ru> (cherry picked from commit 92af2b7) Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com> Co-authored-by: Pavel Esipov <38457822+EsipovPA@users.noreply.github.com> Co-authored-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Description
Add a tutorial for using the
SimpleFilterclass to create new filters.Adresses #130
Is this user-facing behavior change?
Yes. It is a UX improvement.
Did you use Generative AI?
No, I did not.
Additional Information