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

[Gary E. Miller]

Yo Nuno!

On Tue, 21 Jun 2016 13:39:01 +0100
Nuno Gonçalves <address@hidden> wrote:

> 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'll add a comment on that.

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

Nope.  I used the attacments and not the inline patches.  The line
ends are in your original copy.  Just do a search in the editor of
your choice for a sppace or tab before a line end.

> > 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 don't have the NMEA doc, but one reference says only 12 PRNs are
allowed in that sentence.  Another says 14.  We'll need to check that.
Also, will there be GLONASS, BeiDou or other sats in there?  Then maybe
it should be a GNGSA.

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

So what is the delay you see?  Can you identify your model and put in
the fudge you see automagically?  If not can you add in your
observed fudge as a comment?

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

Except someone already commplained that gives them a negative fudge
which is bad.

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

Sounds good, allowing for my above comments.

Also, in your patch 1:

-                * 0x54, One Satellite Bias, data length 4
+                * 0x54, One Satellite Bias, data length 4 - not according to 
the length check below!

According to the doc you send, length 12 is correct.  So just fix the mistake.


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

pgpgkZfXPK90u.pgp
Description: OpenPGP digital signature