[Bug] --queries-timeout 60.0 invalid type: floating point

[Fellfalla]

Describe the bug

Running the bridge I get following error with the queries timeout arg:

ros2 run zenoh_bridge_ros2dds zenoh_bridge_ros2dds \
        -l tcp/0.0.0.0:7447 \
        --no-multicast-scouting \
        --queries-timeout 60.0
[2023-12-10T13:34:21Z INFO  zenoh_bridge_ros2dds] zenoh-bridge-ros2dds veb66be5 built with rustc 1.70.0 (90c541806 2023-05-31) (built from a source tarball)
[2023-12-10T13:34:21Z INFO  zenoh::net::runtime] Using PID: 6b8adfe3b7ac3d9f50bb8cce3dcdc2c4
[2023-12-10T13:34:21Z INFO  zenoh::net::runtime::orchestrator] Zenoh can be reached at: tcp/192.168.178.32:7447
[2023-12-10T13:34:21Z INFO  zenoh::net::runtime::orchestrator] Zenoh can be reached at: tcp/172.17.0.1:7447
[2023-12-10T13:34:21Z INFO  zenoh::net::runtime::orchestrator] Zenoh can be reached at: tcp/172.22.0.1:7447
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Plugin `ros2dds` configuration error: invalid type: floating point `60`, expected struct QueriesTimeouts at zenoh-plugin-ros2dds/src/lib.rs:130.', zenoh-bridge-ros2dds/src/main.rs:220:66
stack backtrace:
   0:     0x61183eabcef7 - std::backtrace_rs::backtrace::libunwind::trace::hc8c748621e1717dc
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x61183eabcef7 - std::backtrace_rs::backtrace::trace_unsynchronized::h1657ffb548bcd1c0
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x61183eabcef7 - std::sys_common::backtrace::_print_fmt::ha3920362e42412b1
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/sys_common/backtrace.rs:65:5
   3:     0x61183eabcef7 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h55a64e3639141d5c
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x61183e8c9bdf - core::fmt::write::h8a6836b78d5c6e17
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/core/src/fmt/mod.rs:1254:17
   5:     0x61183eaca8f4 - std::io::Write::write_fmt::h74083ba874a8bcaf
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/io/mod.rs:1698:15
   6:     0x61183eabcb9f - std::sys_common::backtrace::_print::he2a2a37b05e7e764
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/sys_common/backtrace.rs:47:5
   7:     0x61183eabcb9f - std::sys_common::backtrace::print::hf1818e0fa9ba5bd4
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/sys_common/backtrace.rs:34:9
   8:     0x61183eac80ee - std::panicking::default_hook::{{closure}}::h36f8b00cf3f70373
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/panicking.rs:269:22
   9:     0x61183eac8c41 - std::panicking::default_hook::h0999e9f322268afa
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/panicking.rs:288:9
  10:     0x61183eac8c41 - std::panicking::rust_panic_with_hook::hf0f1736a09665bd3
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/panicking.rs:691:13
  11:     0x61183eabd304 - std::panicking::begin_panic_handler::{{closure}}::h9c059812511f285d
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/panicking.rs:582:13
  12:     0x61183eabd266 - std::sys_common::backtrace::__rust_end_short_backtrace::haeaaa791abdbfc5f
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/sys_common/backtrace.rs:150:18
  13:     0x61183eac84f1 - rust_begin_unwind
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/std/src/panicking.rs:578:5
  14:     0x61183e773fc2 - core::panicking::panic_fmt::h42c1b10c64151f1e
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/core/src/panicking.rs:67:14
  15:     0x61183e773e72 - core::result::unwrap_failed::h4b24ec377bba078a
                               at /build/rustc-wAuwbs/rustc-1.70.0+dfsg0ubuntu1~bpo2/library/core/src/result.rs:1687:5
  16:     0x61183e7af5d1 - <async_std::task::builder::SupportTaskLocals<F> as core::future::future::Future>::poll::h31d76bd20aab43e1
  17:     0x61183e8330d4 - zenoh_bridge_ros2dds::main::h0c723d0e67799cff
  18:     0x61183e7f15b3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h1418525716f9748f
  19:     0x61183e8343d3 - main
  20:     0x71ab3632ad90 - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  21:     0x71ab3632ae40 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:392:3
  22:     0x61183e7a85b5 - _start
  23:                0x0 - <unknown>
[ros2run]: Aborted
make: *** [Makefile:54: zenoh] Error 250

To reproduce

Run

ros2 run zenoh_bridge_ros2dds zenoh_bridge_ros2dds \
		-l tcp/0.0.0.0:7447 \
		--no-multicast-scouting \
		--queries-timeout 60.0

System info

• Ubuntu 22.04 in docker image based on osrf/ros:humble-desktop-full
• zenoh-plugin-ros2dds hash: eb66be5

[Fellfalla]

@JEnoch I had a look at it and I think something with the argument parsing in main.rs line 183 is didn't get updated accordingly with commit 63382f2. As far as i understand, config.rs expects a QueriesTimeout struct rather than a simple f64.

We could apply the timeout as default to all possible values inside the struct:

• for all transient_local_subscribers: transient_local_subscribers: <my-timeout>
• for all services: services: [".*=<my-timeout>"]
• for all actions actions: { send_goal: <my-timeout>, cancel_goal: <my-timeout>, get_result: [".*=<my-timeout>"] }

[JEnoch]

You correctly analysed the issue: I forgot to update the --queries-timeout CLI argument following #19 ... 🤦🏻

My only concern with the solution you propose is that by Zenoh convention, all the CLI arguments overwrite the configurations provided by a file (via -c). Thus, a single float set with --queries-timeout will possibly overwrite a bunch of different timeout settings.
Why not... but I think in such case a warning should be logged.
What do you think ?

[Fellfalla]

Sounds like a solution. Alternatively we can put the same structure that is going to be parsed as a string into the cli. But for that case users would need some example in the docs on that syntax.

And Im not familiar enough with the code to quickly implement this 😅.

My solution internally will be to start using the config file instead.

[JEnoch]

I finally preferred to explicitly expose a queries_timeout/default setting in configuration file that applies if a timeout for a query is not explicitly set via regex in other queries_timeout parameters.
And replace the --queries-timeout argument with a --queries-timeout-default argument that overwrites this queries_timeout/default setting.
See #35 for details.