ntpshm.c PPS bad usec reference

[Håkan Johansson-5]

At rev 5997, ntpshm for PPS went bad.  The entire idea of a PPS pulse is
that the reference is the point with usec = 0 (or possible an even
multiple of the PPS period).  The assignment

shmTimeP->clockTimeStampUSec = usecs;

is the culprit.  The following (see bottom) seems to rectify that while
maintaining the offset logics, by subtracting away leftovers from usec
with respect to the closest PPS period multiple.

On a side note:  I think the calculation of the precision, using the
variable offset has issues.  As one in ntpd can fudge an additional offset
of the reported times, when the system clock is driven towards that, we
will never approach a zero difference, and therefore always report a 'bad'
precision.  I think a precision estimate need to be based on the jitter
(which would require some history...), and not on any absolute offset.

Cheers,
Håkan

Index: ntpshm.c
===================================================================
--- ntpshm.c    (revision 6410)
+++ ntpshm.c    (arbetskopia)
@@ -180,6 +180,7 @@
      usecs = shmTime->clockTimeStampUSec + 1000000 / rate;
      seconds = shmTime->clockTimeStampSec + usecs / 1000000;
      usecs %= 1000000;
+    usecs -= ((usecs - (1500000 / rate)) % (1000000 / rate)) + (500000 / rate);

      /*
       * Curiously, splint complains "Assignment of long int to double"
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Miroslav Lichvar]

On Wed, Nov 04, 2009 at 07:19:31AM +0100, Håkan Johansson wrote:
> At rev 5997, ntpshm for PPS went bad.  The entire idea of a PPS
> pulse is that the reference is the point with usec = 0 (or possible
> an even multiple of the PPS period).  The assignment
>
> shmTimeP->clockTimeStampUSec = usecs;
>
> is the culprit.

Yes, the usecs value should be always 0 unless the PPS signal has
frequency higher than 1 Hz. It's computed from
shmTime->clockTimeStampUSec which should be the time decoded from the
last received message. Most GPSs don't seem to even report anything
below seconds, so I'm wondering where does it come from.

What values do you see there? What kind of GPS unit is it?

> On a side note:  I think the calculation of the precision, using the
> variable offset has issues.  As one in ntpd can fudge an additional
> offset of the reported times, when the system clock is driven
> towards that, we will never approach a zero difference, and
> therefore always report a 'bad' precision.  I think a precision
> estimate need to be based on the jitter (which would require some
> history...), and not on any absolute offset.

I think measuring jitter in gpsd would require that no other
application is controling the local clock. Using a fixed value like
-20 should be good enough.

--
Miroslav Lichvar
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Håkan Johansson-5]

> Yes, the usecs value should be always 0 unless the PPS signal has
> frequency higher than 1 Hz. It's computed from
> shmTime->clockTimeStampUSec which should be the time decoded from the
> last received message. Most GPSs don't seem to even report anything
> below seconds, so I'm wondering where does it come from.
>
> What values do you see there? What kind of GPS unit is it?

It's a motorola oncore.  I saw something like 176 ms.  Aha, it is mostly
the offset of 0.16 s in driver_oncore.c at the call to ntpshm_put(), which
is needed to get that one almost correct (give or take some ms).  Note
that several other devices (almost all) also has these kinds of offsets,
so the assumption that shmTime->clockTimeStampUSec is 0 does not work.

>> On a side note:  I think the calculation of the precision, using the
>> variable offset has issues.  As one in ntpd can fudge an additional
>> offset of the reported times, when the system clock is driven
>> towards that, we will never approach a zero difference, and
>> therefore always report a 'bad' precision.  I think a precision
>> estimate need to be based on the jitter (which would require some
>> history...), and not on any absolute offset.
>
> I think measuring jitter in gpsd would require that no other
> application is controling the local clock. Using a fixed value like
> -20 should be good enough.

True, when running with ntpshm some other program would presumably be
controlling the clock.  But only slowly enough that e.g. measuring the
jitter during the last 10-20 s should be fine.  One may even factor a
linear relationship out.  I'll wrap something up.  (I already almost have
it as I'm running a filter in ntpshm_pps.  The filter itself is perhaps
not needed any longer as new ntpd does filtering.  In principle, the
precision calculation should/could be done by ntpd itself too.)

// Håkan
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Miroslav Lichvar]

On Wed, Nov 04, 2009 at 01:19:37PM +0100, Håkan Johansson wrote:
> It's a motorola oncore.  I saw something like 176 ms.  Aha, it is
> mostly the offset of 0.16 s in driver_oncore.c at the call to
> ntpshm_put(), which is needed to get that one almost correct (give
> or take some ms).  Note that several other devices (almost all) also
> has these kinds of offsets, so the assumption that
> shmTime->clockTimeStampUSec is 0 does not work.

Ok, that explains it. Some of the drivers add very high values like
0.8 and 1.1. The question is should we round to the closest second or
always down?

> >I think measuring jitter in gpsd would require that no other
> >application is controling the local clock. Using a fixed value like
> >-20 should be good enough.
>
> True, when running with ntpshm some other program would presumably
> be controlling the clock.  But only slowly enough that e.g.
> measuring the jitter during the last 10-20 s should be fine.  One
> may even factor a linear relationship out.  I'll wrap something up.
> (I already almost have it as I'm running a filter in ntpshm_pps.
> The filter itself is perhaps not needed any longer as new ntpd does
> filtering.  In principle, the precision calculation should/could be
> done by ntpd itself too.)

ntpd computes the jitter in the median filter, same for chrony. I
really think something like -20 will do fine, other ntpd refclock
drivers do it too.

--
Miroslav Lichvar
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Håkan Johansson-5]

> Ok, that explains it. Some of the drivers add very high values like
> 0.8 and 1.1. The question is should we round to the closest second or
> always down?

If we'd always round down, we might hit the wrong second when the system
clock is late and the sample came 'too early'?  I'd vote for going to the
closest second.  A PPS signal as such is anyhow ill-defined in-between the
second, so my feeling would be against allowing it through at all when it
is more than 10-20 % wrong.  Relative to the low-precision second-marker
produced by the gps unit (ntpshm_put), not relative to the system clock.

> ntpd computes the jitter in the median filter, same for chrony. I
> really think something like -20 will do fine, other ntpd refclock
> drivers do it too.

Yes.  As far as I understand ntpd, the jitter would be the measure of the
current performance, while the precision is more like a static statement
about the maximum achievable performance?  Then, -20 would it.

// Håkan
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Miroslav Lichvar]

On Wed, Nov 04, 2009 at 02:21:54PM +0100, Håkan Johansson wrote:
> If we'd always round down, we might hit the wrong second when the
> system clock is late and the sample came 'too early'?

System clock is not involved here, it's assumed that the pulse was
received at the time that was decoded from last message + 1 second.

> I'd vote for
> going to the closest second.  A PPS signal as such is anyhow
> ill-defined in-between the second, so my feeling would be against
> allowing it through at all when it is more than 10-20 % wrong.
> Relative to the low-precision second-marker produced by the gps unit
> (ntpshm_put), not relative to the system clock.

Rounding down makes more sense to me if the 0.8 value is just a
compensation for a delay. The higher value 1.1 is puzzling. :)

--
Miroslav Lichvar
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Håkan Johansson-5]

> On Wed, Nov 04, 2009 at 02:21:54PM +0100, Håkan Johansson wrote:
>> If we'd always round down, we might hit the wrong second when the
>> system clock is late and the sample came 'too early'?
>
> System clock is not involved here, it's assumed that the pulse was
> received at the time that was decoded from last message + 1 second.

Then perhaps an alternative solution 1:

ntpshm_put() is extended with one more argument, which takes the offset.
The offset is only used to correct shmTime and not used for shmTimeP.
ntpshm_pps() does then not need to guess what strange corrections were
applied, no roundings at all.

Solution 2:

The offset applied when calling ntpshm_put() is presumed to make sure that
it is aligned with the value gotten from gettimeofday() (t1) inside
ntpshm_put().  Keep the offset and consider it in ntpshm_pps().  We
therefore also need to consider the value from gettimeofday() (t2) in
ntpshm_pps().  The difference between t2 and t1 should then be the
difference from the last time statement by ntpshm_put()to the pps pulse
under consideration.  Use this and align the pps pulse to the closest
multiple of the rate, with rounding.

// Håkan
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Miroslav Lichvar]

On Wed, Nov 04, 2009 at 04:58:12PM +0100, Håkan Johansson wrote:
> >System clock is not involved here, it's assumed that the pulse was
> >received at the time that was decoded from last message + 1 second.
>
> Then perhaps an alternative solution 1:
>
> ntpshm_put() is extended with one more argument, which takes the
> offset. The offset is only used to correct shmTime and not used for
> shmTimeP. ntpshm_pps() does then not need to guess what strange
> corrections were applied, no roundings at all.

Will this work correctly when the correction is 1.1? If the messages
are really delayed by 1.1 seconds, the pulse time will need to be two
seconds off the time received in the last message. Or not?

I'd say simple rounding down should work, can anyone with the Zodiac
device confirm this?

Index: ntpshm.c
===================================================================
--- ntpshm.c    (revision 6410)
+++ ntpshm.c    (working copy)
@@ -178,6 +178,7 @@
     }
 
     usecs = shmTime->clockTimeStampUSec + 1000000 / rate;
+    usecs -= usecs % (1000000 / rate);
     seconds = shmTime->clockTimeStampSec + usecs / 1000000;
     usecs %= 1000000;
 


--
Miroslav Lichvar
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Gary E. Miller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yo Håkan!

On Wed, 4 Nov 2009, Håkan Johansson wrote:

> approach a zero difference, and therefore always report a 'bad' precision.

I think you misunderstand 'precision' as used by ntpd.  It is just a
starting point for ntpd.  After ntpd decides the PPS input (or any time
input) it not just wacko it will take over computing the precision.

All gpsd needs to do is report only valid ticks to ntpd and let ntpd handle
the rest.

RGDS
GARY
- ---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97701
        gem@...  Tel:[phone]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFK8b/EBmnRqz71OvMRAlUbAJ9QKQKyEDDFq5j891RYn36WqMEchgCfd5mi
+5FLu/MNTP0BnLvbAwHX+ZE=
=5FGO
-----END PGP SIGNATURE-----

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Gary E. Miller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yo Miroslav!

On Wed, 4 Nov 2009, Miroslav Lichvar wrote:

> Most GPSs don't seem to even report anything
> below seconds, so I'm wondering where does it come from.

But many do.  Garmin has 5Hz models and many from other manufacturers
go at a higher rate.  gpsd strives to support everything GPS.


RGDS
GARY
- ---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97701
        gem@...  Tel:[phone]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFK8cBpBmnRqz71OvMRAo8oAJkB6V1qNOuKvvB8Ck2kBwgL0z+0zwCfUybf
WZQm/55hTKOKt4UjwvmoVjU=
=d5LH
-----END PGP SIGNATURE-----

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Gary E. Miller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yo Håkan!

On Wed, 4 Nov 2009, Håkan Johansson wrote:

> If we'd always round down, we might hit the wrong second when the system clock
> is late and the sample came 'too early'?  I'd vote for going to the closest
> second.  A PPS signal as such is anyhow ill-defined in-between the second, so
> my feeling would be against allowing it through at all when it is more than
> 10-20 % wrong.  Relative to the low-precision second-marker produced by the
> gps unit (ntpshm_put), not relative to the system clock.

I think you are looking at this the wrong way.  it is ntpd's job to decide
when a pulse is 'wrong', not gpsd's.  Most GPS have very stringent guarantees
on their PPS so any mistake is 99.9% a gpsd mistake.  All gpsd needs to
do is figure out which is the proper edge to send to ntpd.

The existing logic save for a few corner cases, has worked very well for
a wide variety of cases for a long time.  Changes (except reverting the
recent breakage) should be carefully considered.

RGDS
GARY
- ---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97701
        gem@...  Tel:[phone]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFK8cKFBmnRqz71OvMRAtZrAJ4x08kDqadYYbfrcw6el2tDVm31uwCeMl3z
VYOvFS+NXrkpD/dikuqmFjo=
=MdoO
-----END PGP SIGNATURE-----

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Gary E. Miller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yo Håkan

On Wed, 4 Nov 2009, Håkan Johansson wrote:

> The offset applied when calling ntpshm_put() is presumed to make sure that it
> is aligned with the value gotten from gettimeofday() (t1) inside ntpshm_put().

Cart, horse wrong order.  Gettimeofday() is the RESULT of a good GPS
time not the creator.

RGDS
GARY
- ---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97701
        gem@...  Tel:[phone]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFK8cNjBmnRqz71OvMRAu1hAJ43ylT3w+F3KiGic0fAEcXSyJodEQCgsdGJ
VvYekiQ/zJ8Jpe68rz30ihM=
=j7eK
-----END PGP SIGNATURE-----

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Eric S. Raymond-2]

Gary E. Miller <gem@...>:
> The existing logic save for a few corner cases, has worked very well for
> a wide variety of cases for a long time.  Changes (except reverting the
> recent breakage) should be carefully considered.

This seems right to me.  I have reverted the changed in r5997.

We now have three conflicting versions of what should be done with ntpsm.c;
one by Hakan, one by Miroslav, and Tim Jackson's patch from back in March.
I don't understand the code well enough to judge which if any, should
be applued.

You're our PPS expert, Gary.  Can you lead a discussioin on thios and
approve whatever changes should be made?

I'm also awaiting froom you changes to make Garmin functional again.
I'm sorry I had to disable the Garmin hunt, but we had a user complaining of
a config loop and you weren't around.  I'd like a better solution for 2.40.
--
                <a href="http://www.catb.org/~esr/">Eric S. Raymond</a>


_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Gary E. Miller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yo Eric!

On Wed, 4 Nov 2009, Eric S. Raymond wrote:

> I don't understand the code well enough to judge which if any, should
> be applued.

Doing some rearrangements and my Garmin PPS is not hooked up.  Hopefully
will be back top where I can debug in a few more days.  Also got a new
SiRF PPS coming in by Friday.

RGDS
GARY
- ---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97701
        gem@...  Tel:[phone]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFK8gUdBmnRqz71OvMRAm/uAKDfwyY+ylRkMT7xzKDKzKZOSl4/1ACgqdg/
tmnppFovWmQAX4Mjv64wZIk=
=hnH1
-----END PGP SIGNATURE-----

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Håkan Johansson-5]

Hi again,

here is another go.  The one-line patch I came with before, please ignore
that.  After manual-, code-, bugreport-, and old mail-reading:

The issue of determining which edge of the PPS pulse to use is handled in
libgpsd_core.c, well separated from the rest.  Helps.  Miroslav's patch in
that file, using the min_interval, max_interval, in_interval macros I
think did make it more readable.  It did however make the acceptance
ranges for intervals larger, and also made some stricter requirements on
the length of the pulses (or rather the time between the pulses).  It is
probably orthogonal enough that we can treat this as a separate issue from
ntpshm_{put,pps}?

According to manuals, the garmin 18, (16+17), oncore and sirf deliver the
serial time-stamp messages _after_ the PPS pulse.  I.e. the time given
refers to a pulse that has occurred.  The zodiac is opposite, giving the
time of the pulse which will come (message ~400 ms beforehand).  This
perhaps explains the > 1 s magic offsets for the zodiac?

I'd propose to re-model the work-division between ntpshm_put and
ntpshm_pps according to this strict chronology of events.  ntpshm_put() is
called from the respective drivers and thus knows which model applies.
It can give this information as an additional argument, PPS_BEFORE_SERIAL,
or PPS_AFTER_SERIAL.  It also should carry either two times or one time
and an offset describing the other time.  One of the times would be the
correct time of the referenced PPS pulse.  The other would be the best
estimate of the correct time of the serial message.  These extra arguments
avoids guesswork by ntpshm_pps.

In any case, ntpshm_put does the update of shmTime just like it does now.

ntpshm_pps would for PPS_BEFORE_SERIAL cases only take the system clock
time stamp of the pps pulse, and store this for processing by the next
ntpshm_put call.  ntpshm_put verifies that not more than 1 period passed
since the last PPS pulse seen.  If ok, then update shmTimeP using the PPS
reference time and declare the pulse consumed.

For PPS_AFTER_SERIAL, ntpshm_pps does the update of shmTimeP, after
ensuring that we are not more than 1 period away from the time info was
delivered by ntpshm_put.

This should work also as is for the 5 Hz pulses from garmin.  According to
manual, they give serial time info 5 times per second, with 0.1 s
resolution.  (I presume 0, 0.2, 0.4, 0.6, 0.8)

I am aware this is a substantial logics change.  Would it be interesting
to anyhow try, such that I come up with a patch?

Additional notes:

Even if one get the 5 Hz pulses to work in gpsd, ntpd will only use one
per second.  I suppose it in principle would be more important for
precision time-keeping to use e.g. the linuxpps time stamping facilities
and get some (e.g. 1 per second) accurate pps pulses rather than a lot
that needs heavy filtering (in ntpd or some other place) and suffer from
varying load-dependent latencies.

If we want the (coarse, shmTime) serial message time stamps to be better
(not very interesting to people with PPS, but perhaps to others), I guess
it may be of interest to start to time-stamp all serial message
receptions, such that corrections for various baud-rates can be applied.

Cheers,
Håkan

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Miroslav Lichvar]

On Wed, Nov 04, 2009 at 10:05:55AM -0800, Gary E. Miller wrote:
> The existing logic save for a few corner cases, has worked very well for
> a wide variety of cases for a long time.  Changes (except reverting the
> recent breakage) should be carefully considered.

If you are talking about the state before r5997, it didn't work well
for me (with GPS 18x LVC). I had to tweak the *MAX_OFFSET values. Then
there was the bug that both edges were reported so there was high
chance that ntpd was syncing to clear edge instead of assert.

With chrony it was unstable as it can slew the local clock very
aggressively which made gpsd stop reporting the pulses.

> On Wed, 4 Nov 2009, Miroslav Lichvar wrote:
>
> > Most GPSs don't seem to even report anything
> > below seconds, so I'm wondering where does it come from.
>
> But many do.  Garmin has 5Hz models and many from other manufacturers
> go at a higher rate.  gpsd strives to support everything GPS.

The patch actually added support for 5Hz PPS. In ntpd only 1Hz PPS is
supported, but it should work with chrony if anyone is interested in
testing it.

Please consider applying the patch again (plus the one-liner from my
previous post), it really makes things simpler and more robust. If
there are any issues with the Zodiac driver I'm sure we can work it
out.

Thanks,

--
Miroslav Lichvar
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Eric S. Raymond-2]

Miroslav Lichvar <mlichvar@...>:
> Please consider applying the patch again (plus the one-liner from my
> previous post), it really makes things simpler and more robust. If
> there are any issues with the Zodiac driver I'm sure we can work it
> out.

Please post the entire, corrected patch so Gary can review it.  He
is the senior developer with expertise in this area and will make
the call, as I don't understand the proble m domain well enough to
do it myself.
--
                <a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Miroslav Lichvar]

On Thu, Nov 05, 2009 at 07:14:57AM -0500, Eric S. Raymond wrote:
> Please post the entire, corrected patch so Gary can review it.  He
> is the senior developer with expertise in this area and will make
> the call, as I don't understand the proble m domain well enough to
> do it myself.

Ok, it's in the attachment. It includes also the change to use the
fixed precision -20.
 
If it doesn't work correctly with the Zodiac driver, I believe all we
need is to adjust the magic value 1.1. Based on the information
provided by Håkan, i.e. that the messages should be associated with
the next pulse instead of previous, changing it to -0.4 might do the
trick.

Thanks,

--
Miroslav Lichvar

Index: gpsd.h-tail
===================================================================
--- gpsd.h-tail (revision 6417)
+++ gpsd.h-tail (working copy)
@@ -487,7 +487,7 @@
 extern int ntpshm_alloc(struct gps_context_t *);
 extern bool ntpshm_free(struct gps_context_t *, int);
 extern int ntpshm_put(struct gps_device_t *, double);
-extern int ntpshm_pps(struct gps_device_t *,struct timeval *);
+extern int ntpshm_pps(struct gps_device_t *,struct timeval *, int rate);
 
 extern void ecef_to_wgs84fix(/*@out@*/struct gps_data_t *,
      double, double, double,
Index: libgpsd_core.c
===================================================================
--- libgpsd_core.c (revision 6417)
+++ libgpsd_core.c (working copy)
@@ -183,34 +183,38 @@
     duration = timediff(tv, pulse[state == 0]);
 #undef timediff
 
-    if (cycle > 199000 && cycle < 201000 ) {
+#define min_interval(center) ((center) * 9 / 10)
+#define max_interval(center) ((center) * 11 / 10)
+#define in_interval(x, center) ((x) > min_interval(center) && (x) < max_interval(center))
+
+    if (in_interval(cycle, 200000)) {
  /* 5Hz cycle */
  /* looks like 5hz PPS pulse */
- if (duration > 45000)
-    (void)ntpshm_pps(session, &tv);
+ if (duration > min_interval(160000))
+    (void)ntpshm_pps(session, &tv, 5);
  gpsd_report(LOG_RAW, "5Hz PPS pulse. cycle: %d, duration: %d\n",
  cycle, duration);
-    } else if (cycle > 999000 && cycle < 1001000 ) {
+    } else if (in_interval(cycle, 1000000)) {
  /* looks like PPS pulse or square wave */
- if (duration > 499000 && duration < 501000
+ if (in_interval(duration, 500000)
 #if defined(NMEA_ENABLE) && defined(GPSCLOCK_ENABLE)
   && session->driver.nmea.ignore_trailing_edge
 #endif /* GPSCLOCK_ENABLE */
   ) {
     /* looks like 1.0 Hz square wave, ignore trailing edge */
     if (state == 1) {
- (void)ntpshm_pps(session, &tv);
+ (void)ntpshm_pps(session, &tv, 1);
     }
- } else {
+ } else if (duration > min_interval(800000)) {
     /* looks like PPS pulse */
-    (void)ntpshm_pps(session, &tv);
+    (void)ntpshm_pps(session, &tv, 1);
  }
  gpsd_report(LOG_RAW, "PPS pulse. cycle: %d, duration: %d\n",
  cycle, duration);
 
-    } else if (cycle > 1999000 && cycle < 2001000) {
+    } else if (in_interval(cycle, 2000000)) {
  /* looks like 0.5 Hz square wave */
- (void)ntpshm_pps(session, &tv);
+ (void)ntpshm_pps(session, &tv, 1);
  gpsd_report(LOG_RAW, "PPS square wave. cycle: %d, duration: %d\n",
  cycle, duration);
     } else {
@@ -222,6 +226,10 @@
  }
  /*@ -boolint @*/
 
+#undef min_interval
+#undef max_interval
+#undef in_interval
+
  pulse[state] = tv;
     }
 
Index: ntpshm.c
===================================================================
--- ntpshm.c (revision 6417)
+++ ntpshm.c (working copy)
@@ -22,9 +22,6 @@
 #include <sys/ipc.h>
 #include <sys/shm.h>
 
-#define PPS_MAX_OFFSET 100000 /* microseconds the PPS can 'pull' */
-#define PUT_MAX_OFFSET 500000 /* microseconds for lost lock */
-
 #define NTPD_BASE 0x4e545030 /* "NTP0" */
 #define SHM_UNIT 0 /* SHM driver unit number (0..3) */
 
@@ -151,11 +148,11 @@
 #ifdef PPS_ENABLE
 /* put NTP shared memory info based on received PPS pulse */
 
-int ntpshm_pps(struct gps_device_t *session, struct timeval *tv)
+int ntpshm_pps(struct gps_device_t *session, struct timeval *tv, int rate)
 {
     struct shmTime *shmTime = NULL, *shmTimeP = NULL;
     time_t seconds;
-    double offset;
+    int usecs;
     long l_offset;
 
     if (session->shmindex < 0 || session->shmTimeP < 0 ||
@@ -168,39 +165,28 @@
 #ifdef S_SPLINT_S /* avoids an internal error in splint 3.1.1 */
     l_offset = 0;
 #else
-    l_offset = shmTime->receiveTimeStampSec - shmTime->clockTimeStampSec;
+    l_offset = tv->tv_sec - shmTime->receiveTimeStampSec;
 #endif
-    /*@ -ignorequals @*/
     l_offset *= 1000000;
-    l_offset += shmTime->receiveTimeStampUSec - shmTime->clockTimeStampUSec;
-    /*@ +ignorequals @*/
-    if (labs( l_offset ) > PUT_MAX_OFFSET) {
+    l_offset += tv->tv_usec - shmTime->receiveTimeStampUSec;
+
+    if (l_offset > 1000000 / rate || l_offset < 0) {
         gpsd_report(LOG_RAW, "ntpshm_pps: not in locking range: %ld\n"
  , (long)l_offset);
  return -1;
     }
-    /*@ -ignorequals @*/
 
-    if (tv->tv_usec < PPS_MAX_OFFSET) {
- seconds = (time_t)tv->tv_sec;
- offset = (double)tv->tv_usec / 1000000.0;
-    } else {
- if (tv->tv_usec > (1000000 - PPS_MAX_OFFSET)) {
-    seconds = (time_t)(tv->tv_sec + 1);
-    offset = 1 - ((double)tv->tv_usec / 1000000.0);
- } else {
-    shmTimeP->precision = -1; /* lost lock */
-    gpsd_report(LOG_INF, "ntpshm_pps: lost PPS lock\n");
-    return -1;
- }
-    }
+    usecs = shmTime->clockTimeStampUSec + 1000000 / rate;
+    usecs -= usecs % (1000000 / rate);
+    seconds = shmTime->clockTimeStampSec + usecs / 1000000;
+    usecs %= 1000000;
 
     shmTimeP->count++;
     shmTimeP->clockTimeStampSec = seconds;
-    shmTimeP->clockTimeStampUSec = 0;
+    shmTimeP->clockTimeStampUSec = usecs;
     shmTimeP->receiveTimeStampSec = (time_t)tv->tv_sec;
     shmTimeP->receiveTimeStampUSec = (int)tv->tv_usec;
-    shmTimeP->precision = offset != 0 ? (int)(ceil(log(offset) / M_LN2)) : -20;
+    shmTimeP->precision = -20;
     shmTimeP->count++;
     shmTimeP->valid = 1;
 

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev

[Gary E. Miller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yo Miroslav!

On Thu, 5 Nov 2009, Miroslav Lichvar wrote:

> Ok, it's in the attachment.

I'll take a look at it.  Since the PPS used to work fine for a large
number of users I will look first at what broke it as opposed to how
to make any serious changes.  Eric wants to get 2.40 out and at this
stage we need to just rifle shoot things.

> It includes also the change to use the
> fixed precision -20.

No way we are going to get -20 out of any gpsd PPS!  In any case that
is just a starting point and ntpd will calculate the observed precision
regardless of what we tell it.  There is NO POINT setting the precision
in gpsd. As the gpsd code already notes:

    /* setting the precision here does not seem to help anything, too
       hard to calculate properly anyway.  Let ntpd figure it out.
       Any NMEA will be about -1 or -2.
       Garmin GPS-18/USB is around -6 or -7.
    */

- From the ntpd doc:

        "In NTP precision is determined automatically, and it is
        measured as a power of two.

You guys need to read the archive for WHY the ntpshm code is the way
it is before trying to improve it.

RGDS
GARY
- ---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97701
        gem@...  Tel:[phone]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQFK9KegBmnRqz71OvMRAv7zAJ9neDCUlduALxj9uaESS9oEeY/fWgCfQqjl
ztn56EeVGY8dFxxMC8R6DbI=
=Meyl
-----END PGP SIGNATURE-----

_______________________________________________
Gpsd-dev mailing list
Gpsd-dev@...
https://lists.berlios.de/mailman/listinfo/gpsd-dev