Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions rosidl_runtime_py/set_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

import array
import copy
from functools import partial

from typing import Any
Expand Down Expand Up @@ -54,6 +55,8 @@ def set_message_fields(
def set_message_fields_internal(
msg: Any, values: Dict[str, str],
timestamp_fields: List[Any]) -> List[Any]:
# Create a deep copy of the input dictionary to avoid modifying it for safety
values = copy.deepcopy(values)
Comment on lines +58 to +59
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function handles the nested structure internally, i would use deepcopy for the safety.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have any sort of performance impact?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course it does. but i think this is not performance sensitive API. (besides python...)

try:
items = values.items()
except AttributeError:
Expand Down
25 changes: 25 additions & 0 deletions test/rosidl_runtime_py/test_set_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,31 @@ def test_set_nested_namespaced_fields():
assert arrays_msg.basic_types_values[2].uint8_value == 0


def test_set_nested_namespaced_fields_with_same_values():
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails with out this PR.

unbounded_sequence_msg = message_fixtures.get_msg_unbounded_sequences()[1]
test_values = {
'basic_types_values': [
{'float64_value': 42.42, 'int8_value': 42},
{'float64_value': 11.11, 'int8_value': 11}
]
}
set_message_fields(unbounded_sequence_msg, test_values)
assert unbounded_sequence_msg.basic_types_values[0].float64_value == 42.42
assert unbounded_sequence_msg.basic_types_values[0].int8_value == 42
assert unbounded_sequence_msg.basic_types_values[0].uint8_value == 0
assert unbounded_sequence_msg.basic_types_values[1].float64_value == 11.11
assert unbounded_sequence_msg.basic_types_values[1].int8_value == 11
assert unbounded_sequence_msg.basic_types_values[1].uint8_value == 0
# This should still succeed because test_values should not be modified
set_message_fields(unbounded_sequence_msg, test_values)
assert unbounded_sequence_msg.basic_types_values[0].float64_value == 42.42
assert unbounded_sequence_msg.basic_types_values[0].int8_value == 42
assert unbounded_sequence_msg.basic_types_values[0].uint8_value == 0
assert unbounded_sequence_msg.basic_types_values[1].float64_value == 11.11
assert unbounded_sequence_msg.basic_types_values[1].int8_value == 11
assert unbounded_sequence_msg.basic_types_values[1].uint8_value == 0


def test_set_message_fields_nested_type():
msg_basic_types = message_fixtures.get_msg_basic_types()[0]
msg0 = message_fixtures.get_msg_nested()[0]
Expand Down