Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect RINEX v2 ionosphere parameter parsing #241

Merged
merged 2 commits into from
May 1, 2024

Conversation

fedosgad
Copy link
Contributor

@fedosgad fedosgad commented Apr 29, 2024

In #239 I made a mistake in parsing code for IONO ... header strings:
String format is 2X,4D12.4 (https://files.igs.org/pub/data/format/rinex211.txt, table A3), which means there must 2 empty characters at the beginning of the line. In my code, I skip first 4 characters instead of 2, which can lead to errors in f64::from_str further down the line. Unfortunately, existing tests were incorrect too, so mistake hasn't been caught early.

I fixed parsing code and added more tests from credible sources.
Sorry for the bug.

@gwbres
Copy link
Collaborator

gwbres commented Apr 30, 2024

Hello, no worries ! I'll look into this tomorrow

there must 2 empty characters at the beginning of the line

Yeah this happens everywhere in RINEX.. Although I don't understand why the previous test did not catch any problem

I guess it all depends whether we want the function to accept the entire line, or something prepared.
I think it's easier, especially when working with the header section, to process the complete line in the function, hence the trap we might fall in here

@gwbres gwbres self-assigned this Apr 30, 2024
@gwbres gwbres added the bug-fix Fix proposal label Apr 30, 2024
@gwbres gwbres merged commit 19e82c7 into georust:main May 1, 2024
4 checks passed
@gwbres
Copy link
Collaborator

gwbres commented May 1, 2024

I'm thinking we should maybe host a complete PPP set in V2, like we do in V3, that would be the best thing to do to test all functionalities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix Fix proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants