search

WorldWindEarth / WorldWindJava

A community supported fork of the NASA WorldWind Java SDK (WWJ) is for building cross-platform 3D geospatial desktop applications in Java.
https://worldwind.earth/WorldWindJava/
47 stars 13 forks source link

MGRS Conversion fix #72

Closed gbburkhardt closed 5 years ago

gbburkhardt commented 5 years ago

Description of the Change

Fix two problems:

a) Index out of b typo in convertMGRSToUPS; clearly the index should have been 1 instead of 12.ounds error due to b) Conversions from geodetic to MGRS would fail for low southern latitudes (zone 0). Geotrans 3.7 has this problem fixed. The WorldWind conversion code was derived from NGA's Geotrans. Test case: Lat: -89.345400 deg, Lon: -48.930600 deg ==> MGRS: AZN 45208 47747

Why Should This Be In Core?

Correct conversion from geodetic to MGRS

Benefits

Potential Drawbacks

None

Applicable Issues

Issue #67.

wcmatthysen commented 5 years ago

@gbburkhardt, I made some small fixes. The UTM_MGRS_test class should have been in the test folder where Gradle can pick it up during the unit-test phase. I also renamed it to CoordTest as we can add tests for all coordinate conversions there in the future. The builds failed because JUnit is only a test-dependency and if you add a test class to the src folder (instead of the test folder) it won't pick up the JUnit classes when compiling. If you want to see why a build fails you can click the red-cross that appears next to the commit and go to the details link where it will show you the different Travis builds. You can then select one of them to see the logs to determine the reason for the build-failure.

ComBatVision commented 5 years ago

Hello, guys! Could you please do not forgot to make the same changes in my Android port when you finished. I do not want to lost convertor code-base synchronization between Java and Android. It will be harder for me to dive deep in to current issue context then you. Thanks. https://github.com/WorldWindEarth/WorldWindAndroid/pull/16

wcmatthysen commented 5 years ago

@Sufaev, I can help with the port of these fixes to the Android code-base.

ComBatVision commented 5 years ago

@wcmatthysen I have unreleased PR 16 in Android repository, which contain ported coordinate converters and graticules from Java. It will be perfect to add commit with all changes related to latest MGRS fixes to the same PR.

By the way, I do not hear @emxsys for a long time. That's why PRs are still unreleased. Is he ok?

wcmatthysen commented 5 years ago

@Sufaev, I'll have a look at that pull-request once we are done merging this one.

I don't know. I haven't heard from him in a while.

gbburkhardt commented 5 years ago

Well, as you can see, I still don't like the verbose coding style w.r.t. curly brackets. 😉 But I see the merit in keeping the style in the code base uniform. I'm fine with the changes you made. Merge when ready, as far as I'm concerned.

gbburkhardt commented 5 years ago

@wcmatthysen : I notice that you added a NASA copyright to the MGRS unit test file. Is that appropriate? NASA didn't write that code. Can a copyright be assigned to an entity without its knowledge or permission? I'm not worried, just curious.

wcmatthysen commented 5 years ago

@gbburkhardt, I also don't like the curly braces on a separate line when it comes to styling. I think it is just important to keep the coding-style consistent with respect to the rest of the code-base. It might be something that we can discus in future depending on what happens to the upstream repository. If they suddenly start to contribute changes again then we might have to stick with the coding-style as it will be easier to pull in changes from them.

I think we need to add the copyright at the top of each new file as the entire code-base is licensed under the NASA open-source license agreement. It doesn't matter if a NASA developer wrote the code or not. As contributors we agree to adhere to the license when we make contributions.

gbburkhardt commented 5 years ago

Just teasing about the coding style.  I strongly agree that only one style should be used, otherwise the code gets really unreadable.  I'll try to be a better boy in the future....

On 8/14/2019 9:20 AM, Wiehann Matthysen wrote:

@gbburkhardt https://github.com/gbburkhardt, I also don't like the curly braces on a separate line when it comes to styling. I think it is just important to keep the coding-style consistent with respect to the rest of the code-base. It might be something that we can discus in future depending on what happens to the upstream repository. If they suddenly start to contribute changes again then we might have to stick with the coding-style as it will be easier to pull in changes from them.

I think we need to add the copyright at the top of each new file as the entire code-base is licensed under the NASA open-source license agreement. It doesn't matter if a NASA developer wrote the code or not. As contributors we agree to adhere to the license when we make contributions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/WorldWindEarth/WorldWindJava/pull/72?email_source=notifications&email_token=ABI6HFF4ZWG2JKBO4HKJXV3QEQBBBA5CNFSM4ILQ6LJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4IYPXY#issuecomment-521242591, or mute the thread https://github.com/notifications/unsubscribe-auth/ABI6HFFGUHMTZNJJSY4MJ3TQEQBBBANCNFSM4ILQ6LJA.

This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus

wcmatthysen commented 5 years ago

@gbburkhardt, no worries. I am more worried about changing the coding-style and then the upstream repository suddenly becomes active again (though that looks doubtful at this point). If the upstream developers add changes and we need to pull those changes through to our repository here then we'll have a hard-time sorting through the changes if we have a lot of changes on our side related to code formatting. So, at this point, keeping the coding style as-is is more a case of us keeping as much in sync with the upstream repository as possible. Maybe down the line if we see that the upstream repository is truly dead we can consider switching to a more compact coding style. We'll have to wait and see what happens.