Fix echo for big array messages

[yechun1]

Issue1: ros2 topic echo pointcould2(big arrays), has no response, updated the logical to make more sensible.

a. (by default) full_length=false, truncate_length=128, then print max 128 (fix big arrays issue)

b. pass truncate_length=X, then print max X.

c. pass full_length=true (whatever truncate_length), then set truncate_length=None and print full_length.

Issue2: pass truncate_length to _convert_value(), this should be required.

Signed-off-by: Chris Ye chris.ye@intel.com

connect to #127

[dirk-thomas]

Since truncate_length is a keyword argument please pass it explicitly as such: truncate_length=truncate_length.

[yechun1]

@dirk-thomas thanks for review, updated the code.

[mikaelarguedas]

lgtm, thanks @yechun1 for the patch!

[mikaelarguedas]

One surprising thing (but not directly related to this change) is that we sometimes end up with '...' and sometimes with '.....' for the truncated part (note it's 5 and not 6 dots, pretty surprising).

As this is a minor detail compared to the improvement provided by this PR, I'll merge this as is a look at or ticket the minor display hiccup

e.g.:

$ ros2 topic echo /image sensor_msgs/Image -l 3
header:
  stamp:
    sec: 0
    nanosec: 0
  frame_id: '29'
height: 240
width: 320
encoding: bgr...
is_bigendian: 0
step: 960
data: [0, 0, 0, '...']

$ ros2 topic echo /image sensor_msgs/Image -l 2
header:
  stamp:
    sec: 0
    nanosec: 0
  frame_id: '12'
height: 240
width: 320
encoding: bg...
is_bigendian: 0
step: 960
data: [0, 0, '.....']

[dirk-thomas]

As this is a minor detail compared to the improvement provided by this PR, I'll merge this as is a look at or ticket the minor display hiccup

Is the "minor display hiccup" introduced by this PR or can it be reproduced without? If it is the former this shouldn't be merged until the "hiccup" is addressed. If it is the latter this can be safely merged and the "hiccup" should be fixed (or at least ticketed if the time to fix it is significant).

[mikaelarguedas]

As the truncation doesnt work without this patch, not sure how to reproduce it without applying this.

I'll have a quick look at the hiccup later today and will merge this + ticket or fix the hiccup before end of day

[yechun1]

root caused the issue related with previous code which inside nested _convert_value function.

Execute: "ros2 topic echo /topic -l 2"

with test code, first time convert data to (first if, for t in (bytes, list, str, tuple))

value [0, 0, '...']
k in value [0, 0, '...']
k: 0
k: 0
k: ...

second time call nested convert_value by (second elfi, for t in (list, tuple))
when k=...
len(k) = 3, truncate_length = 2.
convert the '...' as real data as '..' and append another '...'
so the result is "....."

while "ros2 topic echo /topic -l 3" is correct
when k='...'
if (len("...") > truncate_length) is not ture. skip convert.

Is it possible to remove nested function for _convert_value, it's a little difficult to understand.

[mikaelarguedas]

Thanks @yechun1 for looking into it. I ticketed it at #129 so that this can be merged and backported

[mikaelarguedas]

• Linux
• Linux-aarch64
• macOS
• Windows