RE: Quectel FW version - Merge Request

[Jared Hagel]

Hi Gary,

> Odd,  you should be able to fork gpsd into your own GitLab account.
> But I admit to being too lazy to do that.  What is your GitLab handle?  I'll 
> add it to gpsd project.

I'm not sure what you mean by handle. Here is some information about my GitLab 
account. Is this enough? 
User ID: 9572988
Full name: Jared Hagel
Email: jared.hagel@vontas.com

> I'm not seeing the requested raw data log.  With that I can get my own debug 
> output.

Unfortunately I'm not familiar with this process. From browsing around, it 
looks like I can use gpspipe to get the raw data log (gpspipe -R). To do this, 
would I start gpsd, then quickly after start gpspipe? The trouble I see here is 
that if I don't start gpspipe quickly enough I could miss the first bytes 
received from the GPS receiver. 

> Codacy does not like the use of strlen(), use strnlen() instead.  Yes, the 
> existing code has existing strlen() and that is slowly getting replaced.  New 
> code must meet new Codacy requirement.

Changed to use sizeof. Let me know what you think.

> gpsd does not support undocuemnnted commands.  Please document it.

I've just finished a bunch of digging, and I don't think this command is 
documented ☹ Unfortunately this command is necessary to get the Quectel version 
however (see 
https://github.com/andrethemac/L76GLNSV4/issues/9#issuecomment-603474459 "The 
person responding has been a little hard to follow, but they seem to be saying 
we need to use an undocumented PQVERNO command"). I hope this doesn’t prevent 
this change from getting merged in.... I can email Quectel to see if they can 
provide me documentation for this command, and if they're OK with including 
this documentation in GPSD comments. Would you like me to do this?

> Check that subtype1 is empty first.

Done

I attached an updated patch.diff. Let me know what you think. Thanks!
Jared



-----Original Message-----
From: gpsd-dev <gpsd-dev-bounces+jared.hagel=vontas.com@nongnu.org> On Behalf 
Of Gary E. Miller
Sent: Friday, March 4, 2022 4:34 PM
To: gpsd-dev@nongnu.org
Subject: Re: Quectel FW version - Merge Request

Yo Jared!

On Fri, 4 Mar 2022 23:13:56 +0000
Jared Hagel <jared.hagel@vontas.com> wrote:

> I tried to make this a merge request, but I got this following 
> 'Rejected' email when I tried to do so. I'm not sure what I did wrong.

Odd,  you should be able to fork gpsd into your own GitLab account.
But I admit to being too lazy to do that.  What is your GitLab handle?  I'll 
add it to gpsd project.

> Anyways, as requested I modified the lines of code I added to be less 
> than 80 characters and attached debug output.

I'm not seeing the requested raw data log.  With that I can get my own debug 
output.

> You can see the text
> 'PQVERNO' in this debug output. If you let me know which code I added 
> is insecure I can fix that as well.

Codacy does not like the use of strlen(), use strnlen() instead.  Yes, the 
existing code has existing strlen() and that is slowly getting replaced.  New 
code must meet new Codacy requirement.

        {"PMTK", NULL, 3,  false, processMTK3301},
        {"PQVERNO", NULL, 3,  false, processMTK3301},

Do not use the same decode function for 2 different messages.  This confuses 
the start/end cycle detectors.  Once again, there are existing similar usages, 
but those are slowly being fixed.

field[2] is used before you have checked it contains data:

        (void)strlcpy(session->subtype1, session->nmea.field[2], 

As you said:

        // "$PQVERNO is an internal command and used to query Quectel FW 
        // version. We haven’t added this internal command in GNSS
        // protocol spec."

gpsd does not support undocuemnnted commands.  Please document it.

Potentially the data is queried even if gpsd already has it:

        (void)nmea_send(session, "$PQVERNO,R");

Check that subtype1 is empty first.

RGDS
GARY
---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703
        gem@rellim.com  Tel:+1 541 382 8588

            Veritas liberabit vos. -- Quid est veritas?
    "If you can't measure it, you can't improve it." - Lord Kelvin

patch.diff
Description: patch.diff