[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [gpsd-dev] [PATCH] Support UBX NAV-PVT
From: |
Clark Li |
Subject: |
Re: [gpsd-dev] [PATCH] Support UBX NAV-PVT |
Date: |
Tue, 25 Jul 2017 13:39:18 +0000 |
>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.
That last patch was not based on the very latest in master branch. But I guess
forwarding the output of git-send-email from gmail to outlook should also be
blamed... I am attaching the original git-format-patch output this time,
hopefully this one can be applied without conflicts.
>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"?
The chk file was updated using "regress-driver -b" but again this might be due
to the email forwarding...
>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")?
scons check was run to check all the regression tests. ublox-aek-4t.log might
be the only regression that contains NAV-NAV-SOL.
>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.
Yes sanity check has been done on for positon/speed/time when the log was
recorded.
>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.
TPV for NAV-PVT provides extra 'epx', 'epy' and 'epv' fields while NAV-SOL does
not. I assume you meant 'more fields' by 'better TPV decodes', as positional
accuracy depends on the actual GPS configuration and its internal calculation.
>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.
Thanks for the extra tips!
-----Original Message-----
From: Fred Wright [mailto:address@hidden
Sent: Saturday, July 22, 2017 8:41 AM
To: GPSD Developers List
Cc: Clark Li
Subject: RE: [gpsd-dev] [PATCH] Support UBX NAV-PVT
(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
______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________
This communication may contain confidential or copyright information. If you
are not an intended recipient, you must not keep, forward, copy, use, save or
rely on this communication, and any such action is unauthorized and prohibited.
If you have received this communication in error, please reply to this email to
notify the sender of its incorrect delivery and then delete both the original
message and your reply.
0001-Support-UBX-NAV-PVT.patch
Description: 0001-Support-UBX-NAV-PVT.patch