From 8a07d4d29569cd01d3bffcf3de8f8a0baf7dca78 Mon Sep 17 00:00:00 2001
From: Nate Koenig <nate@openrobotics.org>
Date: Thu, 23 Apr 2020 06:03:05 -0700
Subject: [PATCH] Documentation and cleanup

Signed-off-by: Nate Koenig <nate@openrobotics.org>
---
 Changelog.md                                |  4 ++++
 plugins/websocket_server/WebsocketServer.cc | 21 ++++++++++++---------
 plugins/websocket_server/WebsocketServer.hh |  9 +++++++++
 plugins/websocket_server/ign.js             |  6 ++++++
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index bbacc280..b6fe746b 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,6 +2,10 @@
 
 ### Ignition Launch 1.X.X
 
+1. Changed the websocket messages to be more efficient when transmitting
+   protobuf messages.
+    * [Pull Request 22](https://github.com/ignitionrobotics/ign-launch/pull/22)
+
 1. Added ability to set the publication rate of the websocket server.
     * [Pull Request 17](https://bitbucket.org/ignitionrobotics/ign-launch/pull-requests/17)
 
diff --git a/plugins/websocket_server/WebsocketServer.cc b/plugins/websocket_server/WebsocketServer.cc
index 5f57bae6..db818fee 100644
--- a/plugins/websocket_server/WebsocketServer.cc
+++ b/plugins/websocket_server/WebsocketServer.cc
@@ -25,7 +25,19 @@
 
 using namespace ignition::launch;
 
+/// \brief Construct a websocket frame header.
+/// \param[in] _op The operation string.
+/// \param[in] _topic The topic name string.
+/// \param[in] _type The message type string.
+/// \return A string that is the frame header.
 #define BUILD_HEADER(_op, _topic, _type) ((_op)+","+(_topic)+","+(_type)+",")
+
+/// \brief Construction a complete websocket frame.
+/// \param[in] _op The operation string.
+/// \param[in] _topic The topic name string.
+/// \param[in] _type The message type string.
+/// \param[in] _payload The complete payload string.
+/// \return A string that is the frame header.
 #define BUILD_MSG(_op, _topic, _type, _payload) (BUILD_HEADER(_op, _topic, _type) + _payload)
 
 int rootCallback(struct lws *_wsi,
@@ -270,15 +282,6 @@ void WebsocketServer::OnDisconnect(int _socketId)
   }
 }
 
-// Option 1: Encode everything in a Packet.
-//     Pros: Works, and has serialization.
-//     Cons: Server need to de-serialize and re-serialize messages.
-//
-// Option 2: Send header along with payload. Don't encode the header, and
-// only copy the serialized payload.
-//
-// Option 3: Encode the header and payload into a Packet.
-
 //////////////////////////////////////////////////
 void WebsocketServer::OnMessage(int _socketId, const std::string &_msg)
 {
diff --git a/plugins/websocket_server/WebsocketServer.hh b/plugins/websocket_server/WebsocketServer.hh
index 85c1349e..1126b8a2 100644
--- a/plugins/websocket_server/WebsocketServer.hh
+++ b/plugins/websocket_server/WebsocketServer.hh
@@ -142,8 +142,17 @@ namespace ignition
                    const char *_data, const size_t _size);
 
       public: std::mutex mutex;
+
+      /// \brief A mutex used in the OnWebsocketSubscribedMessage
+      /// function.
       public: std::mutex subscriptionMutex;
+
+      /// \brief All of the websocket connections.
       public: std::map<int, std::unique_ptr<Connection>> connections;
+
+      /// \brief All of the subscribed Ignition topics.
+      /// The key is the topic name, and the value is the set of websocket
+      /// connections that have subscribed to the topic.
       public: std::map<std::string, std::set<int>> topicConnections;
 
       /// \brief Time of last publication for each subscribed topic. The key
diff --git a/plugins/websocket_server/ign.js b/plugins/websocket_server/ign.js
index 838dbd30..f5efcd9c 100644
--- a/plugins/websocket_server/ign.js
+++ b/plugins/websocket_server/ign.js
@@ -15,6 +15,12 @@
  *
  */
 
+/// \brief Construct a complete websocket message.
+/// \param[in] _frameParts This must be an array of four strings:
+///   1. operation,
+///   2. topic name,
+///   3. message type, and
+///   4. payload
 function buildMsg(_frameParts) {
   return _frameParts.join(',');
 }