finish up Walls import - Githubissues

jedwards1211 commented 8 years ago

These are checklists for myself:

[x] add back ability to import .srv files directly
[x] make it possible to pass in Walls for all instances of Survex shown on screen in cwImportSurvexDialog (haven't completely figured out how to work within the constraints of the form designer yet)
[x] use .NAME directives in .wpj files for trip titles
[x] refactor to disable cwSurvexGlobalData's station renaming for Walls import only

Check that the following work properly (ideally, with unit tests):

[x] .PATH directives in .wpj files
[x] calibrations/team members
[x] #units save and #units restore
[x] instrument/target heights and inch parsing
[x] deriving distance and inclination from inst/target heights, inch correction, and taping methods
[x] cd-like behavior of #segment directives

Reverse-engineered spec:
- .. and . can only come at beginning of path
- / and \ are treated identically
- path elements may not begin with . (except initial .. and . elements)
- path elements may not contain /, \, or ;
- trailing whitespace is removed from last path element FIX THIS -- parser currently trims on both sides
- if the path begins with / or \ it resets to the name-defined segment from the .wpj file
- .. beyond root does nothing
[x] incs correction of lruds/ih/th

Double check if Walls supports:

[x] ; comments at the end of lines in .wpj files -- nope
[x] the same station name restrictions as dewalls does Nice. Walls gives invalid errors when TO stations start with < (which is ambiguous with LRUDs)
[x] too many .ENDBOOK directives -- ignores the rest of the file
[x] too many ../ in #segment directives yup
[x] absolute .PATHs of any kind -- yup, support implemented

Process the following if necessary:

[x] preserve length/orientation of vertical shots flags -- Walls adjusts variance according to these, unless survex supports variance overrides we have no use for these
[x] name defines segment flag
[x] derive declinations from #date flag (will defer this)
[x] UTM/GPS grid relative flag -- not necessary, this is for Walls output coordinates only
[x] grid convergence setting from .wpj files (same as #units grid=...?) just affects Walls output coordinates and maps. Will be important for importing Walls calculated coordinates though!

Display warnings when:

[x] #fix stations are present
[x] stations have to get renamed
[x] any entries have derive decl from date checked
[x] move these warnings to the import warnings tab

jlillest commented 8 years ago

nice!

vpicaver commented 8 years ago

Sweet!

I didn't know you could make checkboxes in github, that's pretty cool.

jedwards1211 commented 8 years ago

Yeah, that way we don't have to make a bajillion tiny issues for things

jedwards1211 commented 8 years ago

@vpicaver do you know what library you want to use for computing declination automatically?

vpicaver commented 8 years ago

Haven't researched it much. Do you have one in mind?

On Saturday, December 5, 2015, Andy Edwards notifications@github.com wrote:

@vpicaver https://github.com/vpicaver do you know what library you want to use for computing declination automatically?

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162244131.

jlillest commented 8 years ago

In case it helps, here's the NOAA page for info on third-party software for the World Magnetic Model(WMM): http://www.ngdc.noaa.gov/geomag/WMM/thirdpartycontributions.shtml

vpicaver commented 8 years ago

http://geographiclib.sourceforge.net/html/classGeographicLib_1_1MagneticModel.html

This might be some that could work. We would need to create a qbs build of it so it integrates with our qbs / sub module build system. Currently it uses cmake.

On Saturday, December 5, 2015, jl <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

In case it helps, here's the NOAA page for info on third-party software for the World Magnetic Model(WMM): http://www.ngdc.noaa.gov/geomag/WMM/thirdpartycontributions.shtml

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162248695.

jedwards1211 commented 8 years ago

Okay, cool. I love how many good libs like that are available for C++. Hopefully I'll get to that soon...just did a massive code cleanup today

vpicaver commented 8 years ago

Yea it seems promising, I'm sure there is others out there.

On Sunday, December 6, 2015, Andy Edwards notifications@github.com wrote:

Okay, cool. I love how many good libs like that are available for C++. Hopefully I'll get to that soon...just did a massive code cleanup today

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162382442.

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Any tips on how I can make my Length and Units singletons thread-safe? They're getting initialized twice in a test...

vpicaver commented 8 years ago

So it's calling isNull() returning true both times?

On Sunday, December 6, 2015, Andy Edwards notifications@github.com wrote:

Any tips on how I can make my Length and Units singletons thread-safe? They're getting initialized twice in a test...

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162386600.

Phi|ip

Sent from Gmail Mobile

vpicaver commented 8 years ago

Oh don't need to use a shared ptr for _type in length.h. Just make _type a normal pointer and initialize it in define it statically in length.cpp

Const Length* Length::_type = new Length();

You need to make _type Const in the header as well. Then you won't need an init() function because it will be initialized when the static section is created when the program starts up.

I don't have my computer until Tuesday so I can test or try stuff out.

On Sunday, December 6, 2015, Philip Schuchardt vpicaver@gmail.com wrote:

So it's calling isNull() returning true both times?

On Sunday, December 6, 2015, Andy Edwards <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Any tips on how I can make my Length and Units singletons thread-safe? They're getting initialized twice in a test...

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162386600 .

Phi|ip

Sent from Gmail Mobile

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Actually, I just found out about Q_GLOBAL_STATIC, I'm going to try it out...

vpicaver commented 8 years ago

Interesting, didn't know about. Looks useful

On Sunday, December 6, 2015, Andy Edwards notifications@github.com wrote:

Actually, I just found out about Q_GLOBAL_STATIC, I'm going to try it out...

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162401738.

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Weird. Even when I used Q_GLOBAL_STATIC my Length constructor gets called twice (according to print statements I added) when running the tests.

Another weird thing is that when I try to run the tests in debug mode, I can't get it to break or print anything, but when I run the main app in Debug mode, it works just fine. I wonder if Catch has something to do with that.

jedwards1211 commented 8 years ago

Ah, problem with debugging in test mode was that I had run in console checked. When I unchecked it and it ran in the IDE console, I got it to break :)

jedwards1211 commented 8 years ago

Agh. The double-loading is happening because one access to the singleton is triggered by other statics. Singletons are so difficult in C++...people are so critical of them, but this really is a logical use case

jedwards1211 commented 8 years ago

I think if I load some global constants in WallsParser and CardinalDirection with a Q_GLOBAL_STATIC as well, the problem will resolve itself

balister commented 8 years ago

Hippies.

On 12/06/2015 06:40 PM, Andy Edwards wrote:

I think if I load some global constants in WallsParser and CardinalDirection with a Q_GLOBAL_STATIC as well, the problem will resolve itself

Reply to this email directly or view it on GitHub: https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162411118

jedwards1211 commented 8 years ago

The way I was doing units was so elegant in Java. I guess I'll have to do something different in this code

jedwards1211 commented 8 years ago

Alright! I refactored the units stuff to get rid of the singletons. At the time I wrote them, I wasn't familiar enough with C++ templates to know how to use them in this case

vpicaver commented 8 years ago

Cool I'm glad you figured it out

On Monday, December 7, 2015, Andy Edwards notifications@github.com wrote:

Alright! I refactored the units stuff to get rid of the singletons. At the time I wrote them, I wasn't familiar enough with C++ templates to know how to use them in this case

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162416684.

Phi|ip

Sent from Gmail Mobile

vpicaver commented 8 years ago

Isn't there a singleton keyword in Java?

On Monday, December 7, 2015, Philip Schuchardt vpicaver@gmail.com wrote:

Cool I'm glad you figured it out

On Monday, December 7, 2015, Andy Edwards <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Alright! I refactored the units stuff to get rid of the singletons. At the time I wrote them, I wasn't familiar enough with C++ templates to know how to use them in this case

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162416684 .

Phi|ip

Sent from Gmail Mobile

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Nah, but there are some pretty simple ways to make them, and there's no load-ordering problem with static member variables, the VM makes sure they load in the proper order if one references another (even if they're in different classes)

jedwards1211 commented 8 years ago

On the other hand, C++ templates are awesome and proved to be a totally better solution for how I'm using units

jedwards1211 commented 8 years ago

Here's a singleton for you guys, right in the middle of GeographicLib: http://geographiclib.sourceforge.net/html/classGeographicLib_1_1Geocentric.html#af6e84e6d9d7967df6ba13cae1814cb7b

jedwards1211 commented 8 years ago

Hmmm, calculating declination automatically from data imported from Walls will take quite a bit of work, because Walls supports a pretty wide variety of reference datums. Would you guys rather wait for me to implement all of that before a release, or have me just show import warnings if the walls data is set up to calculate declination automatically, and fully implement that later?

jlillest commented 8 years ago

Walls is setup to calculate declinations from geo reference automatically, so I suspect not having a static declination in project files will be the norm. That being said, I think pushing for an intermediate release where declinations are pegged at zero when not specified otherwise is fine. Then the datums can be implemented one at a time, I suppose. WGS84/NAD83 would be the first one to do, followed by NAD27 to get the majority of US users.

jlillest commented 8 years ago

If it were me, I'd add an import warning that says something like: "Datum not currently supported, please file a feature request on github". :D

jedwards1211 commented 8 years ago

Yeah, I'm fine with that. Phi|ip?

vpicaver commented 8 years ago

Warning is fine for now. 😀

On Monday, December 7, 2015, Andy Edwards notifications@github.com wrote:

Yeah, I'm fine with that. Phi|ip?

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162751781.

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Cool.

What's the equate map stuff for? I discovered it adding x's to the end of my stations unnecessarily (for Walls import, at least), so I'll have to do some kind of refactoring to turn off that behavior for Walls import. Perhaps extract a base class from cwSurvexGlobalData?

jedwards1211 commented 8 years ago

The new warnings/errors in the data pages are really neat though!

vpicaver commented 8 years ago

The equate stuff is for survex imports. To connect stations between trips you must export and equate them. You can also alias stations using them.

Check out the survex documentation: http://survex.com/docs/manual/datafile.htm

On Tuesday, December 8, 2015, Andy Edwards notifications@github.com wrote:

Cool.

What's the equate map stuff for? I discovered it adding x's to the end of my stations unnecessarily (for Walls import, at least), so I'll have to do some kind of refactoring to turn off that feature for Walls import. Perhaps extract a base class from cwSurvexGlobalData?

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-162802321.

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Oh, weird. No way to mark the trip blocks as not adding a prefix to the station names within them, huh? In Walls it's more explicit, on one hand there are these #segments that can follow the tree structure, but can be overridden, and those don't affect uniqueness (and they serve no purpose for import to Cavewhere right now anyway) -- only prefixes, of which there can be 3 and manually set only, affect uniqueness

jedwards1211 commented 8 years ago

Sweet, just got the refactoring done, wasn't too hard

jedwards1211 commented 8 years ago

It was:

rename cwSurvexImportDialog => cwImportTreeDataDialog
rename cwSurvexBlockData => cwTreeImportDataNode
extract a base class from cwSurvexGlobalData named cwTreeImportData, and make its caves() a pure virtual function
create cwWallsImportData that extends cwTreeImportData
use cwSurvexGlobalData in cwSurvexImporter and cwWallsImportData in cwWallsImporter

cwTreeImportDataNode still contains some Survex import-specific things. Do you want me to clean that up too? That would involve making it a base class and having cwSurvexBlockData extend it.

vpicaver commented 8 years ago

That's a good question. How long would it take to make cwSurvexBlockData as a sub class? If you think it'll be easier to understand, do it. :D

Phi|ip

On Fri, Dec 11, 2015 at 6:55 PM, Andy Edwards notifications@github.com wrote:

It was:

rename cwSurvexImportDialog => cwImportTreeDataDialog

rename cwSurvexBlockData => cwTreeImportDataNode

extract a base class from cwSurvexGlobalData named cwTreeImportData, and make its caves() a pure virtual function

create cwWallsImportData that extends cwTreeImportData

use cwSurvexGlobalData in cwSurvexImporter and cwWallsImportData in cwWallsImporter

cwTreeImportDataNode still contains some Survex import-specific things. Do you want me to clean that up too? That would involve making it a base class and having cwSurvexBlockData extend it.

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-164083699.

jedwards1211 commented 8 years ago

probably wouldn't take too long, but I may or may not finish stuff up this weekend. I'm really freaking close though :)

vpicaver commented 8 years ago

No worries. Going caving :D

On Friday, December 11, 2015, Andy Edwards notifications@github.com wrote:

probably wouldn't take too long, but I may or may not finish stuff up this weekend. I'm really freaking close though :)

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-164088348.

Phi|ip

Sent from Gmail Mobile

vpicaver commented 8 years ago

If you get it done next week, we can probably release a new version.

Phi|ip

On Fri, Dec 11, 2015 at 7:33 PM, Philip Schuchardt vpicaver@gmail.com wrote:

No worries. Going caving :D

On Friday, December 11, 2015, Andy Edwards notifications@github.com wrote:

probably wouldn't take too long, but I may or may not finish stuff up this weekend. I'm really freaking close though :)

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-164088348 .

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

actually, using inheritance in trees of interrelated types has always been awkward...it would be pretty nice in JS for example, I could easily tack the survex-specific data onto the tree nodes and process it later. Any idea of a good C++ way to do that using a QMap<QString, QVariant> or something like it?

jedwards1211 commented 8 years ago

actually, what am I thinking...I can just do

template<typename T>
class cwTreeImportDataNode {
  ...
  T UserData;
}

jedwards1211 commented 8 years ago

wait a minute, no I can't...damn non-generic templates...

jedwards1211 commented 8 years ago

I ended up just using a QHash<cwTreeImportDataNode*, cwSurvexNodeData*> in cwSurvexBlockData

jedwards1211 commented 8 years ago

God damnit VC++...

C:\Users\andy\build-cavewhere-Desktop_Qt_5_5_1_MSVC2013_64bit-Release\qtc_Desktop73601786-release\cavewhere-test.qtc-Desktop--73601786.6720c1df\cwWallsImporterTest.cpp.obj:-1: error: LNK2019: unresolved external symbol "public: static void cdecl cwWallsImporter::importCalibrations(class dewalls::WallsUnits,class cwTrip &)" (?importCalibrations@cwWallsImporter@@SAXVWallsUnits@dewalls@@AEAVcwTrip@@@Z) referenced in function "void cdecl __C_A_T_C_HT_E_S_T12(void)" (?C_A_T_C_H____T_E_S_T12@@YAXXZ)

I added a missing const to the cpp file, but it's still giving me the linking error. I don't see what I could possibly be doing wrong

vpicaver commented 8 years ago

Is cwWallsImporterTest in a dynamic library? On windows you have to export the class symbols if you're using in another place, like the testcases. There should be other examples inside of the current master like cwCave.

Phi|ip

On Sun, Dec 13, 2015 at 8:22 PM, Andy Edwards notifications@github.com wrote:

God damnit VC++...

C:\Users\andy\build-cavewhere-Desktop_Qt_5_5_1_MSVC2013_64bit-Release\qtc_Desktop73601786-release\cavewhere-test.qtc-Desktop--73601786.6720c1df\cwWallsImporterTest.cpp.obj[image: :-1:] error: LNK2019: unresolved external symbol "public: static void cdecl cwWallsImporter::importCalibrations(class dewalls::WallsUnits,class cwTrip &)" (?importCalibrations@cwWallsImporter@@SAXVWallsUnits@dewalls @@AEAVcwTrip@@@Z https://github.com/Z) referenced in function "void cdecl __C_A_T_C_HT_E_S_T12(void)" (?C_A_T_C_H____T_E_S_T12@@YAXXZ)

I added a missing const to the cpp file, but it's still giving me the linking error. I don't see what I could possibly be doing wrong

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-164318170.

jedwards1211 commented 8 years ago

Ah, cool, thanks, I don't think I ever would have figured that out!

vpicaver commented 8 years ago

Yea the joys of Windows....

On Monday, December 14, 2015, Andy Edwards notifications@github.com wrote:

Ah, cool, thanks, I don't think I ever would have figured that out!

— Reply to this email directly or view it on GitHub https://github.com/Cavewhere/cavewhere/issues/90#issuecomment-164476415.

Phi|ip

Sent from Gmail Mobile

jedwards1211 commented 8 years ago

Any idea how I can dynamically change the text of the Survex Errors label? If so you can get points on my SO question: [[http://stackoverflow.com/questions/34298208/dynamically-change-text-of-tab-created-using-qt-designer]]

Cavewhere / cavewhere

finish up Walls import #90