open-dis / open-dis-java

Java implementation of the IEEE-1278.1 Distributed Interactive Simulation (DIS) application protocol v6 and v7 :boom:
Other
44 stars 35 forks source link

disutil PduFactory createPdu method is incomplete #32

Closed camejia closed 7 years ago

camejia commented 7 years ago

The createPdu method does not currently handle Entity State Update, or Electromagnetic Emissions (a.k.a. Electronic Emissions) PDUs, and possibly others. Recommend updating the switch statement to include cases for all known PDUs.

leif81 commented 7 years ago

Thanks Chris. Pull requests are welcome!

camejia commented 7 years ago

I've started working on this enhancement. Basically it's just a matter of adding additional cases, matching up pduTypePdu with the appropriate Pdu object type. There are a few that I'm having trouble with:

OTHER: Maybe there is no corresponding Pdu object type, handle this the same as the default case? IFF_ATC_NAVAIDS: Does this map to IffAtcNavAidsLayer1Pdu or IffAtcNavAidsLayer2Pdu? UNDERWATER_ACOUSTIC: Maybe map to UaPdu, but that can't be converted to a Pdu object TSPI: No suitable object found APPEARANCE: No suitable object found ARTICULATED_PARTS: No suitable object found LE_FIRE: No suitable object found LE_DETONATION: No suitable object found

I'd like to submit my code as is, with TODO or FIXME for the cases that still need to be resolved. Also, in the interest of not repeating code I'd like to combine the two createPdu() functions (one takes a byte[] array and the other takes a ByteBuffer) by making one a simpler wrapper around the other. Because I like ByteBuffer's I'm inclined to make that method do all the work, and make the byte[] version a wrapper around it. Let me know what you think.

leif81 commented 7 years ago

I'm not too sure about those either Chris. Maybe @mcgredonps knows.

I'd like to combine the two createPdu() functions

Yes great idea.

mcgredonps commented 7 years ago

I assume you're looking at the Java enum code, for example https://sourceforge.net/p/open-dis/code/HEAD/tree/utilities/Enumerations/src/main/java/edu/nps/moves/disenum/PduType.java .

IFF_ATC_NAVAIDS(28, "IFF/ATC/NAVAIDS"), for example.

This enum implementation is actually about one value light in the mapping. It contains

1) a name, capitals, generated from the XML document 2) the arbitrary but consistent number that maps to the name 3) The text directly from the SISO EBV document SISO_REF_010

What it doesn't contain is the Java class implementation name. That ought to be in there but I apparently skipped over it.

The Java code for open-DIS as a whole is generated from an XML file, which you can see at https://sourceforge.net/p/open-dis/code/HEAD/tree/xml/trunk/ . The idea was to write an XML document that describes the layout of PDUs. Once the XML file is written it's fairly straightforward to write an "compiler" that can generate Javascript, C#, Java, C++, Python, or whatever other language you like from the XML file. With maybe a thousand lines of code and a few days you can generate whatever language you like.

In this case, the best way to match up the Java class name with the number you see in the enumeration is to look in the above XML file, for example DIS2012.xml. Search for value="28", which sets the PDU type field for a subclass.

In this case IFF/ATC was commented out in the XML file:

mcgredonps commented 7 years ago

Ah, rats, the stuff I copied from the DIS2012.xml file got dropped in the above post because it looks like HTML. You should be able to see it in the file, though.

mcgredonps commented 7 years ago

That reference to the XML files I gave should have been at https://github.com/open-dis/DISDescription. The very clever Bob Murray and some other people are working on an XML description of DIS, too. Their version is a bit better, and they use an XML schema to describe the document as well, while here I just used well-formed XML. I don't think they've released it yet.

I've kicked around the idea of using JSON as a PDU description tool because it tends to be easier to parse, but reading a large JSON document as a human tends to make you cross-eyed if there's a typo somewhere. I don't think there's a real good reason to switch to that. Using ASN.1 doesn't really work, despite the conceptual overlap. The binary format is what is standardized, and an ASN.1 description would go to one of their binary formats rather than the IEEE DIS binary format.

leif81 commented 7 years ago

I believe PR #36 fixed this one.