DFDLSchemas / NITF

NITF - MIL-STD-2500C - National Imagery Transmission Format
1 stars 5 forks source link

ImageGeographicLocation #6

Closed deanlinsalata closed 4 years ago

deanlinsalata commented 4 years ago

I found a memory leak ( I guess you might call it that ) when a ImageCoordinateRepresentation value is either MilitaryGridReferenceSystem, NorthernHemisphere or SouthernHemisphere. When the ImageGeographicLocation is built, for these systems, it is using 120 bytes of data ( 30 for each corner ) instead of the allotted 60 for a ImageGeographicLocation field. So eventually it will break an upcoming field, in my case NumberOfBands.

Really great job on the whole schema, I know it is hard to test all the different combinations of NITF values.

Thanks Dean

stevedlawrence commented 4 years ago

Agreed, those three are definitely wrong. I'm not too familiar with NITF. Do you know if there is a reasonable way to split up the coordinates, similar to how DecimalDegrees is split up into Lat/Long, or does it just make sense for the fields to be 15 character strings?

deanlinsalata commented 4 years ago

I am not a mapping expert but I followed the guidelines on page 85 of this document: https://gwg.nga.mil/ntb/baseline/docs/2500c/2500C.pdf and came up with a suggestion on how the ImageGeographicLocation element can be formatted. I attached the xml element.

ImageGeographicLocation.txt

stevedlawrence commented 4 years ago

Seems like a reasonable change to me.

If you create a pull request that makes this change and updates the tests run via sbt test (and maybe adds new tests for the MGRS and Hemisphere coords too?) I'd be happy to merge that change in. I might also suggest that you wrap the Degrees/Minutes/Seconds/Direction for Geogrphic coords in a Latitude and Longitude complex type. That way if someone wants Degrees from the Latitude vs Degrees form the Longitude they can access them specifically.

deanlinsalata commented 4 years ago

OK. Give me some time, maybe by the weekend, to get familiar with how you test and the test data you have, I would like to build the tests too.

stevedlawrence commented 4 years ago

Sounds good. If you have any questions let me know.

deanlinsalata commented 4 years ago

I cloned this repository to my local machine and made the edit to the nitf.dfdl.xsd that I suggested above along with wrapping Geographic coords. I also created a test for each geo system. I'm not sure if I created the tests in the right location but they are all based off of test i_3034 with small changes for the coordinate systems. I am not a github expert so I am not sure how to proceed to check things in.

modified:   src/main/resources/com/tresys/nitf/xsd/nitf.dfdl.xsd

Untracked files: (use "git add ..." to include in what will be committed)

src/test/resources/ntb-baseline/i_3034f_decimal.ntf
src/test/resources/ntb-baseline/i_3034f_decimal.ntf.xml
src/test/resources/ntb-baseline/i_3034f_geographic.ntf
src/test/resources/ntb-baseline/i_3034f_geographic.ntf.xml
src/test/resources/ntb-baseline/i_3034f_military.ntf
src/test/resources/ntb-baseline/i_3034f_military.ntf.xml
src/test/resources/ntb-baseline/i_3034f_utm_north.ntf
src/test/resources/ntb-baseline/i_3034f_utm_north.ntf.xml
src/test/resources/ntb-baseline/i_3034f_utm_south.ntf
src/test/resources/ntb-baseline/i_3034f_utm_south.ntf.xml
stevedlawrence commented 4 years ago

You'll also want to modify these files:

src/test/resources/NTBBaseline.tdml
src/test/scala/com/tresys/nitf/TestNTBBaseline.scala

Once you add your tests to those files, you can run sbt test and it will run your tests.

To get these changes checked in, you'll need to create a "Pull Request". There's probably various pages that desribe the process in more detail, but the steps are essentially this:

  1. In the stop of this page, click the "Fork" button to create a fork of this repository. Your fork will be at https://github.com/deanlinsalata/NITF
  2. Add that fork as a remote to your currently checked out repo with: git remote add deanlinsalata git@github.com:deanlinsalata/NITF.git
  3. Commit your changes: git add . git commit
  4. Push your changes to your fork: git push deanlinsalata master
  5. Go to your fork at https://github.com/deanlinsalata/NITF and click the "create pull request" button. That will alert me and i can review the changes and merge it in.
stevedlawrence commented 4 years ago

Closing. Fixed by commit a84a510597eabc897f9b458de19b7632defddffa