Changed $RPO sentence parsing strategy. Fixes #46

[iglesiasmarianod]

Parser filled the three vector positions for polar coefficients a,b and c with the first value received in the NMEA sentence. Changed parsing strategy from strtok() to using strtof().

[linuxianer99]

What does the WIP in the commit message mean ??

Can you please add are more informative commit message ?? What was the problem, and what is changed, that fixes the problem ?

[iglesiasmarianod]

Hi @linuxianer99 WIP means Work In Progress.

What I found is that for an LS4, around 325kg mass and MC setting of 2, Speed Command sound wanted me to fly at less than 90 km/hr when it should be around 140km/hr. Running Variod with -d2 option on the foreground this is the output I get.

Message from XCSoar: $POV,C,RPO,0.002732,-0.126000,2.077936*57
Get Polar RPO: 0.000211, 0.000759, 0.002732

XCSoar sends polar data in m/s for both, vertical speed and horizontal speed. Variod converts units when parsing from m/s to km/hr, dividing the first parsed item, "a" coefficient, by (3.6x3.6) and "b" , the second coefficient by 3.6. "c" coefficient, is left as is.

If I multiply 0.000211 x 3.6 x 3.6 = 0.002732, 0,000759 x 3,6 = 0,002732 and C left as is 0,002732. As you can see, only the first NMEA sentence value is being parsed and it fills the three polar coefficients "a", "b" and "c".

I believe the error is the usage of strtok() The first time strtok() is run should be passed a string pointer and a delimiter. Subsequent calls should be passed NULL pointer and delimiter so it can tokenize the remaining string.

pointer=strtok(str,delim)
while(pointer!=NULL){
     pointer=strtok(NULL,delim)
}

Using strtok() this way works

bool read_float_from_sentence(int n,float fv[],char *str,const char *delim)
{
    char *vp;
    if (n > NUM_FV) return false;
    for (int i=0; i < n; i++) {
        if(i==0){
            vp = strtok(str,delim);
        }
        else{
            vp = strtok(NULL,delim);
            }
        fv[i] = strtof(vp,NULL);
        if (fv[i]==0.0f){
            return false;
        } 
    }
    return true;
}

Since strtof() returns a pointer to the first character of the string after the conversion, I thought it was worth using it instead of strtok(). I'm not strongly opinioned about using strtok() or strtof(). If you have any preference please let me know and I'll fix the commit.

Thanks for your time!

[linuxianer99]

Hey @iglesiasmarianod , It's fine for me! Just want to understand about the problem.

When do you think the work ist done ?? So it makes no sense to merge, if it still WIP, right ?

[iglesiasmarianod]

Hi @linuxianer99 don't worry, I know my commit messages are somewhat cryptic and more explanation is needed. I'll try to improve that. I'd like to fly with this modification before merging just in case I broke something or there's another problem I don't see on the ground. I'd like to remove the unused "delim" variable too. I think next week I might be able to fly and test. If everything is OK we can merge. Is that OK for you?

Thanks again!

[linuxianer99]

That's fine for me !! Another way of doing the WIP will be, to put the PR into DRAFT state.

[iglesiasmarianod]

Hi @linuxianer99 I'll use the DRAFT state next time, thanks! I flight tested yesterday and speed command seems to be working now. I removed the unused "delim" variable from the parsing function in my last commit. If you think it is OK to merge go ahead. I did not see any regression so far.