open-dis/open-dis-java

Signal PDU data length is in bytes but should be in bits (DIS 7)

jdchn opened this issue · 2 comments

jdchn commented

Per IEEE 1278.1 (5.8.4.2 in the draft available at http://movesinstitute.org), the Signal PDU data length should be in bits.

edu.nps.moves.dis.SignalPDU is consistent with the standard. However, the data length setter is inconsistent with the preceding comment and could result in a malformed PDU.

edu.nps.moves.dis7.SignalPDU is not consistent with the standard. This is a subtle bug because marshalling and unmarshalling will work "correctly" if all simulation applications are using Open DIS and DIS7 Signal PDUs.

In addition, according 5.8.4.3.2, the data length should be the number of valid bits (i.e. excluding padding). Both edu.nps.moves.dis.SignalPDU and edu.nps.moves.dis7.SignalPDU are not consistent with this specification as well as the padding requirement:

  • edu.nps.moves.dis.SignalPDU may lose up to 7 bits of data due to integer division
  • edu.nps.moves.dis7.SignalPDU may erroneously include up to 7 random bits of data if the caller does not explicitly zero-pad or mask the final octet
jdchn commented

The "bytes instead of bits" issue also affects edu.nps.moves.dis.IntercomSignalPDU and edu.nps.moves.dis7.IntercomSignalPDU .

As well as the C++ versions:

Hi @jdchn thank you for the report.

What you're saying sounds right. I recall fixing the DIS 6 Signal PDU a while back.

The DIS 7 part of the code base doesn't get quite as much love or testing as the DIS 6 part.

Pull requests are welcome and appreciated!