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

AggregateState PDU updated #93

Merged
merged 5 commits into from
Dec 8, 2022
Merged

AggregateState PDU updated #93

merged 5 commits into from
Dec 8, 2022

Conversation

fo-ifad
Copy link
Contributor

@fo-ifad fo-ifad commented Dec 2, 2022

added classes to represent Silent Aggregate -and Entity Systems , used these in Aggregate state PDU and changed the "pad2" attribute to be dependent on how many Aggregate and Entity ID are in PDU lists, to fit IEEE Std 1278.1a-1998 section 5.3.9.1

fo-ifad and others added 2 commits December 2, 2022 11:23
… these in Aggregate state pdu and changed the pad2 to be dependendant on how many Aggregate and Entity ID are in pdu to fit IEEE Std 1278.1a-1998 section 5.3.9.1
@leif81
Copy link
Member

leif81 commented Dec 2, 2022

Thank-you @fo-ifad

If possible, could you create a unit test for the AggregateSystem PDU class and at it to this PR?

For an example, see the unit test named EntityStatePduTest. https://github.com/open-dis/open-dis-java/blob/master/src/test/java/edu/nps/moves/dis/EntityStatePduTest.java This was done by capturing a PDU with WireShark (not sent by Open DIS), and then using the unmarshal and marshal methods of the class

@leif81
Copy link
Member

leif81 commented Dec 8, 2022

@fo-ifad great job on the unit test.

I overlooked something earlier because I am not very familar with the Aggregate State PDU. If I'm understanding it this is a new PDU added in DIS 7? If so, can you move the classes into the edu.nps.moves.dis7 package here. The dis package is DIS 6.

@fo-ifad
Copy link
Contributor Author

fo-ifad commented Dec 8, 2022

Thanks

The Aggregate State PDU was introduced in IEEE 1278.1a-1998, so I think it is placed correctly in the dis package, and I think the Aggregate State PDU is a little different in DIS7, I haven't really made a deep dive into that yet.

Version 5 - IEEE 1278.1-1995
Version 6 - IEEE 1278.1a-1998 (amendment to IEEE 1278.1-1995)
Version 7 - IEEE 1278.1-2012 (See External Link - DIS Product Development Group.) Version 7 is also called DIS 7.

@leif81 leif81 merged commit cff90d1 into open-dis:master Dec 8, 2022
@leif81 leif81 requested review from leif81 and removed request for leif81 December 9, 2022 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants