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

[Nuno Gonçalves]

On Tue, Jun 21, 2016 at 2:06 AM, Gary E. Miller <address@hidden> wrote:
> 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.
>
> Got a pointer to some doc?  Soemthing on the GPS chip/system.

Thanks for the time you took to help me fix this contribution.

I based the new sentences code at
http://trl.trimble.com/docushare/dsweb/Get/Document-733473/:

ICM SMT 360TM
& RES SMT 360TM
Multi-GNSS Timing Modules
Version C
Feb 2016
Part Number 94472-00

> I got some errors while applying your patches:

Yes, gmail web interface does not handle line breaks in a compatible
way. I attach the patch files for that reason. Unless the list don't
accept text attachements (which I'm not aware), please use those
instead.

> 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.

Sorry I didn't run the regression tests. They seem to be affected by
the increase in channel count:

-#define TSIP_CHANNELS 12
+#define TSIP_CHANNELS 15

This for some reason changes the NMEA code format (which is not coming
from the device), adding 3 additional empty fields to the GPGSA:

-$GPGSA,A,2,19,18,1,11,3,22,9,,,,,,3.9,3.7,1.0*06
+$GPGSA,A,2,19,18,1,11,3,22,9,,,,,,,,,3.9,3.7,1.0*2A

This looks fair, but I don't know much about NMEA.

> 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.

I don't know. 0.075s is not generic for TSIP. It is also not
documented from where this magic value was obtained.

TSIP protocol does not specify at all the solution completion delay,
so this will change between devices and probably between firmware
versions.

I don't see a safe way, not even a unsafe way, to calculate the offset
value. If this is deemed to break client assumptions I have nothing
against leaving it as it is, I would just suggest for the comments to
be incorporated so that it is mentioned the 0.075 value is just kept
for historical compability.

So if this is the case I would update my patchs to:

(1) - update the regression test output
(2) - do not change the offset value 0.075, but provide comments about
the reasoning.

Regards,
Nuno