open-dis-java icon indicating copy to clipboard operation
open-dis-java copied to clipboard

DIS7 Signal PDU data length is in bytes but should be in bits

Open jdchn opened this issue 7 years ago • 2 comments

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 avatar Aug 30 '18 07:08 jdchn

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:

jdchn avatar Aug 31 '18 18:08 jdchn

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!

leif81 avatar Aug 31 '18 19:08 leif81