gpsd-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [gpsd-dev] [PATCH 1/3] TSIP: improve comments


From: Gary E. Miller
Subject: Re: [gpsd-dev] [PATCH 1/3] TSIP: improve comments
Date: Mon, 20 Jun 2016 18:06:51 -0700

Yo Nuno!

On Tue, 21 Jun 2016 00:33:03 +0100
Nuno Gonçalves <address@hidden> wrote:

> I would say they are self-explanatory, and the 1/3 and 2/3 are
> trivial.

Best to always assume I'm an idiot.  TSIP is not my thing.  Got a
pointer to some doc?  Something on the new message types.

> The 3/3 is to add support to multi-constelation Trimble Resolution/Res
> chips, and I tested it with a Trimble RES SMT 360.

Got a pointer to some doc?  Soemthing on the GPS chip/system.

> Could you please point me if I need to add data samples or regression
> tests, and how to do it?

Easy to test, after you build the new gpsd, just run this in the
same directory you built in:
        # scons check

More in the ahcking guide:
        http://www.catb.org/gpsd/hacking.html

Check out the section: Contribution guidelines

I got some errors while applying your patches:

spidey gpsd # git am -s /tmp/000*
Applying: TSIP: improve comments
.git/rebase-apply/patch:14: trailing whitespace.
 * Trimble RES multi-constelation support by Nuno Goncalves <address@hidden> 
warning: 1 line adds whitespace errors.
Applying: TSIP: fix time offset
.git/rebase-apply/patch:16: trailing whitespace.
     * Trimble Resolution T   : 0.03s to 0.200s (depends on the SV# being 
tracked) 
warning: 1 line adds whitespace errors.
Applying: TSIP: support multi-constelation chip packets
.git/rebase-apply/patch:43: trailing whitespace.
        u5 = getub(buf, 20);    /* old measurement flag */      
.git/rebase-apply/patch:136: trailing whitespace.
        break; 
warning: 2 lines add whitespace errors.

Looks like you have some trailing tabs in the patches.  One of the more
annoying misfeatures of git...  Also you let some line lengths go
beyond 80 chars.

The patches also broke twe regression tests.  That may not be a real problem,
it may just mean that you are now decoding things better than before.

Either way, please check out the regressions and verify that change in
output is what you expected.  Here are the faioling tests:

test/daemon/trimble-lassen_iq-3dfix.log
test/daemon/trimble-lassen_iq.log

Looks to be some change in GPGSA handling?  If the new output looks
right to you then I can update the regressions and fix the whitespace.

I also lookad at the git diff a bit.  Tis looks like it is going backwards:

-    /* FIX-ME: is a constant offset right here? */
-    return 0.075;
+    /* the offset depends on the generation and is not reliable, some example 
measurements:
+     * Trimble Resolution T   : 0.03s to 0.200s (depends on the SV# being 
tracked) 
+     * Trimble Resolution SMTx: 0.300 to 0.500s (unpredictable)
+     * Trimble RES SMT 360    : 0.028s (this appears very stable)*/
+    return 0.0;

Your comments point to a code improvement, but then go backwards.  By
your comments seems that 0.075 is more correct than 0.0 How about a real
fix, or at least a partial fix?  Just remopving it will degrade existing
users.

RGDS
GARY
---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703
        address@hidden  Tel:+1 541 382 8588

Attachment: pgpQySUFACa50.pgp
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]