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 problem with enl extensions #29

Merged
merged 10 commits into from Mar 31, 2018
Merged

Fix problem with enl extensions #29

merged 10 commits into from Mar 31, 2018

Conversation

GliderGeek
Copy link
Collaborator

@GliderGeek GliderGeek commented Mar 20, 2018

The current IGC reader does not correctly parse the record extensions. This branch exposes this bug and fixes it.

processed_b_record = LowLevelReader.process_B_record(decoded_b_record, fix_record_extensions)

assert 'ENL' in processed_b_record
assert expected_enl == processed_b_record['ENL']
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be asserting against a constant here to make sure we don't screw up our own test logic here. if I read correct 00128 and 00196 are the two altitudes and after that are the extensions. the first extension is FXA with 006 and ENL is second with 001 which is 1 as an integer.

in other words, I think this should just be:

assert processed_b_record['ENL'] == 1

@Turbo87 Turbo87 added the bug label Mar 23, 2018
Copy link
Owner

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work, thanks! :)

@Turbo87
Copy link
Owner

Turbo87 commented Mar 30, 2018

oh, looks like we have a small merge conflict now 😞

@GliderGeek
Copy link
Collaborator Author

Any action from me needed?

@Turbo87 Turbo87 merged commit caa7b77 into Turbo87:master Mar 31, 2018
@Turbo87
Copy link
Owner

Turbo87 commented Mar 31, 2018

nope, just needed to wait until CI passed and then forgot to check again 🙈

@GliderGeek
Copy link
Collaborator Author

I know in gitlab there’s an automated merge when pipeline succeeds functionality. Would be nice if github has this as well

@Turbo87
Copy link
Owner

Turbo87 commented Mar 31, 2018

totally, yeah... 😞

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

Successfully merging this pull request may close these issues.

None yet

2 participants