zoranbosnjak / asterix-specs

EUROCONTROL asterix specifications in structured format.
https://zoranbosnjak.github.io/asterix-specs/
BSD 3-Clause "New" or "Revised" License
23 stars 14 forks source link

Extra VALUE, in the field name #14

Closed princepaul closed 2 years ago

princepaul commented 2 years ago

Python script is adding VALUE, to the field name. Highlighted with Yellow Text in attached image for easy reference.

For eg;

    { &hf_001_V1_2_120_VALUE, { "VALUE, Measured Radial Doppler Speed, [NM/s]", "asterix.001_V1_2_120_VALUE", FT_DOUBLE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_130, { "130, Radar Plot Characteristics", "asterix.001_V1_2_130", FT_NONE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_130_IND, { "IND, Indicator", "asterix.001_V1_2_130_IND", FT_UINT8, BASE_DEC, NULL, 0xfe, NULL, HFILL } },
    { &hf_001_V1_2_131, { "131, Received Power", "asterix.001_V1_2_131", FT_NONE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_131_VALUE, { "VALUE, Received Power, [dBm]", "asterix.001_V1_2_131_VALUE", FT_DOUBLE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_141, { "141, Truncated Time of Day", "asterix.001_V1_2_141", FT_NONE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_141_VALUE, { "VALUE, Truncated Time of Day, [s]", "asterix.001_V1_2_141_VALUE", FT_DOUBLE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_150, { "150, Presence of X-Pulse", "asterix.001_V1_2_150", FT_NONE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_150_XA, { "XA", "asterix.001_V1_2_150_XA", FT_UINT8, BASE_DEC, VALS (valstr_001_V1_2_150_XA), 0x80, NULL, HFILL } },
    { &hf_001_V1_2_150_XC, { "XC", "asterix.001_V1_2_150_XC", FT_UINT8, BASE_DEC, VALS (valstr_001_V1_2_150_XC), 0x20, NULL, HFILL } },
    { &hf_001_V1_2_150_X2, { "X2", "asterix.001_V1_2_150_X2", FT_UINT8, BASE_DEC, VALS (valstr_001_V1_2_150_X2), 0x04, NULL, HFILL } },
    { &hf_001_V1_2_161, { "161, Track Plot Number", "asterix.001_V1_2_161", FT_NONE, BASE_NONE, NULL, 0x00, NULL, HFILL } },
    { &hf_001_V1_2_161_VALUE, { "VALUE, Track Plot Number", "asterix.001_V1_2_161_VALUE", FT_UINT16, BASE_DEC, NULL, 0x00, NULL, HFILL } },

image

zoranbosnjak commented 2 years ago

I belive you are refering to asterix-specs to wireshark conversion python script (it's part of the wireshark project). Anyway, the "VALUE" comes from the following difference.

In asterix-specs, items (and subitems) are organized in a tree-like structure, for example:

I001/010        <- 16 bits (at level1, with deeper structure)
I001/010/SAC    <- 8 bits content (at level2)
I001/010/SIC    <- 8 bits content (at level2)

I001/120    <- 8 bit content (at level1), there is no level 2 here

Some items have content at deeper level (level 3, level 4). But in the case of I001/120, there is only one level. Indeed there is no need for another level, since there is no further structure.

The problem is that the current wireshark asterix dissector requires minimu 2 levels of item nesting, so for example I001/010/SAC is OK, but I001/120 is not. In such cases (to bridge the small difference between the structures), the python script generates a second level, but it needs to put a name to the generated subitem, such that you can later refer to the subitem by name. However, the python script could use any of the following:

princepaul commented 2 years ago

Yes, I was referring to the wireshark conversion python script which generates packet-asterix.c.

The problem is that the current wireshark asterix dissector requires minimu 2 levels of item nesting, so for example I001/010/SAC is OK, but I001/120 is not. In such cases (to bridge the small difference between the structures), the python script generates a second level, but it needs to put a name to the generated subitem, such that you can later refer to the subitem by name.

In that case, I think it will be better to sanitize the packet-asterix.c file by replacing the respective string (VALUE, after the generation of script for better visibility. Or, another routine for writing the same with only one level.

Screenshot is attached with old version on left and the right one with the current version generated using python script:

image

PS: This screenshot reveals another bug. The HEX is not decoded correctly. I noticed it after posting it here. Submitted a merge request at wireshark to resolve the same.

zoranbosnjak commented 2 years ago

@princepaul please check the following branch: https://gitlab.com/zoranbosnjak/wireshark/-/tree/asterix-value

I hope I understand your suggestion correctly. The change is to mark "generated" items and strip off the name (VALUE) in such cases. It now behaves like the left side (with the HEX problem being solved separately).

princepaul commented 2 years ago

@zoranbosnjak Thank you. It is fixed.

Regarding HEX, I thought that by changing decoding Raw from DEC to HEX will work. But it breaks all other DEC decoding. I think new type HEX can be defined for those need to be decoded as HEX or change the routine.

zoranbosnjak commented 2 years ago

@princepaul Please check this branch again (rebased to upstream master, with a new commit): https://gitlab.com/zoranbosnjak/wireshark/-/tree/asterix-value

I addition to the "VALUE" fix, this branch now also includes a change in HEX/DEC display. It's a little tricky, because this information is not stored in the asterix-specs. Nor it should be, because the HEX/DEC preferences are highly subjective and it's up to the application to decide how to display a value. Anyway, for the wireshark a reasonable general rule might be: if the raw value is long (2 bytes or more) and the bit length is a multiple of 8, then it's HEX, otherwise it's DEC. This is what this branch implements. So, for example, the "Aircraft Address" fits into the HEX rule, but small values, like SAC/SIC are displayed as DEC. Testcases are changed accordingly.

Please have a look. I will wait for your feedback, before making a merge request.

princepaul commented 2 years ago

@zoranbosnjak

I don't think 2 bytes or more will work for all the fields.

For eg:

  1. CAT 004/020 (Time of Mesage) is 3 bytes decimal
  2. CAT 004/030 (Track Number 1) is 2 bytes decimal
  3. CAT 004/035 (Track Number 2) is 2 bytes decimal
  4. CAT 004/040 (Alert Identifier) is 2 bytes decimal
  5. CAT 011/380 (B1B) is 1 byte (bits 12/9) but decoded as hex

There are lot of other fields in various Categories as example. I only pointed out what ever in CAT 004.

I think it will be good to define type "hex" explicitly and decode as "hex". Only certain fields are decoded as hex in the entire ASTERIX family.

zoranbosnjak commented 2 years ago

@princepaul I understand your point. Adding a new "hex" tag to some items would be a structural change. To solve the HEX problem, it would be better, to redefine some items from "raw", to "unsigned integer". The hex rule I've mentioned applies only to "raw" items. The "unsigned integer" items would always be in decimal. So, for example from the list above:

  1. Is not defined as raw, so this should be OK already (please check)
  2. replace "raw" with "unsigned integer"
  3. replace "raw" with "unsigned integer"
  4. replace "raw" with "unsigned integer"
  5. This is only half of byte, why would one want it to be shown in hex?

Would this solve the HEX problem? It makes sense to me.

zoranbosnjak commented 2 years ago

After email discussion and testing, the problem was resolved in wireshark project, see https://gitlab.com/wireshark/wireshark/-/merge_requests/5967