RobotWebTools / rosbridge_suite

Server Implementations of the rosbridge v2 Protocol
https://robotwebtools.github.io
BSD 3-Clause "New" or "Revised" License
866 stars 506 forks source link

rosapi tries to parse `IDL` files as `msg` files and errors #913

Open russkel opened 3 months ago

russkel commented 3 months ago

Description

If there is no msg files get_interface_path will return an idl path. This is then fed into the parse_message_string function for msg files. This ends up with a lot of errors spamming in the log.

https://github.com/RobotWebTools/rosbridge_suite/blob/ros2/rosapi/src/rosapi/stringify_field_types.py#L20

https://github.com/RobotWebTools/rosbridge_suite/blob/ros2/rosapi/src/rosapi/stringify_field_types.py#L24

Steps To Reproduce

Expected Behavior

Either parses IDL using the rosidl parser or errors once it detects the IDL extension and doesn't try to load it again.

Actual Behavior

[rosapi_node-19] Error processing '// generated from nmea0183_adapter/resource/msg_definition.idl.em' of 'nmea0183_msgs/UtcDateTime': '//'
charlielito commented 2 weeks ago

I think this happens for custom-built messages. I managed to implement a workaround to be able to parse them:

def extract_msg_path_from_idl(idl_path):
    with open(idl_path, 'r', encoding='utf-8') as idl_file:
        idl_content = idl_file.read()

    # Try to extract the original .msg file path from content. Usually in the first lines
    # there is a part that says "// with input from <path>" and path is related to the original .msg file
    msg_file_info = [line for line in idl_content.split('\n') if '// with input from' in line]
    if msg_file_info:
        msg_path = msg_file_info[0].split('// with input from')[-1].strip()
        # Remove "msg/" from the path, since the actual path doesn't have it
        msg_path = msg_path.replace('msg/', '')
        # Find parent directory of the idl just before the share folder
        share_dir = idl_path.split('share')[0]
        # Construct the full path to the .msg file
        return os.path.join(share_dir, 'share', msg_path)

    return idl_path

def stringify_field_types(root_type):
    definition = ""
    seen_types = set()
    deps = [root_type]
    is_root = True
    while deps:
        ty = deps.pop()
        parts = ty.split("/")
        if not is_root:
            definition += "\n================================================================================\n"
            definition += f"MSG: {ty}\n"
        is_root = False

        msg_name = parts[2] if len(parts) == 3 else parts[1]
        interface_name = ty if len(parts) == 3 else f"{parts[0]}/msg/{parts[1]}"

        # Try to get the .msg file first
        msg_path = get_interface_path(interface_name)

        # If custom we get instead the .idl implementation
        if msg_path.endswith('.idl'):
            msg_path = extract_msg_path_from_idl(msg_path)

        with open(msg_path, encoding="utf-8") as msg_file:
            msg_definition = msg_file.read()
        definition += msg_definition

        spec = parse_message_string(parts[0], msg_name, msg_definition)
        for field in spec.fields:
            is_builtin = field.type.pkg_name is None
            if not is_builtin:
                field_ty = f"{field.type.pkg_name}/{field.type.type}"
                if field_ty not in seen_types:
                    deps.append(field_ty)
                    seen_types.add(field_ty)

    return definition

Hope it helps

russkel commented 2 weeks ago

I have a PR on rosidl_parser which adds a parser for IDL files, but as per tradition it just sits there unreviewed.

Thanks for your patch, but we stopped using rosbridge and changed to foxbridge due to performance increases.

charlielito commented 2 weeks ago

@russkel can you point me to the PR url? Thanks!

russkel commented 2 weeks ago

https://github.com/ros2/rosidl/pull/697

@russkel can you point me to the PR url? Thanks!