Support non-spec message dividers (#57)

Parsing in genmsg leniently considers any line
starting with '---' to be a valid message delimiter.
Adopts this leniency to rosnodejs parsing.

See
https://github.com/ros/genmsg/blob/indigo-devel/src/genmsg/msg_loader.py#L459

Author: chris-smith

src/utils/messageGeneration/MessageSpec.js

@@ -165,12 +165,16 @@ class RosMsgSpec {
       fileContents = this._loadMessageFile(filePath);
     }
     if (fileContents !== null) {
-      this.fileContents = this._extractRelevantMessage(fileContents);
+      this.fileContents = fileContents;
 
-      this._extractFields(this.fileContents);
+      this._parseMessage(fileContents);
     }
   }
 
+  _parseMessage() {
+    throw new Error('Unable to parse message file for base class RosMsgSpec');
+  }
+
   /**
    * Generates the file data for this class
    */
@@ -218,7 +222,89 @@ class RosMsgSpec {
   }
 
   /**
-   * TODO: move this to MsgSpec? Really only makes sense there...
+   * For this message spec, generates the text used to calculate the message's md5 sum
+   * @returns {string}
+   */
+  getMd5text() {
+    return '';
+  }
+
+  /**
+   * Get the md5 sum of this message
+   * @returns {string}
+   */
+  getMd5sum() {
+    return md5(this.getMd5text());
+  }
+
+  /**
+   * Generates a depth-first list of all dependencies of this message in field order.
+   * @param [deps] {Array}
+   * @returns {Array}
+   */
+  getFullDependencies(deps = []) {
+    return [];
+  }
+
+  /**
+   * Computes the full text of a message/service.
+   * Necessary for rosbags.
+   * Mirrors gentools.
+   * See compute_full_text() in
+   *   https://github.com/ros/ros/blob/kinetic-devel/core/roslib/src/roslib/gentools.py
+   */
+  computeFullText() {
+    const w = new IndentedWriter();
+
+    const deps = this.getFullDependencies();
+    const sep = '='.repeat(80);
+    w.write(this.fileContents.trim())
+      .newline();
+
+    deps.forEach((dep) => {
+      w.write(sep)
+        .write(`MSG: ${dep.getFullMessageName()}`)
+        .write(dep.fileContents.trim())
+        .newline();
+    });
+
+    return w.get().trim();
+  }
+}
+
+/**
+ * Subclass of RosMsgSpec
+ * Implements logic for individual ros messages as well as separated parts of services and actions
+ * (e.g. Request, Response, Goal, ActionResult, ...)
+ * @class MsgSpec
+ */
+class MsgSpec extends RosMsgSpec {
+  constructor(msgCache, packageName, messageName, type, filePath=null, fileContents=null) {
+    super(msgCache, packageName, messageName, type, filePath);
+    this.constants = [];
+    this.fields = [];
+
+    this.loadFile(filePath, fileContents);
+  }
+
+  /**
+   * Parses through message definition for fields and constants
+   * @param content {string} relevant portion of message definition
+   * @private
+   */
+  _parseMessage(content) {
+    let lines = content.split('\n').map((line) => line.trim());
+
+    try {
+      lines.forEach(this._parseLine.bind(this));
+    }
+    catch (err) {
+      console.error('Error while parsing message %s: %s', this.getFullMessageName(), err);
+      throw err;
+    }
+  }
+
+  /**
    * Given a line from the message file, parse it for useful contents
    * @param line {string}
    * @private
@@ -284,135 +370,6 @@ class RosMsgSpec {
         this.fields.push(f);
       }
     }
-  };
-
-  /**
-   * Takes a full definition and pulls out the piece relevant to this specific message spec
-   * (e.g. only the goal from the action message or only the request from the service message)
-   * @param fileContents string
-   * @returns {string}
-   * @private
-   */
-  _extractRelevantMessage(fileContents) {
-    let lines = fileContents.split('\n').map((line) => line.trim());
-
-    switch (this.type) {
-      case SRV_REQUEST_TYPE: {
-        let divider = lines.indexOf(MSG_DIVIDER);
-        lines = lines.slice(0, divider);
-        break;
-      }
-      case SRV_RESPONSE_TYPE: {
-        let divider = lines.indexOf(MSG_DIVIDER);
-        lines = lines.slice(divider + 1);
-        break;
-      }
-      case ACTION_GOAL_TYPE: {
-        let divider = lines.indexOf(MSG_DIVIDER);
-        lines = lines.slice(0, divider);
-        break;
-      }
-      case ACTION_RESULT_TYPE: {
-        const divider1 = lines.indexOf(MSG_DIVIDER) + 1;
-        const divider2 = lines.indexOf(MSG_DIVIDER, divider1);
-        lines = lines.slice(divider1, divider2);
-        break;
-      }
-      case ACTION_FEEDBACK_TYPE: {
-        let divider = lines.indexOf(MSG_DIVIDER);
-        divider = lines.indexOf(MSG_DIVIDER, divider+1);
-        lines = lines.slice(divider + 1);
-        break;
-      }
-      default:
-        break;
-    }
-
-    return lines.join('\n');
-  }
-
-  /**
-   * Parses through message definition for fields and constants
-   * Todo: move this to MsgSpec?
-   * @param content {string} raw message definition
-   * @private
-   */
-  _extractFields(content) {
-    let lines = content.split('\n').map((line) => line.trim());
-
-    try {
-      lines.forEach(this._parseLine.bind(this));
-    }
-    catch (err) {
-      console.error('Error while parsing message %s: %s', this.getFullMessageName(), err);
-      throw err;
-    }
-  }
-
-  /**
-   * For this message spec, generates the text used to calculate the message's md5 sum
-   * @returns {string}
-   */
-  getMd5text() {
-    return '';
-  }
-
-  /**
-   * Get the md5 sum of this message
-   * @returns {string}
-   */
-  getMd5sum() {
-    return md5(this.getMd5text());
-  }
-
-  /**
-   * Generates a depth-first list of all dependencies of this message in field order.
-   * @param [deps] {Array}
-   * @returns {Array}
-   */
-  getFullDependencies(deps = []) {
-    return [];
-  }
-
-  /**
-   * Computes the full text of a message/service.
-   * Necessary for rosbags.
-   * Mirrors gentools.
-   * See compute_full_text() in
-   *   https://github.com/ros/ros/blob/kinetic-devel/core/roslib/src/roslib/gentools.py
-   */
-  computeFullText() {
-    const w = new IndentedWriter();
-
-    const deps = this.getFullDependencies();
-    const sep = '='.repeat(80);
-    w.write(this.fileContents.trim())
-      .newline();
-
-    deps.forEach((dep) => {
-      w.write(sep)
-        .write(`MSG: ${dep.getFullMessageName()}`)
-        .write(dep.fileContents.trim())
-        .newline();
-    });
-
-    return w.get().trim();
-  }
-}
-
-/**
- * Subclass of RosMsgSpec
- * Implements logic for individual ros messages as well as separated parts of services and actions
- * (e.g. Request, Response, Goal, ActionResult, ...)
- * @class MsgSpec
- */
-class MsgSpec extends RosMsgSpec {
-  constructor(msgCache, packageName, messageName, type, filePath=null, fileContents=null) {
-    super(msgCache, packageName, messageName, type, filePath);
-    this.constants = [];
-    this.fields = [];
-
-    this.loadFile(filePath, fileContents);
   }
 
   /**
@@ -558,9 +515,38 @@ class SrvSpec extends RosMsgSpec {
     super(msgCache, packageName, messageName, type, filePath);
 
     this.fileContents = this._loadMessageFile(filePath);
+    const {req, resp} = this._extractMessageSections(this.fileContents);
 
-    this.request = new MsgSpec(msgCache, packageName, messageName + 'Request', SRV_REQUEST_TYPE, null, this.fileContents);
-    this.response = new MsgSpec(msgCache, packageName, messageName + 'Response', SRV_RESPONSE_TYPE, null, this.fileContents);
+    this.request = new MsgSpec(msgCache, packageName, messageName + 'Request', SRV_REQUEST_TYPE, null, req);
+    this.response = new MsgSpec(msgCache, packageName, messageName + 'Response', SRV_RESPONSE_TYPE, null, resp);
+  }
+
+  /**
+   * Takes a full service definition and pulls out the request and response sections
+   * @param fileContents {string}
+   * @returns {object}
+   * @private
+   */
+  _extractMessageSections(fileContents) {
+    let lines = fileContents.split('\n').map((line) => line.trim());
+
+    const sections = {
+      req: '',
+      resp: ''
+    };
+
+    let currentSection = 'req';
+
+    lines.forEach((line) => {
+      if (line.startsWith(MSG_DIVIDER)) {
+        currentSection = 'resp';
+      }
+      else {
+        sections[currentSection] += `\n${line}`;
+      }
+    });
+
+    return sections;
   }
 
   getMd5text() {
@@ -594,14 +580,47 @@ class ActionSpec extends RosMsgSpec {
     super(msgCache, packageName, messageName, type, filePath);
 
     this.fileContents = this._loadMessageFile(filePath);
+    const {goal, result, feedback} = this._extractMessageSections(this.fileContents);
 
     // Parse the action definition into its 3 respective parts
-    this.goal = new MsgSpec(msgCache, packageName, messageName + 'Goal', ACTION_GOAL_TYPE, null, this.fileContents);
-    this.result = new MsgSpec(msgCache, packageName, messageName + 'Result', ACTION_RESULT_TYPE, null, this.fileContents);
-    this.feedback = new MsgSpec(msgCache, packageName, messageName + 'Feedback', ACTION_FEEDBACK_TYPE, null, this.fileContents);
+    this.goal = new MsgSpec(msgCache, packageName, messageName + 'Goal', ACTION_GOAL_TYPE, null, goal);
+    this.result = new MsgSpec(msgCache, packageName, messageName + 'Result', ACTION_RESULT_TYPE, null, result);
+    this.feedback = new MsgSpec(msgCache, packageName, messageName + 'Feedback', ACTION_FEEDBACK_TYPE, null, feedback);
     this.generateActionMessages();
   }
 
+  /**
+   * Takes a full service definition and pulls out the request and response sections
+   * @param fileContents {string}
+   * @returns {object}
+   * @private
+   */
+  _extractMessageSections(fileContents) {
+    let lines = fileContents.split('\n').map((line) => line.trim());
+
+    const sections = {
+      goal: '',
+      result: '',
+      feedback: ''
+    };
+
+    let currentSection = 'goal';
+
+    lines.forEach((line) => {
+      if (line.startsWith(MSG_DIVIDER)) {
+        currentSection = {
+          goal: 'result',
+          result: 'feedback'
+        }[currentSection];
+      }
+      else {
+        sections[currentSection] += `\n${line}`;
+      }
+    });
+
+    return sections;
+  }
+
   /**
    * Get a list of all the message specs created by this ros action
    * @returns {MsgSpec[]}
@@ -670,4 +689,4 @@ RosMsgSpec.ACTION_ACTION_FEEDBACK_TYPE = ACTION_ACTION_FEEDBACK_TYPE;
 RosMsgSpec.ACTION_ACTION_RESULT_TYPE = ACTION_ACTION_RESULT_TYPE;
 RosMsgSpec.ACTION_ACTION_TYPE = ACTION_ACTION_TYPE;
 
-module.exports = RosMsgSpec;
+module.exports = RosMsgSpec;

test/gennodejsTest.js

@@ -735,6 +735,20 @@ describe('gennodejsTests', () => {
 
       done();
     });
+
+    it('Service with improper divider', (done) => {
+      // correct message divider is '---'
+      // we're allowing any line starting with '---' though, parroting genmsg
+      const HeaderService = msgUtils.getHandlerForSrvType('test_msgs/NonSpecServiceDivider');
+
+      const requestMsg = HeaderService.Request.messageDefinition().trim();
+      expect(requestMsg).to.equal('string data');
+
+      const responseMsg = HeaderService.Response.messageDefinition().trim();
+      expect(responseMsg).to.equal('uint8 response');
+
+      done();
+    });
   });
 
   describe('actions', () => {
@@ -996,4 +1010,4 @@ describe('gennodejsTests', () => {
       expect(() => VariableLengthArray.serialize(fullMsg, buf, 0)).to.not.throw(Error);
     });
   });
-});
+});

test/onTheFlyMessages.js

@@ -740,6 +740,20 @@ describe('On The Fly Message Tests', () => {
 
       done();
     });
+
+    it('Service with improper divider', (done) => {
+      // correct message divider is '---'
+      // we're allowing any line starting with '---' though, parroting genmsg
+      const HeaderService = msgUtils.getHandlerForSrvType('test_msgs/NonSpecServiceDivider');
+
+      const requestMsg = HeaderService.Request.messageDefinition().trim();
+      expect(requestMsg).to.equal('string data');
+
+      const responseMsg = HeaderService.Response.messageDefinition().trim();
+      expect(responseMsg).to.equal('uint8 response');
+
+      done();
+    });
   });
 
   describe('actions', () => {