search

WKHarmon / led-sectional

MIT License
35 stars 20 forks source link

Improved XML parsing. #7

Closed vprzebinda closed 4 years ago

vprzebinda commented 4 years ago

Better handling when metar doesn't contain flight category ie when SPECI is released. Now we look back up to 3 hours to get a flight category and use the newest available metar with a flight category.

WKHarmon commented 4 years ago

Oh, hah ... someone who actually knows what they're doing looked at the code. Sweet. I ran out of Wemos modules and am waiting for some more, so I'll test and merge as soon as I get them.

vprzebinda commented 4 years ago

lol. Free wifi at my house! thanks for the heads up. -v

On Tue, Apr 28, 2020 at 2:14 PM Kyle Harmon notifications@github.com wrote:

@WKHarmon requested changes on this pull request.

You should probably remove your WiFi credentials, although the cat is mostly out of the bag now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/WKHarmon/led-sectional/pull/7#pullrequestreview-402091730, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNW6TFTVXEMVJDQA5PEMCTRO4MGTANCNFSM4MS7Y7WA .

WKHarmon commented 4 years ago

@vprzebinda Hola! I know it's been a long time since you submitted this, but I haven't had a ton of time to work on it. Sorry for that. I've merged in most of your XML parsing changes and published a branch here: https://github.com/WKHarmon/led-sectional/tree/improved-parsing

This keeps most of the changes you made, but reimplements the light sensor, slightly changes the calculation for the the lightning LEDs, allows for a key, and reverts to most of the defaults that were in the original file (e.g. LED type, etc.)

I wasn't able to merge your more recent commits because it doesn't allow for an airport to appear on the list twice (some people have an inset that zooms in on a region that appears elsewhere on the map), and it makes it doesn't support a key (although that shouldn't be too hard to add). It also sends the key LEDs and NULL airports where there's a blank LED in the API request, but that doesn't really seem to hurt anything so it doesn't really matter.

I'm going to test this for a few weeks then merge it in, but if you wanted to look over it with my changes I'd welcome any feedback. When I have time I'll try to add the later commits in a way that doesn't break the other functionality, but I wanted to go ahead and push the earlier ones.

Thanks!

WKHarmon commented 4 years ago

I went ahead and merged the modified branch into master.