• Hopefully I've covered the question of memory usage in the other post...

    I'm afraid I don't have that much time at the moment, so can't give you a very in-depth reply...

    Returning just {line:...} was actually a tweak that was made when I was trying to debug some issues that someone was having. Originally nothing was returned.

    Moving to a more object oriented approach would be better I guess - it would push the memory usage up but it might be worth it.

    I'm not convinced about your approach with the array. To me, it is slower and uses more memory in all cases, and users have to understand about the array and structure their code around it (even if it's not what they want to do). Surely it is much better to have them define an array themselves if they want that behaviour.

    Same with enabled. Surely it's better to handle that in your own code if you need it? I can understand that a user might want to totally remove the listener from the Serial port, but that's another modification.

    What about (not tested):

    function GPS(serial,callback) {
      this.serial = serial;
      this.line = "";
      this.start(callback);
    }
    
    GPS.prototype.lineHandler = function (line) {
      var tag = line.substr(3,3);
      if (tag=="GGA") {
        var d = line.split(",");
        var dlat = d[2].indexOf(".");
        var dlon = d[4].indexOf(".");
        return {
          time : d[1].substr(0,2)+":"+d[1].substr(2,2)+":"+d[1].substr(4,2),
          lat : (parseInt(d[2].substr(0,dlat-2),10)+parseFloat(d[2].substr(dlat-2))/60)*(d[3]=="S"?-1:1),
          lon : (parseInt(d[4].substr(0,dlon-2),10)+parseFloat(d[4].substr(dlon-2))/60)*(d[5]=="W"?-1:1),
          fix : parseInt(d[6],10),
          satellites : parseInt(d[7],10),
          altitude : parseFloat(d[9])
        };
      }
    }
    
    GPS.prototype.start = function(callback) {
     var gps = this;
     this.serial.on('data', function(data) {
        gps.line += data;
        var idx = gps.line.indexOf("\n");
        while (idx>=0) {
          var line = gps.line.substr(0, idx);
          gps.line = gps.line.substr(idx+1);
          var gpsData = gps.lineHandler(line); 
          if (gpsData && callback) callback(gpsData);
          idx = gps.line.indexOf("\n");     
        }
        if (gps.line.length > 80)
          gps.line = gps.line.substr(-80);
      });
    }
    
    GPS.prototype.stop = function() {
     this.serial.removeAllListeners('data');
    }
    
    exports.connect = function(serial, callback) {
      return new GPS(serial, callback);
    }
    
    // then normal:
    require("GPS").connect(Serial2, function(data) {
     ...
    });
    // or advanced
    require("GPS").connect(Serial2);
    GPS.prototype.lineHander = function(line) {
      // do whatever you want
    };
    // or advanced while keeping the old handler and callback pattern
    require("GPS").connect(Serial2, function(data) {
    });
    GPS.prototype.oldHander = GPS.prototype.lineHander;
    GPS.prototype.lineHander = function(line) {
      if (iCanHandleLine) {
       return { my:Data };
      } else return this.oldHandler(line);
    };
    

    I believe this will use a bit more memory, but it has a few advantages:

    • It's in the 'suggested' form for writing modules
    • You can now properly start and stop the GPS module - so no processing is done when a line comes in
    • Because functions are not defined in the module scope, when you say GPS.prototype.lineHander = ... it will overwrite the old one, freeing its memory.
    • When you override lineHandler you get full control over everything
    • You can still 'keep' lineHandler if you want to use it yourself.
    • You don't have to use the callback pattern for handlers if you don't want to
    • You can create an 'AdvancedGPS' module pretty easily

    What do you think?

About

Avatar for Gordon @Gordon started