open-dis / open-dis-javascript

Javascript implementation of the IEEE-1278.1 Distributed Interactive Simulation (DIS) application protocol v6 and v7
BSD 2-Clause "Simplified" License
11 stars 12 forks source link

Handling of DataPDUs #2

Closed keckxde closed 5 years ago

keckxde commented 5 years ago

Hi, first - thank you very much for this nice little javascript open-dis implementation. It really comes handy and simple to use.

I went into some problems using the DataPDU implementation.

Problem 1 - easy to solve

the dis.DataPdu.prototype.encodeToBinary Function has a problem with fixed and variableDatums. In the implementation the "this" is missing when calling the encodeBinary Function of the array elements for fixed & variable Datums The following version works for me:

       for(var idx = 0; idx < this.fixedDatums.length; idx++)
       {
          this.fixedDatums[idx].encodeToBinary(outputStream);
       }

       for(var idx = 0; idx < this.variableDatums.length; idx++)
       {
          this.variableDatums[idx].encodeToBinary(outputStream);
       } 

Problem 2 - decoding VariableDatumRecords

The default code of dis.DataPdu.prototype.initFromBinary crashes with my installation when I try to decode DataPdus with VariableDatums. I need to investigate that. for the moment I just put a try/catch around, so I can work with Fixed Datums, but this needs some more investigation.

Problem 3 - starting with node-open-dis

I had some troubles finding an entry point for the usage with the library. At the end I grabbed some JAVA examples and rewrote them to use the javascript version. To make it more user friendly, I would suggest to put some demo implementation including some UDP Packet and also the Buffer-Handling into the README. That would help other potential users, that struggle find the right entrypoint. If you like, I could push the example applications I did for sending and receiving DIS packets with node-js

Thomas

leif81 commented 5 years ago

Hi @keckxde , thank-you for sharing your experiences.

Would you be interested in submitting a pull request for any or all of these problems you encountered? They'd be appreciated.

keckxde commented 5 years ago

Hi @leif81 ,

sure, I will do so. I wanted to wait, until I had some time for testing.

For problem 1, I made that manually for all occurances in a fork, I was not sure if you create that automatically, as the error was very "programmatic" in encodeToBinary-Functions when arrays are used. See here: https://github.com/keckxde/open-dis-javascript/commit/0fd799e736fd436bfe14bdb9d45db6abcbc90fc5?diff=split

For Problem 2, I found the solution in the DIS specification. The length for the variable data Fields is specified in bits, not bytes - I solved it in a fork (at least for DataPDUs) , and will provide you with the pull requests.

For Problem 3 - examples, I created some very simple examples for sending and receiving DIS packets with nodejs. Feel free to copy or link them if you like. https://github.com/keckxde/node-disnetworkclient

Again, thank you very much for your work with the package itself. I plan to use and integrate some of the PDUs, that are currently not yet active in the factory - like the ones for Intercom&Radio.

Thomas

leif81 commented 5 years ago

Thank-you @keckxde . This is very helpful!

I've merged your PR in #4. That takes care of 1 and 2. And for 3 I've added a link in the README.md here to your repository.

Take a look at issue #3 , there's some code that I need to merge in here that may be of interest to you if you're interested in Intercom & Radio.