Wrong MD5 sum for ROS messages with String constants

[rgleichman]

Roshask generates the wrong MD5 sum for messages with String constants. The culprit is sanitizeConstants. If sanitizeConstants is changed from

sanitizeConstants :: (a, MsgType, ByteString) -> (a, MsgType, ByteString)
sanitizeConstants (name, RString, val) = 
    (name, RString, B.concat ["\"", escapeQuotes val,"\""])
  where escapeQuotes = B.intercalate "\\\"" . B.split '"'
sanitizeConstants (name, t, val) = 
    (name, t, B.takeWhile (\c -> c /= '#' && not (isSpace c)) val)

to the identity for strings

sanitizeConstants :: (a, MsgType, ByteString) -> (a, MsgType, ByteString)
sanitizeConstants x@(_, RString, _) = x
sanitizeConstants (name, t, val) = 
    (name, t, B.takeWhile (\c -> c /= '#' && not (isSpace c)) val)

It generates the correct MD5 sum.

Example msg file: arm_navigation_msgs/AttachedCollisionObject Its MD5 should be "3fd8ca730863e3d97d109c317d106cf9"

# The CollisionObject will be attached with a fixed joint to this link
# If link name is set to REMOVE_ALL_ATTACHED_OBJECTS and object.operation 
# is set to REMOVE will remove all attached bodies attached to any object
string link_name

#Reserved for indicating that all attached objects should be removed
string REMOVE_ALL_ATTACHED_OBJECTS = "all"

#This contains the actual shapes and poses for the CollisionObject
#to be attached to the link
#If action is remove and no object.id is set, all objects
#attached to the link indicated by link_name will be removed
CollisionObject object

# The set of links that the attached objects are allowed to touch
# by default - the link_name is included by default
string[] touch_links

[acowley]

Okay, we can fix this by sanitizing strings after generating the MD5 during Haskell code gen, but this still leaves me uncertain how to handle string constants. The one example on the wiki has a quoted part and an unquoted part, how would we want to handle that? I suspect they are counting on a loosely specified "do what I mean" interpretation, in which case we could detect the case where a constant is bracketed by quotes. The simplest fix is to just change how the Haskell message code is generated to avoid double quotes.

[acowley]

To follow up here: I don't have a horse in this race, so I could use some input. Do you think your example (string BLAH = "all") is representative? Would you expect the string constant to have quotation marks as part of its content?

The spec says the entire line after the equals sign is taken after trimming leading and trailing whitespace. Given that rule, the use of quotation marks only makes sense to me if they are intended to be included in the string literal rather than delimiting its extents.

Edit to clarify: My feeling is that we should escape the quotation marks in the generated code, but we just have to be careful to not escape them before computing the MD5. Your example would then produce a string literal "\"all\"".

[rgleichman]

String constants in ROS messages seem uncommon, so I would support escaping quotation marks if that would be more consistent with the specification.

[acowley]

Can you check if the commit referenced there (the StringConstants branch) works for you? I can't run against the test case you gave as I don't have moveit installed.

[rgleichman]

Yes, StringConstants generates the correct MD5 for the example message arm_navigation_msgs/AttachedCollisionObject.

[acowley]

Thanks, this is now merged into the dev and master branches.

[acowley]

Added a set of quotation marks around the string literals in generated message code.

[acowley]

Hopefully this is fixed now, but, even if it's not we at least have tests and expected output in the repo to pin things down.