-
Notifications
You must be signed in to change notification settings - Fork 145
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
Added a simple ros2 rust logger example #459
base: main
Are you sure you want to change the base?
Conversation
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.
@Guelakais thanks for your contribution. Could you revert the file mode changes? All the files are marked as executable in your PR and it makes it more difficult to review the actual changes when there are more than 100 files changed
151c8c7
to
d8a4ccf
Compare
I think it's better now. I have split the whole thing into more commits and left out the modechanges. |
975f6fa
to
d8a4ccf
Compare
Then I'm a bit confused about the errors. For me, the example was built without any problems. |
@Guelakais what errors? The CI passes: |
Seems to be gone. Maybe it was just a coincidence. |
If I may add a comment, I think that this new example is a lot of duplication of the minimal_publisher example. |
I understand your perspective. However, I disagree with the implication that we must choose between the two approaches. While it's technically feasible to augment all examples with logging, the core issue is not time, but runtime efficiency. Specifically, ROS 2 logging relies on publishing messages to the |
This point does not matter, these packages are examples on how to use the logging API, not fully fledged production-ready applications. In going to pick one of the two PRs, whichever seems more complete. #460 seems to exercise more parts of the logging API, which is the end goal of these examples In any case, thanks @Guelakais for your contribution |
Well... I didn't create my PR to replace the one from @Guelakais . It is more "Ok we have a small and very simple example to start with. Now let's show some other features". The issue #431 talks about creating logging "examples". Now we have 2 examples 👍 |
In my point of view, examples in an available the documentation is less maintenance that a full example for a simple use case. |
On the other hand, there are simply programmers who like to run the working examples on their computer first to get a feel for what the tool can do. In addition, with the rust compiler toolchain, the examples are always compiled at the same time and tested during compilation, which makes maintenance very easy. The Rust compiler usually provides very helpful explanations as to why something is not working and possible solutions. |
Mentioning #431 I've made a simple ros2 rust node wich includes a simple publisher and logging. It's based on the minimal publisher.
added simple example for ros2 rust node with logging
added the manifest file for the example
added the ros2 manifest file
modechange