Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with Data Length #84

Open
crhowell3 opened this issue Apr 27, 2023 · 2 comments
Open

Issue with Data Length #84

crhowell3 opened this issue Apr 27, 2023 · 2 comments

Comments

@crhowell3
Copy link
Contributor

void SignalPdu::marshal(DataStream &dataStream) const {
  RadioCommunicationsFamilyPdu::marshal(
      dataStream); // Marshal information in superclass first
  dataStream << _encodingScheme;
  dataStream << _tdlType;
  dataStream << _sampleRate;
  dataStream << (short)_data.size();  // ISSUE
  dataStream << _samples;
  for (auto byte : _data) {
    dataStream << byte;
  }
}

There is an issue related to the PDUs that inherit from the RadioCommunicationsFamilyPdu (the IntercomSignalPdu and SignalPdu classes) where the size of the data being marshaled into the datastream is incorrect and not compliant with the DIS Standard. The data length should be the size of the data in bits (as per the SISO-STD-001-2015 standard), whereas the size being marshaled in currently is in bytes. I will open a PR to fix this issue.

It is possible that this issue is present elsewhere in the code, so I will do a once-over before requesting merge.

@leif81
Copy link
Member

leif81 commented Apr 27, 2023

@crhowell3 I believe you are correct about it being a bug in SignalPDU. We saw the same problem in the Java implementation and fixed it. Note that I think this has been logged before against the CPP implementation, see this issue #10 for more info and how it was fixed in Java. A PR would be very welcome

@leif81
Copy link
Member

leif81 commented Jul 31, 2023

I was going to mark as a duplicate of #10, but this one mentions the problem affects not only Signal Pdu but also Intercom Signal Pdu, so I will leave this as a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants