Re: [gpsd-dev] [PATCH] [RFC] gpsmon: add PPS monitoring at -D2 debug level

[Gerry Creager - NOAA Affiliate]

I can accept Greg's rationale for -D2, but I consider the PPS message something that should be in the startup debug data, if PPS is present. 

Greg, in the olden days of GPS, the debate on rising vs. falling edge was heated. I've always preferred rising edge as it was generally more reliably generated there, from what I recall. 

On Fri, Nov 1, 2013 at 11:13 PM, Gerry Creager - NOAA Affiliate <address@hidden> wrote:

I would prefer to see the messages at any debug level rather than -d2. I'm not currently competent to comment on the first two. I've gotta spend some quality time with the code.

gerry

On Fri, Nov 1, 2013 at 11:08 PM, Beat Bolli <address@hidden> wrote:

This is a request for comment for adding PPS output to gpsmon, as seen
on Eric's TODO list.

The patch works, but has two issues, IMO. Both are marked with "FIXME"
in the source:

- I'm not sure about updating device->last_fixtime where it is now. It
  works but maybe there's a better place?

- An allocated SHM segment has to be faked, otherwise
  pps_thread_activate() won't even start the thread.

A different issue is whether it's OK to output the PPS messages only at
-D2. Should they be visible at any debug level?

Comments are welcome!

Signed-off-by: Beat Bolli <address@hidden>
---
 gpsmon.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/gpsmon.c b/gpsmon.c
index b415560..7b50b8b 100644
--- a/gpsmon.c
+++ b/gpsmon.c
@@ -775,7 +775,10 @@ static void gpsmon_hook(struct gps_device_t *device, gps_mask_t changed UNUSED)
        (void)wnoutrefresh(packetwin);

     (void)doupdate();
-
+
+    /* Update the last fix time seen for PPS. FIXME: do this here? */
+    device->last_fixtime = device->newdata.time;
+
     if (logfile != NULL && device->packet.outbuflen > 0) {
         /*@ -shiftimplementation -sefparams +charint @*/
         assert(fwrite
@@ -784,7 +787,17 @@ static void gpsmon_hook(struct gps_device_t *device, gps_mask_t changed UNUSED)
         /*@ +shiftimplementation +sefparams -charint @*/
     }
 }
-   /address@hidden +nullpass address@hidden/
+/address@hidden +nullpass address@hidden/
+
+/* The PPS thread_report_hook callback. This or a pps_hook callback is
+ * needed to keep the PPS thread running.
+ */
+static char *pps_report(struct gps_device_t *session UNUSED,
+                       struct timeval *actual_tv UNUSED,
+                       struct timespec *ts UNUSED,
+                       double edge_offset UNUSED) {
+   return "";
+}

 static jmp_buf assertbuf;

@@ -961,6 +974,11 @@ int main(int argc, char **argv)
        }

        serial = true;
+
+       /* Setup PPS monitoring. */
+       session.shmTimeP = 0;   /* FIXME: fake one allocated SHM segment */
+       session.thread_report_hook = pps_report;
+       pps_thread_activate(&session);
     }
     /*@ +boolops */
     /*@ +nullpass +branchstate @*/
@@ -1051,6 +1069,11 @@ int main(int argc, char **argv)

   quit:
     /* we'll fall through to here on longjmp() */
+
+    /* Shut down PPS monitoring. */
+    if (serial)
+       (void)pps_thread_deactivate(&session);
+
     gpsd_close(&session);
     if (logfile)
        (void)fclose(logfile);
--
1.8.4.rc3

--

Gerry Creager

NSSL/CIMMS

405.325.6371

++++++++++++++++++++++

“Big whorls have little whorls,

That feed on their velocity; 

And little whorls have lesser whorls, 

And so on to viscosity.” 

Lewis Fry Richardson (1881-1953)

--

Gerry Creager

NSSL/CIMMS

405.325.6371

++++++++++++++++++++++

“Big whorls have little whorls,

That feed on their velocity; 

And little whorls have lesser whorls, 

And so on to viscosity.” 

Lewis Fry Richardson (1881-1953)