rosidl icon indicating copy to clipboard operation
rosidl copied to clipboard

Inconsistent APIs between String and U16String

Open allsey87 opened this issue 1 year ago • 1 comments

Bug report

rosidl_runtime_c__U16String__resize exists, however, there is no rosidl_runtime_c__String__resize.

Required Info:

  • Version or commit hash: Rolling

Expected behavior

Consistent APIs between String and U16String

Actual behavior

Inconsistent APIs between String and U16String

Feature request

Make APIs consistent?

Feature description

Sometimes a char * is not directly available, e.g., from a deserialization library. This means that it is not possible to use assignn without allocating an intermediate buffer. Having a resize function for both String and U16String would solve this problem.

Implementation considerations

The workaround I will use is to use rcutils_get_default_allocator to update the internals of the string manually.

allsey87 avatar Aug 28 '23 16:08 allsey87

Thank you for the report! It looks like a resize function is missing. There's a resize_assignn test, but it doesn't resize an existing string. I'll leave this open as a help wanted. Fixing it means doing the following:

Make a declaration similar to this one, but using char instead of uint16_t and put it in string_functions.h

https://github.com/ros2/rosidl/blob/d66b6db0cb93963bdf11b3b315d95fd5fc5f0ef5/rosidl_runtime_c/include/rosidl_runtime_c/u16string_functions.h#L156-L167

Implement it similar to this function, but using char instead of uint16_t and put it in string_functions.c

https://github.com/ros2/rosidl/blob/d66b6db0cb93963bdf11b3b315d95fd5fc5f0ef5/rosidl_runtime_c/src/u16string_functions.c#L167-L188

Take the expectations on U16String_resize from the test below, and put them into into test_string_functions.cpp for the new function.

https://github.com/ros2/rosidl/blob/d66b6db0cb93963bdf11b3b315d95fd5fc5f0ef5/rosidl_runtime_c/test/test_u16string_functions.cpp#L73-L147

sloretz avatar Sep 08 '23 23:09 sloretz

Seems this issue is resolved by https://github.com/ros2/rosidl/pull/806 . @allsey87 or @sloretz could you please close this issue?

r7vme avatar Jul 27 '24 15:07 r7vme