Merged in networking_2_louise (pull request gazebosim#136)

More documentation and some extra tests for pull request gazebosim#135

Approved-by: Michael Carroll <[email protected]>

include/ignition/gazebo/network/NetworkRole.hh

@@ -17,7 +17,6 @@
 #ifndef IGNITION_GAZEBO_NETWORK_NETWORKROLE_HH_
 #define IGNITION_GAZEBO_NETWORK_NETWORKROLE_HH_
 
-#include <memory>
 #include <ignition/gazebo/config.hh>
 #include <ignition/gazebo/Export.hh>
 

src/msgs/peer_info.proto

@@ -21,6 +21,7 @@ package ignition.gazebo.msgs;
 
 import "ignition/msgs/header.proto";
 
+/// \brief Holds information about a peer in the network.
 message PeerInfo
 {
   /// \brief Optional header data
@@ -29,30 +30,48 @@ message PeerInfo
   /// \brief ID of the peer
   string id = 2;
 
-  /// \brief hostname of the peer
+  /// \brief Hostname of the peer
   string hostname = 3;
 
+  /// \brief Possible roles for a peer
   enum PeerRole
   {
+    /// \brief No role in the network
     NONE = 0;
+
+    /// \brief Read-only role, not executing simulation
     READ_ONLY = 1;
+
+    /// \brief Configures and assigns work to secondaries
     SIMULATION_PRIMARY = 2;
+
+    /// \brief Receives work from primary and executes
     SIMULATION_SECONDARY = 3;
   };
 
+  /// \brief This peer's role in the network
   PeerRole role = 4;
 };
 
+/// \brief Notify a peer's state to the network.
 message PeerAnnounce
 {
+  /// \brief General information about the peer
   PeerInfo info = 1;
 
+  /// \brief Possible states announced by a peer
   enum PeerState
   {
+    /// \brief An error occurred
     ERROR = 0;
+
+    /// \brief Peer is connecting right now
     CONNECTING = 1;
+
+    /// \brief Peer is disconnecting right now
     DISCONNECTING = 2;
   };
 
+  /// \brief State which the peer is announcing
   PeerState state = 2;
 };

src/network/PeerInfo.cc

@@ -36,10 +36,8 @@ ignition::gazebo::msgs::PeerInfo ignition::gazebo::toProto(
   proto.set_id(_info.id);
   proto.set_hostname(_info.hostname);
 
-  switch (_info.role) {
-    case NetworkRole::None:
-      proto.set_role(ignition::gazebo::msgs::PeerInfo::NONE);
-      break;
+  switch (_info.role)
+  {
     case NetworkRole::ReadOnly:
       proto.set_role(ignition::gazebo::msgs::PeerInfo::READ_ONLY);
       break;
@@ -49,6 +47,7 @@ ignition::gazebo::msgs::PeerInfo ignition::gazebo::toProto(
     case NetworkRole::SimulationSecondary:
       proto.set_role(ignition::gazebo::msgs::PeerInfo::SIMULATION_SECONDARY);
       break;
+    case NetworkRole::None:
     default:
       proto.set_role(ignition::gazebo::msgs::PeerInfo::NONE);
   }

src/network/PeerInfo.hh

@@ -32,19 +32,32 @@ namespace ignition
     // Inline bracket to help doxygen filtering.
     inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE {
     class IGNITION_GAZEBO_VISIBLE PeerInfo {
+      //
+      /// \brief Constructor
       public: PeerInfo();
+
+      /// \brief Unique peer ID in the network
       public: std::string id;
+
+      /// \brief Peer hostname
       public: std::string hostname;
+
+      /// \brief Peer's role in the network
       public: NetworkRole role;
     };
     }
 
+    /// \brief Construct a `PeerInfo` object from a message.
+    /// \param[in] _proto Message
+    /// \result Equivalent PeerInfo
     IGNITION_GAZEBO_VISIBLE PeerInfo fromProto(const msgs::PeerInfo &_proto);
-    IGNITION_GAZEBO_VISIBLE msgs::PeerInfo toProto(const PeerInfo &_info);
-
 
+    /// \brief Construct a `PeerInfo` message from an object.
+    /// \param[in] _info Peer info object
+    /// \result Equivalent message
+    IGNITION_GAZEBO_VISIBLE msgs::PeerInfo toProto(const PeerInfo &_info);
   }  // namespace gazebo
 }  // namespace ignition
 
-#endif  // IGNITION_GAZEBO_NETWORKCONFIG_HH_
+#endif  // IGNITION_GAZEBO_NETWORK_PEERINFO_HH_
 

src/network/PeerTracker.cc

@@ -21,7 +21,7 @@ using namespace gazebo;
 
 /////////////////////////////////////////////////
 PeerTracker::PeerTracker(
-    EventManager* _eventMgr,
+    EventManager *_eventMgr,
     const ignition::transport::NodeOptions &_options):
   node(_options),
   eventMgr(_eventMgr)
@@ -31,7 +31,7 @@ PeerTracker::PeerTracker(
 /////////////////////////////////////////////////
 PeerTracker::~PeerTracker()
 {
-  Disconnect();
+  this->Disconnect();
 }
 
 /////////////////////////////////////////////////
@@ -74,14 +74,21 @@ void PeerTracker::Connect(std::shared_ptr<PeerInfo> _info)
   this->announcePub.Publish(msg);
 
   this->heartbeatRunning = true;
-  this->heartbeatThread = std::thread([this](){ this->HeartbeatLoop(); });
+  this->heartbeatThread = std::thread([this]()
+    {
+      this->HeartbeatLoop();
+    });
 }
 
 /////////////////////////////////////////////////
 void PeerTracker::Disconnect()
 {
+  this->node.Unsubscribe("heartbeat");
+  this->node.Unsubscribe("announce");
+
   this->heartbeatRunning = false;
-  if (this->heartbeatThread.joinable()) {
+  if (this->heartbeatThread.joinable())
+  {
     this->heartbeatThread.join();
   }
 
@@ -94,9 +101,6 @@ void PeerTracker::Disconnect()
     this->announcePub.Publish(msg);
     this->info.reset();
   }
-
-  this->node.Unsubscribe("heartbeat");
-  this->node.Unsubscribe("announce");
 }
 
 /////////////////////////////////////////////////
@@ -134,7 +138,7 @@ void PeerTracker::HeartbeatLoop()
     this->heartbeatPub.Publish(toProto(*this->info));
 
     std::vector<PeerInfo> toRemove;
-    for (auto peer : peers)
+    for (auto peer : this->peers)
     {
       auto age = Clock::now() - peer.second.lastSeen;
       if (age > (this->staleMultiplier * this->heartbeatPeriod))
@@ -173,19 +177,26 @@ void PeerTracker::AddPeer(const PeerInfo &_info)
 }
 
 /////////////////////////////////////////////////
-void PeerTracker::RemovePeer(const PeerInfo &_info)
+bool PeerTracker::RemovePeer(const PeerInfo &_info)
 {
+  if (!this->info)
+  {
+    ignerr << "Internal error: peer missing info." << std::endl;
+    return false;
+  }
+
   auto lock = PeerLock(this->peersMutex);
 
   auto iter = this->peers.find(_info.id);
   if (iter == this->peers.end())
   {
-    igndbg << "Atempting to remove a peer that wasn't connected" << std::endl;
-  }
-  else
-  {
-    this->peers.erase(iter);
+    igndbg << "Attempting to remove peer [" << _info.id << "] from ["
+           << this->info->id << "] but it wasn't connected" << std::endl;
+    return false;
   }
+
+  this->peers.erase(iter);
+  return true;
 }
 
 /////////////////////////////////////////////////
@@ -197,7 +208,8 @@ void PeerTracker::OnPeerAnnounce(const msgs::PeerAnnounce &_announce)
   if (this->info && peer.id == this->info->id)
     return;
 
-  switch (_announce.state()) {
+  switch (_announce.state())
+  {
     case msgs::PeerAnnounce::CONNECTING:
       this->OnPeerAdded(peer);
       break;
@@ -237,7 +249,7 @@ void PeerTracker::OnPeerHeartbeat(const msgs::PeerInfo &_info)
   }
 
   // Update information about the state of this peer.
-  auto& peerState = this->peers[peer.id];
+  auto &peerState = this->peers[peer.id];
   peerState.lastSeen = std::chrono::steady_clock::now();
   peerState.lastHeader = std::chrono::steady_clock::time_point(
       std::chrono::seconds(_info.header().stamp().sec()) +
@@ -247,10 +259,10 @@ void PeerTracker::OnPeerHeartbeat(const msgs::PeerInfo &_info)
 /////////////////////////////////////////////////
 void PeerTracker::OnPeerError(const PeerInfo &_info)
 {
-  this->RemovePeer(_info);
+  auto success = this->RemovePeer(_info);
 
   // Emit event for any consumers
-  if (eventMgr)
+  if (success && eventMgr)
     eventMgr->Emit<PeerError>(_info);
 }
 
@@ -267,19 +279,19 @@ void PeerTracker::OnPeerAdded(const PeerInfo &_info)
 /////////////////////////////////////////////////
 void PeerTracker::OnPeerRemoved(const PeerInfo &_info)
 {
-  this->RemovePeer(_info);
+  auto success = this->RemovePeer(_info);
 
   // Emit event for any consumers
-  if (eventMgr)
+  if (success && eventMgr)
     eventMgr->Emit<PeerRemoved>(_info);
 }
 
 /////////////////////////////////////////////////
 void PeerTracker::OnPeerStale(const PeerInfo &_info)
 {
-  this->RemovePeer(_info);
+  auto success = this->RemovePeer(_info);
 
   // Emit event for any consumers
-  if (eventMgr)
+  if (success && eventMgr)
     eventMgr->Emit<PeerStale>(_info);
 }