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

Incomplete implementation of DIS 7 #14

Open
bamamarc opened this issue Dec 28, 2021 · 22 comments
Open

Incomplete implementation of DIS 7 #14

bamamarc opened this issue Dec 28, 2021 · 22 comments

Comments

@bamamarc
Copy link

The there are file(s) in the DIS 7 implementation that appear to be missing. For example; the files in the DISnet.DataStreamUtilities namespace which don't appear to exist. Can one use the similar files in the OpenDis.Core name space or is the functionality implemented that is be implemented in the DISnet.DataStreamUtilities namespace need to be created?

@leif81
Copy link
Member

leif81 commented Jan 6, 2022

@bamamarc Sorry for delay. Yes this is a rather common issue betwen the DI6 and DIS7 implementations in Open DIS. The DIS6 version is typically more tested and has more utility functions. Resolving this is something that's been bothering me for a while. I've been tempted to investigate whether the DIS7 implementation could/should become the "the" default implementation that supports both DIS6 and DIS7 protocol versions. My understanding of the spec is a DIS7 implementation should be backwards compatible in terms of receiving DIS6 PDU's and a DIS7 PDU that is received by a DIS6 implementation will just ignore the new fields. So dropping the DIS6 implementation of the PDU's and focussing all effort ont he DIS7 implementation may be the way forward to resolve this in the long term. The trick is not losing all the fixes done to the DIS6 implementation and having to repeat the fixes on the DIS7 side.

@bamamarc
Copy link
Author

bamamarc commented Jan 7, 2022

Thank for your response. In the meantime I merged the the CSharpDis7 implementation into the CSharpDis6 implementation, (now simply called CSharpDis) and created the necessary interfaces, factories, etc to use either version from a common assembly. This is not a complete solution as I only implemented the PDUs that our application generates and processes. With that disclaimer, I would be happy to share what I have done. However, If you want me to do this I will have to learn more about Git and the proper ways do this.

@leif81
Copy link
Member

leif81 commented Jan 21, 2022

I think what you've done is a good approach. There's another large pull request coming in soon, it could conflict. Once that's merged if you could rebase your work on that that'd be a good time to open a pull request for review here. Thanks

@Coz540
Copy link

Coz540 commented Nov 1, 2022

@bamamarc I was wondering if you could share those changes since the DIS7 seems to be incomplete. I had been using the DIS6 implementation previously since it had solutions I could run and test. Future work requires I use the DIS7 - IEEE 1278.1-2012 spec. @leif81 all pull requests appear to be merged in. I'm just wondering if have missed any setup to get the DIS7 implementation working?

@bamamarc
Copy link
Author

bamamarc commented Nov 1, 2022 via email

@Schreck425
Copy link

Sorry if this is a dumb question. People above are saying the the DIS 7 version is incomplete, does that mean it's not usable yet? With the current baseline I can go into CsharpDis6 to make an OpenDis.DLL, but the CSharpDis7 folder doesn't have the same options. Is there a way to build a DLL from those files to try version 7?

@alearci96
Copy link

alearci96 commented May 18, 2023

Hello everyone, this is my first post on github. I know 6 months have passed from the last post here, but I have managed to merge the DIS7 library into the DIS6 one, I have uploaded everything on a public repo on my profile:

https://github.com/alearci96/OpenDIS7

Just a few remarks:

  • I have put the DIS7 Library under the "Dis2012" library
  • I have modified the namespace and the file "PduProcessor" to include the DIS7 library too, in addition to the 1995 and 1998 library
  • I have fixed the missing DISnet.DataStreamUtilities library, linking those functions to the DIS6 equivalent library

I hope that someone will use this and will be helped from my code in the same way that the original openDIS code helped me with both my work and hobbies.
Let me know if this is useful and if I can do anything else to better it!

@Schreck425
Copy link

Very happy to see this! Trying it now. I don't fully understand all of how this works but I copied your stuff into my solution and it compiled. I had to remove your unity file but once I did that it worked great. Now I'm seeing the differences between the libraries and will have to figure out the best way of listening to the network if I have both DIS 6 and DIS7 on the network. This gets me started and I appreciate it!!!

@Schreck425
Copy link

So do 1998 and 2012 have to be treated like two totally different classes or is there a better way to use them?

I'm very glad I can now at least get the data off the network for the application I maintain. Before DIS7 packets just errored out and got tossed. Now I have these functions that were expecting the 1998 packet and I find myself not really knowing the best way to deal with 7 since they're different but still similar. I find myself starting to just convert them to DIS6 and using the information the application needs. Is this really dumb or is this kind of expected with the nature of Dis6 and 7?

This is what I just created so that I can use the DIS7 packet and still use my application functions.

private static OpenDis.Dis1998.EntityStatePdu ConvertEntTo7(OpenDis.Dis2012.EntityStatePdu dis7_ent)
{
OpenDis.Dis1998.EntityStatePdu converted6 = new OpenDis.Dis1998.EntityStatePdu();

      converted6.EntityID.Site =          dis7_ent.EntityID.SimulationAddress.Site;
      converted6.EntityID.Application =   dis7_ent.EntityID.SimulationAddress.Application;
      converted6.EntityID.Entity =        dis7_ent.EntityID.EntityNumber;

      //...  more to go,  just will copy what I want.
      return converted6;

}

@alearci96
Copy link

@Schreck425 I'm so glad this helped you! Actually, I've only built a DIS 7 Receiver since the devices I work with use only that DIS version. However, the local packages conversion is not a dumb way to manage multiple DIS versions, instead it is very common in military and non-military applications. See for example the MAK VR Exchange commercial product that literally does this (https://www.mak.com/mak-one/tools/vr-exchange). I noticed too the difference for the DIS EntityID for which you developed the function and I would not know a more intelligent way to deal with it :) Let me know if you have any updates!

@Schreck425
Copy link

@alearci96

So I noticed that Transmitter/SIgnal PDU's weren't in your baseline. I've never seen DIS7 versions of those, our systems all use 6. I've seen documentation showing the differences with 6 and 7 on them. Did you just not get to Transmitters and Signals on your version?

@alearci96
Copy link

alearci96 commented Jun 25, 2023

@Schreck425 as you can see from all the PDUs .cs files of the DIS7 library from which I started, there isn't any Signal PDU .cs, so I didn't know how to introduce them unfortunately :(. I'll let you know if I will find some info about it or if I will put hand on that! For now, I will just try to use the DIS1998 (DIS6) ones.

@agerrius
Copy link

agerrius commented Sep 7, 2023

For some of the missing PDU types, like transmitter and signal PDU, I am willing to compare the standards and figure out what the differences are. I guess we can use the DIS6 auto generated CS file as a start to make a version that is DIS7 compatible by hand. Although that would be too much work to do for all the PDU types missing I guess, but for the commonly used ones I think it would be an useful activity.

@alearci96
Copy link

@agerrius have you done anything in these months? If you have any news on the transmitter and signal PDUs I would be very grateful to know that :)

@Schreck425
Copy link

@agerrius have you done anything in these months? If you have any news on the transmitter and signal PDUs I would be very grateful to know that :)

I have a modified version locally that marshalls/unmarshalls DIS7 packets correctly. It's still a work in progress though. I've been too embarassed to share it. It works but it's sloppy and hacked. The difference between 6 and 7 sometimes is hardly anything sometimes. It was very apparent when I used both the Dis1998 namespace with a Dis2012 one to convert versions between each other. It looks like some all you would have to do is change the Protocol field and that's it because the structure is still the same.

I didn't understand the autogenerate that they used before and I kind of just blew it away and it doesn't really match theirs, but what's important is that wireshark seemed to confirm that I was fixing things that weren't completed yet for C# DIS7.

@agerrius
Copy link

agerrius commented Jan 9, 2024

@agerrius have you done anything in these months? If you have any news on the transmitter and signal PDUs I would be very grateful to know that :)

@alearci96, no unfortunately not yet, the project for which I would need this has been delayed a bit. But it is something we are going to look at soon. I had also hoped that there would be some discussion after the posts I made in September but it remained silent :)

@Schreck425, if you already have a working basis would it be an idea that we all work from that instead of reinventing the wheel by implementing something else? Did you start with the implementation @alearci96 made that has the DIS7 PDUs in the same library?

@Schreck425
Copy link

What do I need to do to push my branch up for you guys to look? Push failed.

fatal: unable to access 'https://github.com/open-dis/open-dis-csharp.git/': The requested URL returned error: 403

@leif81
Copy link
Member

leif81 commented Jan 10, 2024

@Schreck425 you'll want to fork the repository, commit your changes and then open a pull request. More detail about the process can be found here

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests

@Schreck425
Copy link

Thanks! I forked it and have a branch called Dis7 that I'm working off of. It compiles and works for me but on limited amount of types of PDU's. Our simulations don't typically require much more than what I've done so that's all I did. I just needed it to not choke on DIS7 and this works for me.

https://github.com/Schreck425/open-dis-csharp.git

What I did probably would take @leif81 a fraction of the time to do, and he would have done it better because he understands it much better, but hey this is how I learn I guess.

@leif81
Copy link
Member

leif81 commented Jan 10, 2024

@Schreck425 I'm actually not very familiar with Csharp. Inherited this project from Don McGregor who passed away. So any suggested changes in the form of pull requests are very much appreciated.

@agerrius
Copy link

@Schreck425 I finally found some time to have a look at your fork. In general I was able to using it in our application and process DIS 7 messages now as well. I had to make some changes to the PduProcessor and PduFactory classes to make sure they support the DIS 7 PDUs as well.

Also I found out that the SetData and Data PDUs store the VariableDatum with the wrong data type (uint instead of EightByteChunk). I just took the VariableDatum class from the DIS 6 code and that seems to work fine. Not sure if this is the right approach, as the file seems to be autogenerated, but I don't know how that generation works and why for DIS7 it is using a different datatype.

I haven't taken a good look at the EE PDU yet, I noticed there is a copy of that class in the code which gave me some compilation errors at first.

I'll share my modifications once I am done with testing it a bit more.

@agerrius
Copy link

I just went through the different standards and checked which PDUs are availble in OpenDIS for the different versions. I made an XLS file with the overview. Green cells mean present (I did not check all fields are present in that case), red cells are missing from OpenDIS.

It indeed seems some are missing in DIS7, while DIS6 has them. But I guess for most applications these are not the most necessary ones.

opendis_pdu_comparison.xlsx

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

6 participants