gbiggs / rtshell

Shell commands for managing RT-Middleware running on OpenRTM-aist.
http://openrtm.org
GNU Lesser General Public License v3.0
10 stars 17 forks source link

Rtstodot draws a right-to-left graph #38

Closed rafaelxero closed 7 years ago

rafaelxero commented 7 years ago

The dot file generated by rtstodot places the "in" ports on the right of the components, and the "out" ports on the left, leading to very confusing plotting by dot who tries to keep at best the left-to-right structure.

The fix is simple, in rtstodot.py:72:

- label_str = '{{{{{0}}}|{1}|{{{2}}}}}'.format(in_ports_str[:-1], c.instance_name,
-            out_ports_str[:-1])
+ label_str = '{{{{{0}}}|{1}|{{{2}}}}}'.format(out_ports_str[:-1], c.instance_name,
+            in_ports_str[:-1])

I don't know why dot uses this syntax though...

rafaelxero commented 7 years ago

Note: I am using the pip version, it might have been corrected since then but the outputted file contains this as label:

"{{out_ports}|name|{in_ports}}"

While we obviously want the reverse. I think that the in_ports and out_ports logic in rtstodot is reversed somehow...

rafaelxero commented 7 years ago

Ok I think I found the culprit:

    for dp in rtsp.data_port_connectors:
        in_ports.append((dp.source_data_port.instance_name,
            port_name(dp.source_data_port.port_name)))
        out_ports.append((dp.target_data_port.instance_name,
            port_name(dp.target_data_port.port_name)))
    for sp in rtsp.service_port_connectors:
        out_ports.append((dp.source_data_port.instance_name,
            port_name(dp.source_data_port.port_name)))
        in_ports.append((dp.target_data_port.instance_name,
            port_name(dp.target_data_port.port_name)))

Shouldn't the first loop fill in_ports with target connectors and the out_ports with source connectors? That way it would also match the logic of service ports...

Note: it might be beneficial to refactor into

for dp in rtsp.data_port_connectors+rtsp.service_port_connectors:
  #logic
gbiggs commented 7 years ago

Fixed by #39