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

logging_callback in pendulum_log.cpp fails to output to fstream #450

Closed
y-okumura-isp opened this issue Jun 16, 2020 · 2 comments
Closed
Labels
help wanted Extra attention is needed

Comments

@y-okumura-isp
Copy link
Contributor

Bug report

I'm working on ros2/realtime_support#94.
We want to change some rttest APIs such as timespec_to_long to timspec_to_uint64.
As these functions are used in pendulum_control, now I'm changing and testing pendulum_control.
In this work, I found the result of timespec_to_long should be printed but actually not.

Required Info:

  • Operating System:
    • Ubuntu 18.04
  • Installation type:
    • trunk
  • Version or commit hash:
  • DDS implementation:
    • FastRTPS
  • Client library (if applicable):
    • NA

Steps to reproduce issue

./build/pendulum_control/pendulum_logger > stdout.csv &
./build/pendulum_control/pendulum_demo -i 10000

See pendulum_logger_results.csv or stdout or stderr, but I couldn't see output of following line.
https://github.com/ros2/demos/blob/master/pendulum_control/src/pendulum_logger.cpp#L62

The code tries to output to fstream (may be pendulum_logger_result.csv),
but pendulum_logger_result.csv is following, only one line.

iteration timestamp latency minor_pagefaults minor_pagefaults

Expected behavior

We get log in pendulum_logger_results.csv.

Actual behavior

No log in pendulum_logger_results.csv.

Additional information

I think the problem comes from the capture of logging_callback lambda.
The variable fstream is opened outside lambda but not captured.
And fstream is defined in logging_callback lambda without file specification.
According to http://www.cplusplus.com/reference/fstream/ofstream/ofstream/, fstream may be the internal file stream buffer and not associated with file.

  // pendulum_logger.cpp L43-67
  fstream.open(filename, std::ios_base::app);
  size_t i = 0;
  auto logging_callback =
    [&i](const pendulum_msgs::msg::RttestResults::SharedPtr msg) {
      // snip
      std::ofstream fstream;
      // snip
      fstream << i << ...;
  };
  
  auto subscription = logger_node->create_subscription<pendulum_msgs::msg::RttestResults>(
    "pendulum_statistics", qos, logging_callback);

I fixed as bellow,

  auto logging_callback =
    [&i, &fstream](const pendulum_msgs::msg::RttestResults::SharedPtr msg) {
      // std::ofstream fstream;  // delete this lins
      fstream << i << ...;

and get output in pendulum_logger_results.csv.

iteration timestamp latency minor_pagefaults minor_pagefaults
0 16255610679777739 53398 0 0
1 16255610680782905 53605 0 0
2 16255610681776418 54227 0 0
3 16255610682780484 53133 0 0
4 16255610683779059 53202 0 0

If it's OK, I'll send PR.

@clalancette
Copy link
Contributor

Honestly, that looks like debugging code. And since it is hasn't been enabled in a long time, I think I would just delete that whole block (most of the information is printed by the printf above anyway).

@sloretz sloretz added the help wanted Extra attention is needed label Jun 25, 2020
y-okumura-isp added a commit to y-okumura-isp/demos that referenced this issue Dec 21, 2020
Signed-off-by: y-okumura-isp <y-okumura@isp.co.jp>
clalancette pushed a commit that referenced this issue Dec 23, 2020
Signed-off-by: y-okumura-isp <y-okumura@isp.co.jp>
@clalancette
Copy link
Contributor

Fixed by #477, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants