osqzss / gps-sdr-sim

Software-Defined GPS Signal Simulator
MIT License
2.62k stars 762 forks source link

corrected spelling mistakes in gpssim.c #277

Closed lenhart closed 3 years ago

lenhart commented 3 years ago

some minor spelling fixes in gpssim.c

Plus I discovered 2 things I wanted to ask about before modifying:

  1. while parsing options there is one break in a if conditional. is this deliberate? Line 1823:

    case 'T':
            timeoverwrite = TRUE;
            if (strncmp(optarg, "now", 3)==0)
            {
                time_t timer;
                struct tm *gmt;
    
                time(&timer);
                gmt = gmtime(&timer);
    
                t0.y = gmt->tm_year+1900;
                t0.m = gmt->tm_mon+1;
                t0.d = gmt->tm_mday;
                t0.hh = gmt->tm_hour;
                t0.mm = gmt->tm_min;
                t0.sec = (double)gmt->tm_sec;
    
                date2gps(&t0, &g0);
    
                break;  // <------------- this break. should be below } imo
            }

    Also I would suggest to mark all deliberate fall-through in switch cases with comments to prevent future errors. I can do that quickly if wanted.

  2. would code simplification be welcomed as PR?

    if (azel[1]*R2D > elvMask)
        return (1); // Visible
    // else
    return (0); // Invisible

    could be simplified to return (azel[1]*R2D > elvMask); //true if visible, false ow. that would be a bit simpler but now that I write it, I notice that I thought of returning a bool.. fct is only used once in an if statement though..

Thanks for the good work!

osqzss commented 3 years ago

Thanks for the corrections.

lenhart commented 3 years ago

Very welcome! Any comments on my question in the initial PR? (especially the break statement). I admit that it isn't the best place to ask unrelated questions in the PR, but I didn't want to open an issue for that question either..

osqzss commented 3 years ago

Thank you for your suggestions. The wrong break location in the parse options is now fixed. For the second suggestion, I'd rather leave it as it is.