jseldent / wireshark-dds-xrce

DDS-XRCE Protocol dissector for Wireshark
Apache License 2.0
8 stars 3 forks source link

Add menu to register object profile, topic and/or type #2

Closed jseldent closed 2 years ago

jseldent commented 2 years ago

This pull requests adds a menu that allows the user to manually specify the profile and/or topic name of an object. This makes it possible to deserialize payloads, even if the creation of the entities was not captured.

gavanderhoorn commented 2 years ago

Tested it. Works perfectly.

Might be nice to indicate which fields are absolutely required?

Would be even more perfect if it would be a context menu which opens when right-clicking on the object_id field of the already dissected payload field :sweat_smile:. The ID field could then be auto-populated from the value of payload.

And seeing as a user already needs to fill in the topic_type_names and types tables, would it perhaps be possible to make the profile and topic fields dropdowns in the Set object dialog?

gavanderhoorn commented 2 years ago

Oh, and just thought of this: what about a way for users to store the mapping in a companion .lua file, next to a .pcap(ng) file with the same name?

Would save recreating it every time the file gets loaded.

(and by store, I mean writing it by hand. Not that the dissector should necessarily be able to serialise the tables and mapping)


Edit: the pcapng format supports user comments, both at the file and per-packet level. Might be able to use those to store the information needed by the dissector.

jseldent commented 2 years ago

I've looked into creating a context menu, but as far as I can see, you can only create regular menus with Wireshark. And it seems that it's only possible to create text fields in dialog windows, not drop-down lists.

As for saving and loading the mapping to file, I'll have to look into that.

gavanderhoorn commented 2 years ago

I've looked into creating a context menu, but as far as I can see, you can only create regular menus with Wireshark. And it seems that it's only possible to create text fields in dialog windows, not drop-down lists.

yes, I agree. Spent some time looking through the WS Lua API docs and it doesn't appear this is supported.

An alternative flow could be:

That would not need a context menu.

I'll see if I can add that later. It's just convenience of course.

gavanderhoorn commented 2 years ago

As an update: this has been absolutely fantastic trying to make sense of a captured XRCE-DDS exchange.

Example dissection of a control_msgs_action_FollowJointTrajectory_Feedback message from micro-ROS to a ROS 2 node:

Click to expand ```yaml DDS-XRCE Protocol, sessionId: 0x81, streamId: 0x80 sessionId: 0x81 streamId: 0x80 (STREAMID_BUILTIN_RELIABLE) reliability: RELIABLE sequenceNr: 59522 FRAGMENT submessageId: 13 flags: 0x03, Last Fragment bit, Endianness bit .... ..1. = Last Fragment bit: Set (0x1) .... ...1 = Endianness bit: Set (0x1) submessageLength: 24 payload: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 WRITE_DATA submessageId: 7 flags: 0x01, FORMAT_DATA, Endianness bit .... 000. = DataFormat: FORMAT_DATA (0x0) .... ...1 = Endianness bit: Set (0x1) submessageLength: 520 payload (WRITE_DATA_Payload_Data) request_id: eb08 object_id: 0045 (DATAWRITER) [topic: follow_joint_trajectory/feedback] [type: control_msgs_action_FollowJointTrajectory_Feedback] data serialized_data (control_msgs_action_FollowJointTrajectory_Feedback) header: stamp: sec: 1667560689 nanosec: 292663282 frame_id: joint_names: [0] joint_1_s [1] joint_2_l [2] joint_3_u [3] joint_4_r [4] joint_5_b [5] joint_6_t desired: positions: [0] -1,1056729197868 [1] 0,078984450729999 [2] -0,21364010920187 [3] 3,1439919131703 [4] 1,2757758196946 [5] -2,0344917177803 velocities: [empty] accelerations: [empty] effort: [empty] time_from_start: sec: 0 nanosec: 0 actual: positions: [0] -1,0808321755732 [1] 0,018796922777456 [2] -0,25565574650528 [3] 3,1445080692983 [4] 1,2935085593382 [5] -2,0590944999525 velocities: [0] 0,11955462765584 [1] 0,27226823038587 [2] 0,19939868945321 [3] 0 [4] 0,070685599912793 [5] 0,12085772868228 accelerations: [empty] effort: [0] -0,541872 [1] 0,226098 [2] 1,673924 [3] 0,02226 [4] -0,092379 [5] 0,106157 time_from_start: sec: 0 nanosec: 0 error: positions: [0] -0,024840744213665 [1] 0,060187527952543 [2] 0,042015637303406 [3] -0,00051615612803468 [4] -0,01773273964363 [5] 0,024602782172236 velocities: [empty] accelerations: [empty] effort: [empty] time_from_start: sec: 0 nanosec: 0 multi_dof_joint_names: [empty] multi_dof_desired: transforms: [empty] velocities: [empty] accelerations: [empty] time_from_start: sec: 0 nanosec: 0 multi_dof_actual: transforms: [empty] velocities: [empty] accelerations: [empty] time_from_start: sec: 0 nanosec: 0 multi_dof_error: transforms: [empty] velocities: [empty] accelerations: [empty] time_from_start: sec: 0 nanosec: 0 ```

Edit: one thing I'm wondering about: could we remove the need for a topic name? I realise the dissector currently uses it to lookup type information, but couldn't the object_id be used for this directly?

jseldent commented 2 years ago

An alternative flow could be:

  • user selects packet (in packet listing) which contains the XRCE DDS pkt with the payload he wants to associate with a particular custom dissector
  • dissector stores the payload value in a wireshark dissector field (as part of the normal dissector control flow)
  • code-behind-dialog retrieves the value of the field which stores the ID for the selected packet

That would not need a context menu.

I've looked into that, but I can't find a way of discovering which packet is selected by the user. If we could, the rest would be easy. We could even pre-fill the profile and topic if we already have values for them.

Edit: one thing I'm wondering about: could we remove the need for a topic name? I realise the dissector currently uses it to lookup type information, but couldn't the object_id be used for this directly?

I've added a type field to the "Set object..." dialog. You can use that to manually specify the type of an object. It takes precedence over the inferred type from the topic or profile name.

gavanderhoorn commented 2 years ago

I've looked into that, but I can't find a way of discovering which packet is selected by the user.

would that be needed?

If instead of generic tvb text items, the dissector would add a ProtoField for payload.object_id (similar to f_submessage_id and the others), retrieving the value for that field would always return the value for the currently dissected (ie: displayed) packet AFAIK. That's at least how it works for me in other dissectors.

jseldent commented 2 years ago

If instead of generic tvb text items, the dissector would add a ProtoField for the object_id (similar to f_submessage_id and the others), retrieving the value for that field would always return the value for the currently dissected (ie: displayed) packet AFAIK. That's at least how it works for me in other dissectors.

The problem is that there are often multiple submessages in a single packet, each with their own object ID. So it wouldn't be clear which ID to use.

Also, in the current implementation the deserialization of the submessages (in dds-xrce-types.lua) is separate from the main dissector code (in dds-xrce-proto.lua). So it's not trivial to use a field instead of a text item for the object ID.

jseldent commented 2 years ago

I've tried to use a Field extractor, but you cannot use those outside a dissector. So it doesn't work for a GUI dialog.

However, the dissector implementation already tracks the object ID of the most recently dissected packet. Which, in practice, is the currently selected packet. If there are multiple submessages, it's the object ID of the last submessage. This is actually a useful default. For example, CREATE messages are often combined, with the last message being the data reader/writer. And that's generally the one you want to configure anyway.

I've added the known values for ID, profile, topic and type name to the dialog. It seems to work quite well.

jseldent commented 2 years ago

I'll merge this now. Saving/loading the object type info can be a future PR.