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

Update Metar.php #965

Merged
merged 2 commits into from
Dec 21, 2020
Merged

Update Metar.php #965

merged 2 commits into from
Dec 21, 2020

Conversation

FatihKoz
Copy link
Contributor

The reported pressure was always being assigned as hPa thus resulting display and conversion errors when the reported pressure was inHg.

Also the VFR/IFR (VMC/IMC) determination was using minimum 5 nmi and 3000 ft cloud base, it must be 5 km and 3000 ft.

With this state, this PR should fix issue #948 and partly fixes issue #963

The reported pressure was always being assigned as hPa thus resulting display and conversion errors when the reported pressure was inHg.

Also the VFR/IFR (VMC/IMC) determination was using minimum 5 nmi and 3000 ft cloud base, it must be 5 km and 3000 ft.

With this state, this PR should fix issue #948 and partly fixes issue #963
@FatihKoz
Copy link
Contributor Author

u1
u2

Conversion and view just works fine as expected.

MetarTest must be checked for inHg and hPa logic. As any reported pressure with Annnn is inHg and Qnnnn is hPa. But as far as I see on the metarTest1 the units are wrong. Did not wanted to commit a change on the test.

@nabeelio
Copy link
Owner

The tests need to be fixed as part of this

@FatihKoz
Copy link
Contributor Author

Do you approve the change of VFR to VMC and IFR to IMC , I can do that too ... It is kind of habit thing , technically they are VMC/IMC but on everyday use people tend to say VFR/IFR too often :)

Will send the MetarTest commit soon btw

@nabeelio
Copy link
Owner

I would leave it as VFR/IFR, which's normally used... that's also how I learned it in my PPL classes. IIRC, there is a difference between VMC and VFR. I'll have to look it up in more detail but I would leave that as-is. I will think about turning that into a translation maybe.

@nabeelio nabeelio merged commit a5513b6 into nabeelio:dev Dec 21, 2020
@nabeelio nabeelio added the bug label Dec 21, 2020
@nabeelio nabeelio added this to the 7.0.0 milestone Dec 21, 2020
@FatihKoz FatihKoz deleted the patch-3 branch December 21, 2020 21:59
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.

2 participants