Re: [Gpsbabel-code] testo
Brought to you by:
robertl
From: Robert L. <rob...@gp...> - 2013-07-18 03:48:42
|
This is kind of applying pain killers to code that should be amputated, but for discussion's sake, does this bring Ozi to a happy place? Index: waypt.cc =================================================================== --- waypt.cc (revision 4453) +++ waypt.cc (working copy) @@ -535,11 +535,16 @@ double waypt_time(const waypoint *wpt) { +#if 0 if (wpt->creation_time <= 0) { return (double) 0; } else { return ((double)wpt->creation_time + ((double)wpt->microseconds / 1000000)); } +#else + return wpt->creation_time.toTime_t() + wpt->creation_time.time().msec() / 1000.0; + +#endif } /* That eliminates an implicit time_t conversion in a code path that Ozi rather uniquely uses. It's still technically wrong for the position filter and the waypt_speed brothers, but it at least lets us know that we're on the right path with Ozi. If that change makes things better, I'll figure something appropriately brilliant out that balances success and torture. On Wed, Jul 17, 2013 at 10:27 PM, Robert Lipe <rob...@gp...>wrote: > > > > On Wed, Jul 17, 2013 at 10:18 PM, tsteven4 <tst...@gm...> wrote: > >> r4453 fixed gpx on cygwin. >> > > Awesome. It's almost like we know what we're doing. :-) > > >> down to >> bushnell >> cetus >> gtm >> ozi >> raymarine >> > > Of those, I'd rank them Ozi, Bushnell, ...approaching epsilon. > > Because it's easy to try, does adding a ".0" in the obvious place in > > gbfputint32(wpt->latitude * 10000000, file_out); > gbfputint32(wpt->longitude * 10000000, file_out); > > to force these to floats in bushnell make a difference? > > gbfputint32(wpt->latitude * 10000000*.0*, file_out); > gbfputint32(wpt->longitude * 10000000.*0,* file_out); > > If the difference is numeric instability in Ozi times, does > waypointp->SetCreationTime((ozi_time - DAYS_SINCE_1990) * > SECONDS_PER_DAY, > 1000000 * (ozi_time - (int) ozi_time)); > > to > waypointp->SetCreationTime((ozi_time - DAYS_SINCE_1990) * > SECONDS_PER_DAY, > 1000000*.0* * (ozi_time - (int) ozi_time)); > > help? > > > Thanx for the hand, Steve. > > RJL > > > > > >> >> ERROR binary comparing ./reference/bushnell.wpt >> /tmp/gpsbabel.5288/bushnell-0.wpt >> ERROR comparing ./reference/track/cetus-track~gpx.gpx >> /tmp/gpsbabel.5288/cetus-track~gpx.gpx >> ERROR binary comparing /tmp/gpsbabel.5288/gtm.gtm ./reference/sample.gtm >> ERROR binary comparing /tmp/gpsbabel.5288/gtm.gtm.gz >> ./reference/sample.gtm.gz >> ERROR binary comparing /tmp/gpsbabel.5288/gtm.gtm ./reference/sample.gtm >> ERROR comparing /tmp/gpsbabel.5288/ozi~gpx.gpx ./reference/route/ >> ERROR comparing ./reference/expertgps.rwf /tmp/gpsbabel.5288/expertgps.rwf >> >> >> >> On 7/17/2013 9:09 PM, Robert Lipe wrote: >> >> Aha! The warnings above that were the hint I (think I) needed. There >> was indeed an implicit time_t conversion remaining...which would have >> resulted in the time being zeroed out and thus, not displayed in GPX. >> >> I'm committing this. >> >> >> Index: waypt.cc >> =================================================================== >> --- waypt.cc (revision 4449) >> +++ waypt.cc (working copy) >> @@ -158,7 +158,7 @@ >> if ((wpt->longitude < -180) || (wpt->longitude > 180.0)) >> fatal("Invalid longitude %f in waypoint %s.\n", >> lon_orig, wpt->shortname ? wpt->shortname : ""); >> - if (wpt->creation_time < 0) { >> + if (!wpt->creation_time.isValid()) { >> warning("%s: Invalid timestamp in waypoint %s.\n", >> wpt->session->name, >> wpt->shortname ? wpt->shortname : ""); >> >> >> >> On Wed, Jul 17, 2013 at 9:58 PM, tsteven4 <tst...@gm...> wrote: >> >>> 10 failures in 7 tests for me on cygwin with a modified jenkins script >>> as of r4450, I included the gpx failure below. >>> bushnell >>> cetus >>> *gpx* >>> gtm >>> gtrnctr >>> ozi >>> raymarine >>> >>> Running ./testo.d/gpx.test >>> garmin_fs: Unable to convert category "Unlisted Data"! >>> garmin_fs: Unable to convert category "Unlisted Data"! >>> gpx: Invalid timestamp in waypoint Early. >>> gpx: Invalid timestamp in waypoint Late. >>> --- ./reference/bigtime.gpx 2013-07-17 19:31:40.207189700 -0600 >>> +++ /home/strabert/gpsbabel/gpsbabel/gbtemp/gpsbabel.5220/bigtime.gpx >>> 2013-07-17 20:51:13.586331900 -0600 >>> @@ -3,13 +3,11 @@ >>> <time>1970-01-01T00:00:00Z</time> >>> <bounds minlat="35.972033333" minlon="-87.134700000" >>> maxlat="36.090683333" maxlon="-86.679550000"/> >>> <wpt lat="35.972033333" lon="-87.134700000"> >>> - <time>1234-12-12T08:00:00.123Z</time> >>> <name>Early</name> >>> <cmt>Early</cmt> >>> <desc>Early</desc> >>> </wpt> >>> <wpt lat="36.090683333" lon="-86.679550000"> >>> - <time>4096-12-12T08:00:00.410Z</time> >>> <name>Late</name> >>> <cmt>Late</cmt> >>> <desc>Late</desc> >>> ERROR comparing ./reference/bigtime.gpx >>> /home/strabert/gpsbabel/gpsbabel/gbtemp/gpsbabel.5220/bigtime.gpx >>> >>> >>> >>> On 7/17/2013 8:26 PM, Robert Lipe wrote: >>> >>> gtrnctr, nmea applied. >>> >>> waypt patch not applied. I want to make that code go away, not apply >>> pain killers to it. the separate microseconds member must die. >>> >>> None of ozi, kml, or gpx SHOULD have time_t in their paths any more. >>> (I'm not saying they don't - I'm saying I'd treat their presence as a bug >>> to be fixed....) GPX bigtime, which I added last night, should use a >>> QDateTime end to end. If there's a time_ transition in there, we need to >>> track that down and whack it. I really don't want to add more special >>> cases for time_t > 2038; I want to remove pretty much every occurrence of >>> time_t throughout the code. Obviously, those formats that actually really, >>> really use UNIX epoch aren't going to handle < 1970--01-01 or > >>> 2038-whatever, but that's not ours to fix. >>> >>> Re: bushnell, cetus, gtm, raymarine - if you choose to flee instead of >>> fight those, I'd totally apply a patch to testo.d/WHATEVER.test that >>> ignores them on Windows. I have reason to think that our bushnell writer >>> on Windows works well enough and the others are obscure enough that I don't >>> see pouring a lot of development time into them. >>> >>> >>> >>> On Wed, Jul 17, 2013 at 7:37 PM, Gerhard Olsson < >>> ger...@gm...> wrote: >>> >>>> >>>> Thanks for applying. >>>> >>>> Most tests work fine in Cygwin now, which should indicate that the >>>> setup is more stable. >>>> The microsecond changes look fine to me, but it was only gpx that was >>>> visible in testo. >>>> >>>> New >>>> There is a rounding bug/"limitation" for ms output in waypt.cc. >>>> However when applying other format fails... To be continued (including >>>> patch for reference). >>>> Similarly, there is a rounding error in nmea/itracku. This affects >>>> reference files for nmea. Patch included. >>>> >>>> >>>> Summary of failures remaining for testo: >>>> >>>> gtrnctr >>>> csv compare still fail. However, there is an option for lat/lon >>>> precision in csv already. So the csv output for test scripts can be adopted >>>> to test that the input is the same as the output without any rounding, same >>>> as was done for tcx reference files. Patch included, no update of test >>>> reference outputs. (SetCreationTime(,) should handle milliseconds...) >>>> >>>> bushnell cetus gtm raymarine >>>> I will just blacklist in tests >>>> In my opinion, nothing else need to be done for these formats. >>>> For a binary format like gtm, the reference script could be separate >>>> per platform. >>>> Depreciating format and decreasing testing is a possible strategy >>>> (need to be documented). >>>> >>>> ozi, kml (segmented tracks), gpx (bigtime) >>>> Fail due to illegal time handling. (also after latest ozi update, in >>>> Cygwin) >>>> (my datetime.h "invalid time_t" proposed patch should handle this >>>> together with reference test updates) (the time_t handling should maybe >>>> check for >2038 too?) >>>> >>>> >>>> >>>> On Wed, Jul 17, 2013 at 7:24 AM, Robert Lipe <rob...@gp...>wrote: >>>> >>>>> I made a moderate attack on the code, applying the patches of yours >>>>> that I agree are an uncontested good idea and spiffing them up when >>>>> necessary, such as fixing reference files. The fact that I textually >>>>> agreed with so many of your patches before even reading them shows we're in >>>>> harmony. I'm not saying the ones that are left aren't a good idea, they >>>>> just need more consideration. For those of you that want to play along, >>>>> you can see the progress at >>>>> https://code.google.com/p/gpsbabel/source/list >>>>> >>>>> I also stirred in more changes that move us to my promised land of >>>>> explicit access to the microseconds field that's not really attached to the >>>>> QTime. We still have references remaining in at least >>>>> >>>>> arcdist.cc >>>>> destinator.cc >>>>> mtk_logger.cc >>>>> ggv_log.cc >>>>> unicsv.cc >>>>> stmwpp.cc >>>>> ozi.cc >>>>> nmea.cc >>>>> mapsend.cc >>>>> magproto.cc >>>>> csv_util.cc (along with unicsv.cc, needs better test coverage to >>>>> handle the bizarre way we let dates and times exist - could really benefit >>>>> from QDate and QTime rethink) >>>>> waypt.cc (needs a rethink...) >>>>> >>>>> They're probably not individually rocket surgery to approach; >>>>> they're just a little more than mechanical cargo culting for the >>>>> night/morning. The end of 'microseconds' as a separate member of struct >>>>> waypoint is in sight. >>>>> >>>>> This might all seem like busy work, but we have a whole bunch of >>>>> things internally that don't really know about the microseconds sidecar. >>>>> (It's an afterthought in the code, and it shows...) Once we have a >>>>> QDateTime used throughout, with the key implicit conversions to a time_t >>>>> removed (I care less about those in input and output formats than in our >>>>> core and in the filters) things like using the track filters on GPSes that >>>>> collect sub-second times - which are increasingly common in fitness and >>>>> automotive worlds - will no longer be such a gamble. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> On Tue, Jul 16, 2013 at 6:54 PM, Robert Lipe <rob...@gp...>wrote: >>>>> >>>>>> Fast answer. More later. >>>>>> >>>>>> double/float precision (garmin_fit, html, text) >>>>>>> While precision in the GPS is likely not even float, it makes a >>>>>>> difference when reading data and converting to float and then comparing as >>>>>>> double as done in these modules. >>>>>>> As output is compared with higher precision than in float, the tests >>>>>>> will be very platform specific >>>>>>> Note: a lot of the garmin_fit testo scripts are affected by this. >>>>>>> >>>>>> >>>>>> I've always been surprised at the murder we get away with on >>>>>> floating point in general. >>>>>> >>>>>> We totally get to make up the rules on html and text. Those can be >>>>>> changed to doubles w/o issue. I'd have to look more at garmin_fit. >>>>>> >>>>>> invalid time_t (ozi,kml) >>>>>>> [ ... ] >>>>>>> >>>>>> Timestamp with milliseconds >>>>>>> >>>>>> >>>>>> There are definite rubber bands in this area that need to come off. >>>>>> My plan is for milliseconds as a separate field to go away and all of the >>>>>> time_t accesses and overloads to go away. time is always stored, compared, >>>>>> manipulated in a QDateTime in GMT period. Anything that actually wants a >>>>>> time_t can get one via toTime_t(). All of the horrible code doing things >>>>>> like building a struct tm and then passing to and from localtime/gmtime to >>>>>> get the current TZ offset and dealing with microseconds as a separate >>>>>> member needs to just plain quit it. I simply couldn't replace it all at >>>>>> once, so I write a bunch of horrible overloads to keep things somewhat >>>>>> coherent. >>>>>> >>>>>> It should be easier to fix this generally if microseconds are >>>>>>> removed as a separate accessed field (possibly accessed with functions for >>>>>>> those needing it?) >>>>>>> >>>>>> >>>>>> microseconds should definitely go away. That's been my plan for a >>>>>> while; I've only partially executed it. Some modules, like gpx.cc have >>>>>> completely removed separate access to time_t and microsseconds and now >>>>>> support arbitrary, high resolution times. Some modules, like nmea.cc, >>>>>> torture themselves with the old way. Some code, like waypt_time(), >>>>>> waypt_speed(), and waypt_speed_ex() ultimately become simpler, but right >>>>>> now, are propped up by horrible crutches that rely on silent time_t >>>>>> conversions. There's even some code in there somewhere that sets the >>>>>> microsecond field from the msecs() of the QDateTime just to further present >>>>>> the illusion that we don't (temporarily) have two different concepts of >>>>>> time. >>>>>> >>>>>> time offsets (maggeo) >>>>>>> time input is done with mktime, output is done with gmtime instead >>>>>>> of localtime. Depending on when the test is run, this could give an error >>>>>>> in the test scripts depending on time offset (that read/writes to maggeo >>>>>>> format, systematic errors are not catched...) >>>>>>> >>>>>> >>>>>> I have inside info on maggeo. Frankly, our reader exists only for >>>>>> our own convenience; it never makes sense for us to read these files "in >>>>>> the wild" and i don't think the time ever actually matters in these files >>>>>> anyway as it only writes dates into the file. Yes, that means our days >>>>>> could be off one on some edge case, but it's all not very interesting. I'm >>>>>> pretty skeptical that many of these GPSes are still in use anyway; as >>>>>> perspective, the product line that replaced this product line is >>>>>> discontinued. >>>>>> >>>>>> Other rounding (gtrnctr) >>>>>>> This affects several modules, primarily tests for gtrnctr fails >>>>>>> I do not believe that there can be something done about it with the >>>>>>> current setup, it depends on the precision and rounding. So multi platform >>>>>>> support likely requires either a special built program or that output is >>>>>>> rounded before comparing. GPX already limits number of decimals at output >>>>>>> to make it possible to test formats. >>>>>>> >>>>>> >>>>>> That ghost has haunted us forever. We still get people insisting >>>>>> we support 32 decimal digits in GPX which is, of course, total fantasy, >>>>>> just because some program wrote a position that looks like it has 32 >>>>>> significant digits. You know, for all those GPSes with sub-micron >>>>>> precision. But I have considered some mode for our GPX writer that let >>>>>> you control the number of digits in lat/lon/alt just to make things more >>>>>> testable since it's not like 'diff' suppports a "near" concept. >>>>>> >>>>>> For grtnctr yhere can be something that is transparent to users >>>>>>> though: Set precision to 7 decimals for lat/lon, (same as Garmin uses by >>>>>>> default) and round (simple) at output. Affects all test compare files. Same >>>>>>> thing needs to be done for csv output (or use gpx instead). >>>>>>> The current error in testo now is that 34.1967295 in the source >>>>>>> can be rounded up or down. >>>>>>> >>>>>> >>>>>> Offhand, if a format specificer in >>>>>> gtc_write_xml(0, "<LatitudeDegrees>%f</LatitudeDegrees>\n", >>>>>> wpt->latitude); >>>>>> gtc_write_xml(0, "<LongitudeDegrees>%f</LongitudeDegrees>\n", >>>>>> wpt->longitude); >>>>>> >>>>>> brings peace, great. I'd like to replace all that gtc_write_xml() >>>>>> stuff with calls to a better XML serializer anyway. >>>>>> >>>>>> >>>>>>> Obsolete formats >>>>>>> Some other formats that I have not investigated. Those could maybe >>>>>>> be excluded blacklisted in normal tests? >>>>>>> * Cetus (Palm related). Speed differs 2ppm (rounding) >>>>>>> >>>>>> >>>>>> I'll admit I'm one temper tantrum away from dropping all the >>>>>> Palm/OS stuff >>>>>> >>>>>> * Bushnell (obsolete?) >>>>>>> >>>>>> >>>>>> Bushnell (yes, the binocular people) made a run in the GPS biz in >>>>>> the mid 2000's. They dumped them in various fire sales (Woot, etc.) in the >>>>>> U.S. a few years ago and we saw a brief spike of interest in them, but I >>>>>> think the people that bought them moved on pretty quickly. That said, that >>>>>> format is dead simple and shouldn't be too hard to keep alive, plus I >>>>>> actually have one for testing if we need to >>>>>> >>>>>> >>>>>>> * gtm Binary protocol, seem to be lat/lon rounding. Those are >>>>>>> stored as double, so the conversion done when reading will show in the test. >>>>>>> * Raymarine/expertgps Uses 12 decimals comparing lat/lon (so this >>>>>>> is the general compare issue) >>>>>>> >>>>>> >>>>>> We used to hear a lot about GTM and a little about Raymarine. I >>>>>> honestly don't know how popular they are. >>>>>> >>>>>> >>>>>> This is an awkward thing about GPSBabel. We really can't tell the >>>>>> difference between "works so well that it Just Works and nobody ever asks >>>>>> questions about it" and "nobody cares about this dead format". So in the >>>>>> current round of overhaul, I'm spending brain power worrying about code >>>>>> that I'm not _really_ sure anybody cares about. >>>>>> >>>>>> If you look at the gpsbabel-misc traffic for the last, say, year, >>>>>> we could support only gpx, kml, unicsv, garmin, maybe a few filters, and >>>>>> pretty much be done. Throw in a couple of the $40 logger products for good >>>>>> measure, but honestly I think they're a disproportionate percentage of our >>>>>> support load just because that market is full of $40 products. Web >>>>>> analytics for our help pages pretty much confirm that. At what point >>>>>> should we quit being the historians of the equivalent of DEC or Data >>>>>> General gear? Do we see no traffic on, say, Cetus or IGC because it works >>>>>> great or because nobody cares? We have formats in our arsenal that I'm >>>>>> unconvinced have been used by anyone other than the submitter. >>>>>> >>>>>> This is why I've relied on things like the overloads for time >>>>>> operators instead of trying to figure out if some obscure PocketPC program >>>>>> really can do times beyond that. >>>>>> >>>>>> So much for fast answer...still, more later. >>>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> ejgo@ejgo3 /cygdrive/f/dev/gc/gpsbabel-read-only/gpsbabel >>>>>>> $ ./testo maggeo >>>>>>> Running ./testo.d/maggeo.test >>>>>>> --- ./reference/gc/maggeo.gs 2011-12-25 22:07:51.147246100 +0100 >>>>>>> +++ /tmp/gpsbabel.5012/maggeo2.gs 2013-07-15 >>>>>>> 13:45:06.647035300 +0200 >>>>>>> @@ -1,3 +1,3 @@ >>>>>>> -$PMGNGEO,4608.000,N,7300.000,W,0000,F,GC7FA4,Points geodesiques >>>>>>> d,Sverdrup2,,Locationless (Reverse) Cache,1508102,1207105,1.0,1.0*3E >>>>>>> -$PMGNGEO,3555.300,N,8651.700,W,0000,F,GCGCA8,Oozy rat in a >>>>>>> sanita,robertlipe,,Mystery Cache,2906103,0307105,3.0,2.0*4A >>>>>>> +$PMGNGEO,4608.000,N,7300.000,W,0000,F,GC7FA4,Points geodesiques >>>>>>> d,Sverdrup2,,Locationless (Reverse) Cache,1408102,1107105,1.0,1.0*3C >>>>>>> +$PMGNGEO,3555.300,N,8651.700,W,0000,F,GCGCA8,Oozy rat in a >>>>>>> sanita,robertlipe,,Mystery Cache,2806103,0207105,3.0,2.0*4A >>>>>>> $PMGNCMD,END*3D >>>>>>> ERROR comparing ./reference/gc/maggeo.gs /tmp/gpsbabel.5012/ >>>>>>> maggeo2.gs (4 lines differ) >>>>>>> >>>>>>> ejgo@ejgo3 /cygdrive/f/dev/gc/gpsbabel-read-only/gpsbabel >>>>>>> $ ./testo ozi >>>>>>> Running ./testo.d/ozi.test >>>>>>> --- /tmp/gpsbabel.6444/ozi~gpx.gpx 2013-07-15 >>>>>>> 13:45:13.315166100 +0200 >>>>>>> +++ ./reference/route/ozi~gpx.gpx 2013-07-11 >>>>>>> 18:27:48.227345800 +0200 >>>>>>> @@ -19,7 +19,7 @@ >>>>>>> <name>1 PCHI COLONIA</name> >>>>>>> <number>1</number> >>>>>>> <rtept lat="-34.467500000" lon="-57.854170000"> >>>>>>> - <time>2038-01-19T03:48:41.437Z</time> >>>>>>> + <time>2036-02-05T06:28:16Z</time> >>>>>>> <name>COLONI</name> >>>>>>> <cmt>07-OCT-00 18:22</cmt> >>>>>>> <desc>07-OCT-00 18:22</desc> >>>>>>> ERROR comparing /tmp/gpsbabel.6444/ozi~gpx.gpx ./reference/route/ (2 >>>>>>> lines differ) >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> ------------------------------------------------------------------------------ >>>>>>> See everything from the browser to the database with AppDynamics >>>>>>> Get end-to-end visibility with application monitoring from >>>>>>> AppDynamics >>>>>>> Isolate bottlenecks and diagnose root cause in seconds. >>>>>>> Start your free trial of AppDynamics Pro today! >>>>>>> >>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk >>>>>>> _______________________________________________ >>>>>>> Gpsbabel-code mailing list http://www.gpsbabel.org >>>>>>> Gps...@li... >>>>>>> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> See everything from the browser to the database with AppDynamics >>>> Get end-to-end visibility with application monitoring from AppDynamics >>>> Isolate bottlenecks and diagnose root cause in seconds. >>>> Start your free trial of AppDynamics Pro today! >>>> >>>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk >>>> _______________________________________________ >>>> Gpsbabel-code mailing list http://www.gpsbabel.org >>>> Gps...@li... >>>> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code >>>> >>>> >>> >>> >>> ------------------------------------------------------------------------------ >>> See everything from the browser to the database with AppDynamics >>> Get end-to-end visibility with application monitoring from AppDynamics >>> Isolate bottlenecks and diagnose root cause in seconds. >>> Start your free trial of AppDynamics Pro today!http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk >>> >>> >>> >>> _______________________________________________ >>> Gpsbabel-code mailing list http://www...@li...https://lists.sourceforge.net/lists/listinfo/gpsbabel-code >>> >>> >>> >> >> > |