Re: [gpsd-dev] [PATCH] Support UBX NAV-PVT

[Fred Wright]

(Back on-list since some issues might be of wider interest)

On Thu, 20 Jul 2017, Clark Li wrote:

> Thanks Fred. I've sent out another one with log/check for NAV-PVT from 
> ublox-neo-m8n.

Previously, I hadn't bothered to try to apply your patch after seeing the
issue I reported, but I've now tried it and it (either version) fails to
apply.  There seems to be some inconsistency between the state of
driver_ubx.c and what the patch expects.  Ordinarily this might be due to
the patch's being based on a different revision of the file, but in this
case the blob ID in the patch matches the state of the file, so I'm
puzzled.  Did you have uncommitted changes to the file at the time you ran
git format-patch?  I don't know what the effect of that is, but it's my
best guess as to why the patch is bad.

> Changes for ublox-aek-4t.log.chk is actually for the processing fTOW in 
> driver_ubx.c

Except that if you look closely at the diff, you'll see that the only
actual change to that file is the introduction of some spurious line
breaks.  It makes sense that the output is effectively unchanged, but it
doesn't make sense that some spurious newlines appeared.  Did you perhaps
obtain the "updated" version via copy-n-paste rather than from
"regress-driver -b"?

It's also odd that only that one case was affected by whatever you did.
Did you only run the one test case, rather than all of them (typically via
"scons check")?

With regard to the new test data, did you do some form of sanity check on
it?  All the regression tests can do is verify that the code continues to
produce the same results as before; they have no built-in concept of
"correct" results.  I don't know the specific details, but since it looks
like you're adding support for a new message type that duplicates the
functionality of an older message type, a possible approach would be to
configure the receiver to output both and then verify that both the old
and new versions of the code come up with the same (or possibly better)
TPV decodes.  Though if supporting both results in duplicate TPV outputs,
some tweaking would be needed to be able to compare them.

Incidentally, when updating regression-test data, a fairly convenient
approach (if one is working in a git repo) is just to run "scons
gps-makeregress" and then let git tell you what actually changed (and see
if it's plausible).  That lets you view the diffs much more flexibly than
looking at the test output from gps-regress.

Fred Wright