AIT-Assistive-Autonomous-Systems / ros2bag_tools

Tool extensions for ros2bag cli
122 stars 34 forks source link

export: more logging, assert messages, rgb-bgr swap, arg for save directory #32

Closed abhishek47kashyap closed 7 months ago

abhishek47kashyap commented 8 months ago

I've a bag file which has color and depth messages of type sensor_msgs/msg/Image. I'm using this repo to export them to .png files (thank you for the work).

Had a question about the color swap. Additionally, wanted to make some suggestions I think other users of this repo might find helpful. I've only really played around with export so far, maybe what's described down below may be applicable to the other verbs too.

More logging

On running the following command:

ros2 bag export --in ./some_rosbag/ --topic /camera/rgb/image_raw image

the only log is

[rosbag2_storage]: Opened database './some_rosbag/some_rosbag.db3' for READ_ONLY.

How about adding more logs that offer information as the export progresses? For example, no. of images converted and how many more to go. The bag file I'm using is quite large so it's hard to tell what's going on as I'm waiting for the export to complete.

Assert messages

During my first export attempt, I missed providing the $EXPORTER i.e. image, and got the following traceback:

ros2 bag export --in ./some_rosbag/ --topic /camera/rgb/image_raw

Traceback (most recent call last):
  File "/opt/ros/humble/bin/ros2", line 33, in <module>
    sys.exit(load_entry_point('ros2cli==0.18.7', 'console_scripts', 'ros2')())
  File "/opt/ros/humble/lib/python3.10/site-packages/ros2cli/cli.py", line 89, in main
    rc = extension.main(parser=parser, args=args)
  File "/opt/ros/humble/lib/python3.10/site-packages/ros2bag/command/bag.py", line 38, in main
    return extension.main(args=args)
  File "/path/to/ros2bag_tools_ws/build/ros2bag_tools/ros2bag_tools/verb/export.py", line 81, in main
    assert args.config
AssertionError

While I understood an assertion check failed, an assert message could describe what the failed assertion is about. On peeking inside ros2bag_tools/verb/export.py, there are 4 assert's (including the one on line 81 which is the one that failed for me) and adding messages to them can be informative.

RGB-BGR color swap

From the limitations:

".. experience a color swap in the output. Force an output encoding if this is not desired."

Is the functionality to "force an output encoding" through the ros2 bag export CLI supported? If yes, how does one go about it?

Arg for save directory

Adding functionality to allow specifying a destination filepath might be a good feature to have. For example:

ros2 bag export --in ./some_rosbag/ --topic /camera/rgb/image_raw image --save-dir /path/to/save/directory
devrite commented 8 months ago

HI @abhishek47kashyap ,

thanks for using ros2bag_tools and reporting back to us.

Regarding the CLI-args error:

I would have to check why argparse does not detect the positional argument is not being provided. It could be the way we integrate via verbs and subparsing to rosbag2 or a limit of argparse. Either way a PR is welcome.

Regarding output-dir:

The specific args are handled by the respective implementations, like image:

https://github.com/AIT-Assistive-Autonomous-Systems/ros2bag_tools/blob/585d863596a95ef248100a0b5fc91a62f22ef96d/ros2bag_tools/ros2bag_tools/exporter/image.py#L285C1-L298C95

Does it solve your problem?

Regarding color swap:

For raw message we try to reuse cv_bridge color conversion to get it to the desired output encoding otherwise we just use cv_bridge to get it to an array / cvMat. This means if put in raw rgb8 images they are passed as is to the cv image encoder, which can not infer the type anymore. You might want to store it as is and thus we let the user force it color conversion. We could provide a flag to autoforce it for rgb to bgr only. But if you know your input is rgb/bgr than this is not much difference then setting the output encoding (see flags linked above).

https://github.com/AIT-Assistive-Autonomous-Systems/ros2bag_tools/blob/585d863596a95ef248100a0b5fc91a62f22ef96d/ros2bag_tools/ros2bag_tools/exporter/image.py#L145C1-L147C51

Does this solve your issue or have any other Ideas/remarks?

Regarding more logging:

We added logs only recently using the ros2bag tools logging infrastructure with the sync tools. Thus most older components do not have it yet. PRs.

So there will be two places. The exporters / filters themselves for any errors or post states (like sync with the amount of messages being dropped). Don't forget that some components will be called as a chain. Thus providing progress per component will generate a lot of logs with the current infrastructure

We would need to check if it could be possible to update logss instead. But I have not looked into that (tqdm style) if there is one that does not interfere with the logs and integrates with rosbag2.

A progress log for every n-secs/messages could be probably achieved on verb level even without tqdm like integration.

devrite commented 7 months ago

@AiVerisimilitude any comments?

abhishek47kashyap commented 7 months ago

Thank you for the response, I could export to a specified directory and also enforce input and output encodings (BGR to RGB conversion) with the following CLI command:

ros2 bag export --in ./some_rosbag --topic /camera/rgb/image_raw image --dir ./some_output_dir/ --input-encoding bgr8 --output-encoding rgb8

(It would be nice to have such complete CLI examples in the readme.)


Regarding logging and assert messages:

We added logs only recently using the ros2bag tools logging infrastructure with the sync tools. Thus most older components do not have it yet. PRs.

I may come back to this. If anyone else wants to have a go please feel free.