Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework python ROS2 (de)serialization using parsed ROS2 messages directly #415

Merged
merged 14 commits into from
Jan 25, 2024

Conversation

phil-opp
Copy link
Collaborator

@phil-opp phil-opp commented Jan 20, 2024

Use ROS2 types as type info to ensure that serialization type matches exactly. This is necessary because the types need to match exactly, otherwise the serialization or deserialization will result in invalid data.

Using arrow schemas to specify the ROS2 message types does not work because not all ROS2 message types can be represented. For example, ROS2 serializes arrays with known length differently than sequences with dynamic length.

Other improvements:

Use ROS2 types as type info to ensure that serialization type matches exactly. This is necessary because the types need to match exactly, otherwise the serialization or deserialization will result in invalid data.

Using arrow schemas to specify the ROS2 message types does not work because not all ROS2 message types can be represented. For example, ROS2 serializes arrays with known length differently than sequences with dynamic length.
@phil-opp phil-opp changed the title Use ROS2 types as type info to ensure that serialization type matches exactly Rework python ROS2 (de)serialization using parsed ROS2 messages directly Jan 21, 2024
@phil-opp phil-opp marked this pull request as ready for review January 21, 2024 15:26
@phil-opp phil-opp requested a review from haixuanTao January 21, 2024 15:26
Arrow has a special `BinaryArray` type for storing lists of variable-sized binary data efficiently. This type is equivalent to a `ListArray` of `PrimitiveArray<u8>`, but it is a different data type.

This commit updates the Python ROS2 serialization code to permit BinaryArray for all uint8 array or sequence fields.
@haixuanTao haixuanTao changed the base branch from fix-ros2-array-bug to main January 23, 2024 15:37
haixuanTao and others added 4 commits January 24, 2024 12:21
Required because arrow uses column-oriented data format, which requires all struct fields to have length 1.
Copy link
Collaborator

@haixuanTao haixuanTao left a comment

Choose a reason for hiding this comment

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

Looks good to me. Pushed couple of amelioration. But that's it.

Comment on lines +32 to +45
let empty = HashMap::new();
let package_messages = self
.type_info
.messages
.get(self.type_info.package_name.as_ref())
.unwrap_or(&empty);
let message = package_messages
.get(self.type_info.message_name.as_ref())
.ok_or_else(|| {
error(format!(
"could not find message type {}::{}",
self.type_info.package_name, self.type_info.message_name
))
})?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing I think, message can be cached, otherwise this is going to happen at each ser/de.

@phil-opp phil-opp merged commit f9b142b into main Jan 25, 2024
17 checks passed
@phil-opp phil-opp deleted the ros2-type-info branch January 25, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants