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

The RxROS logging framework should be redesigned #17

Open
henrik7264 opened this issue Aug 31, 2019 · 1 comment
Open

The RxROS logging framework should be redesigned #17

henrik7264 opened this issue Aug 31, 2019 · 1 comment

Comments

@henrik7264
Copy link
Collaborator

henrik7264 commented Aug 31, 2019

It should be possible to write something similar to the following code:

rxros::Logging().info() << "Hello world  ";
rxros::observable::from_device<input_event>(keyboardDevice)
        | map(keyboardEvent2KeyboardMsg)
        | log_info(...)
        | publish_to_topic<teleop_msgs::Keyboard>("/keyboard");

It is important to provide a consistent and simple logging framework. It should be equally easy to log in a sequence of operator as well as outside. A proposed framework should be presented before implementation.

@henrik7264 henrik7264 changed the title The RxROS logging framework should be reworked The RxROS logging framework should be redesigned Aug 31, 2019
@wasowski
Copy link
Member

This is a fragment how I did logging in the rxros_talker (still not merged):

rxcpp::observable<>::interval (std::chrono::milliseconds (1000))
  | map ([&](int i)٠                                            
      { return mk_msg(hello + std::to_string(i)); })            
  | tap ([](const std_msgs::String& msg)٠                       
      { ROS_INFO ("%s", msg.data.c_str()); })                   
  | publish_to_topic<std_msgs::String> ("/chatter", 1000);      

As you can see tap does almost what you want. It is probably very hard to do more -- you want to have access to the current message when formatting the string. So the only thing we can save is to remove ROS_INFO call, perhaps a simple wrapper around tap for rxros_info and the associated siblings of ROS_INFO could be added, to save this one call.

But it is probably not a very high priority. I would not argue against it though. Seems like a nice-to-have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants