Re: [gpsd-dev] PPS from position hold / time-only modes

[Håkan Johansson]

Hi Gary,

On Mon, 14 Mar 2016, Gary E. Miller wrote:

> Yo Håkan!
>
> On Sun, 13 Mar 2016 13:00:46 +0100
> Håkan Johansson <address@hidden> wrote:
>
> > This is thwarted by commit 82d4eb48 (in all_reports() in gpsd.c)
> > which does not pass the fix-time to the pps thread if there is no
> > fixcnt. The basically same check is also performed in report_hook()
> > in timehint.c, but against PPS_MIN_FIXES instead of 0.
> >
> > (Removing these two checks, the PPS pulses from the oncore device
> > make it through to chrony.)
>
> Ugh, please no.  Many GPS put out unsyncronized PPS and the only
> way we know the PPS is valid on most GPS is that we have 3 or more
> valid fixes just before the PPS.

Ahh, sorry.  Was not a suggestion, just showing where the issue is.

> > /*
> >   * The PPSTIME_IS mask bit exists distinctly from TIME_SET exactly
> >   * so an OnCore running in time-service mode (and other GPS clocks)
> >   * can signal that it's returning time even though no position fixes
> >   * have been available.
> >   */
> >
> > which would imply that PPSTIME_IS actually means that we have a good
> > time value despite having no position fix reported.
>
> Sadly that is not the assumption much of the code makes.
>
> > Before going to the solution, I believe the two checks in
> > report_hook() in timehint.c inside the
> >
> > /* FIXME! these two validations need to move back into ppsthread.c */
> >
> > are duplicates of the (longer) check block in gpsd.c,
>
> If they were duplicates then I would not have added the comment.
>
> > (Saving the comment in gpsd.c and also using
> > PPS_MIN_FIXES instead of 0 for the check.)
>
> You can't just use PPS_MIN_FIXES, you need positive indication from the
> GPS that the PPS is good, in this cycle.
>
> > A flag with the meaning 'time is available and good despite no
> > position fix' is required, such that it can overrule the check on
> > fixcnt.
>
> Possibly, but how would you set that flag?

In the driver.

> > Question would be if PPSTIME_IS in its current sense is useful and
> > should be kept as is and a new flag introduced, or just repurposed.
>
> gpsd uses flags all over the place, somewhat inconsistently.  Every time
> we repurpose a flag bad things happen.  Coders tend to trust their
> memory on flags, and do not bother to hunt down every use, every time, when
> then patch things.  So we should use a new flag.
>
> > Personally, I would rather go for a one-more-flag solution, as there
> > could be a use to mark that the time with a fix is good enough for
> > PPS purposes. The garmin driver seems to use it in that way (and thus
> > itself also duplicate the fixcnt check....).
>
> The Garmin spec requires 3 valid fixes before using PPS.  So that is the
> primary Garmin requirement.  The PPSTIME_IS is the unneeded graft on.

I have attached a patch which introduces a new flag GOODTIME_IS.  This 
would not be used by e.g. garmins, which need to check that they have 
these good fixes.  But is used by e.g. the OnCore to override the 
common checks on fixcnt.

Perhaps some other name than GOODTIME?  TIME_EVEN_IF_NO_FIX but that is a 
bit too long...

Cheers,
Håkan

0004-Introduce-the-flag-GOODTIME_IS-to-mark-that-the-devi.patch
Description: Text Data