SignalK / nmea0183-signalk

NMEA0183 to Signal K parser
Other
32 stars 48 forks source link

Missing Magnetic Variation in GPRMC #22

Closed bobmor99 closed 9 years ago

bobmor99 commented 9 years ago

nmea0183-signalk crashes when processing GPRMC messages that are missing Magnetic Variation e.g. $GPRMC,171302,A,3011.8466,N,08123.1192,W,0.5315,22.551,190215,,*08

  ~/signalk-server-node$ bin/nmea-from-file 
signalk-server running at 0.0.0.0:3000
TypeError: Cannot call method 'toUpperCase' of undefined
    at NMEA0183.magneticVariaton (/home/bobmor99/signalk-server-node/node_modules/nmea0183-signalk/lib/NMEA0183.js:116:15)
    at null._decoder (/home/bobmor99/signalk-server-node/node_modules/nmea0183-signalk/codecs
... (keeping it brief)

No more crashes when I comment out the following line in signalk-server-node/node_modules/nmea0183-signalk/codecs/RMC.js

{ path: 'magneticVariation', value: this.magneticVariaton(values[9], values[10]) },

My data source is gpspipe reading a BU-353 USB GPS.

bobmor99 commented 9 years ago

Sorry, inadvertant markdown invocation. :-(

fabdrol commented 9 years ago

@bobmor99 you can make code highlighting work by wrapping code in backticks (one for inline, three for multiline).

I'll push a fix for the issue, and investigate if this is an issue in more codecs when I have more time!

keesverruijt commented 9 years ago

Or indent by four spaces...

this is source code

Don't forget the "preview" option, and that you can edit comments.

fabdrol commented 9 years ago

@bobmor99 could you update to the latest version and try again?

bobmor99 commented 9 years ago

Hi Fabian,

Short answer: Yes, your new RMC.js works.

Long answer (with questions):

Some more basics for me... Believe it or not, I started programming in the '80s. Maybe that's the problem... too old. lol

I went to https://github.com/SignalK/signalk-parser-nmea0183, downloaded the ZIP, unzipped, and ended up with a folder named signalk-parser-nmea0183-master.

I moved that folder to signalk-server-node/node_modules, renamed the nmea0183-signalk folder to nmea-signalk-old and then renamed signalk-parser-nmea0183-master to nmea0183-signalk.

Next, I ran bin/nmea-from-gpsd (which had worked with my hacked/patched version of codecs/RMC.js). This crashed because ~/signalk-server-node/node_modules/nmea0183-signalk/node_modules/ggencoder is missing. ~/signalk-server-node/node_modules/nmea0183-signalk/node_modules is missing. FWIW, there is a bin directory that is not in the older ~/signalk-server-node/node_modules/nmea0183-signalk directory.

So, my basic questions are: 1) Is this the normal way to install a patch? I could have easily just replaced the old RMC.js with the new version but then I wouldn't be sure if the fix to Issue 22 involved other pieces. 2) Can I assume that zipped "master" version on GitHub is usually the "good stuff"?

Sorry for being lazy yesterday with markdown and git pull. My brain was exhausted. I have to choose carefully what I focus on. :-)

--Bob

On Sat, Feb 21, 2015 at 6:04 AM, Fabian Tollenaar notifications@github.com wrote:

@bobmor99 https://github.com/bobmor99 could you update to the latest version and try again?

— Reply to this email directly or view it on GitHub https://github.com/SignalK/signalk-parser-nmea0183/issues/22#issuecomment-75366949 .

tkurki commented 9 years ago

@bobmor99: The dependencies between packages in the Node world are defined in package.json.

Server's package.json specifies "nmea0183-signalk": "^0.2.2", but the dependency can be specified as "master branch in Github" by changing that to "nmea0183-signalk": "signalk/signalk-parser-nmea0183", After that npm install should install the master version where @fabdrol's fix is. If you want to make sure you can remove node_modules/nmea0183-signalk.

https://github.com/SignalK/signalk-server-node/blob/master/package.json#L56

bobmor99 commented 9 years ago

The fix for issue #22 works. Thanks Fabian.