search

ros2 / rosidl_python

rosidl support for Python
Apache License 2.0
19 stars 45 forks source link

Add null support for constants and defaults #208

Open Ryanf55 opened 3 months ago

Ryanf55 commented 3 months ago

Purpose

Finish out the Null support in ROSIDL_python.

Tests Performed

We've tested this on humble and it works great! Note, I added a bunch more checks for equality that should protect against regressions in similar areas.

Ticket

Fixes #207

Attribution

This work was contributed on behalf of the MacCreadyWorks department at AeroVironment, Inc

mjcarroll commented 3 months ago

I think this brings up kind of a larger question. By IEEE-754 definition nan != nan, so if we have a message with a single float data field, both set to nan, should those messages be equal to each other?

Ryanf55 commented 3 months ago

I think this brings up kind of a larger question. By IEEE-754 definition nan != nan, so if we have a message with a single float data field, both set to nan, should those messages be equal to each other?

To me, they are the same message. You are checking equality of the messages and practically means the contents are the same.

For purposes of the equality, I use the operator just before a call to publish to check if the message is the same. If it's the same, publishing the same thing is a waste of CPU. That said, we can't assume that use case for everyone. As implemented, I vote for this behavior, but am open to other options.

mjcarroll commented 3 months ago

I'm going to bring this up at our next team meeting, because I think you aren't wrong, but want to make sure that we think through the potential impacts.

Ryanf55 commented 1 month ago

I'm going to bring this up at our next team meeting, because I think you aren't wrong, but want to make sure that we think through the potential impacts.

Did you get any feedback? I'm still carrying all these patches internally, and would love to get them merged!