search

dotMorten / NmeaParser

Library for handling NMEA message in Windows Desktop, Store, Phone, Universal, and Xamarin (Android + iOS), coming from files, bluetooth, serial port or any stream
https://dotmorten.github.io/NmeaParser/
Apache License 2.0
262 stars 89 forks source link

Interfaces to aid with consumption of common NMEA data #19

Closed SteveK-GS closed 8 years ago

dotMorten commented 8 years ago

Thank you for your PR. However, please open issues for API design discussion first prior to submitting large architectural changes.

dotMorten commented 8 years ago

Going through the code review of this, I'm not really sold on the idea of these interfaces. The benefit isn't really clear to me. The NMEA parser is very much about just parsing the messages as is, and exposing the values according to the spec. Having shared interfaces for properties that aren't necessarily the same thing on different message isn't clear, and the interfaces aren't even reused that much.

What does it mean to have ILatLng on a message? Lat/long for what? There's more than current location, so you wouldn't really write code just against the interface for any message type. For instance a laser range finder would return the location you're pointing add, while the GPS for where you're standing, and a waypoint for where you're headed, so if you rely on this interface to get a location, you'll think your location jumps around.

dotMorten commented 8 years ago

@SteveK-GS Btw sorry to be so tough on you in my PR reviews. I really don't want to discourage you contributing to open source projects. Keep up the good work!