Changes based on comments, added any missing method descriptions

11 files changed, 77 insertions, 20 deletions

diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h
index e6148a3c1d..0c8a30b47a 100644
--- a/src/components/application_manager/include/application_manager/application_manager_impl.h
+++ b/src/components/application_manager/include/application_manager/application_manager_impl.h
@@ -1082,8 +1082,8 @@ class ApplicationManagerImpl
     bool operator()(const ApplicationSharedPtr app) const {
       return app
                  ? handle_ == app->device() &&
-                       ProtocolVersion::kV4 <= app->protocol_version() &&
-                       ProtocolVersion::kV5 >= app->protocol_version()
+                       Message::is_sufficient_version(ProtocolVersion::kV4,
+                                                      app->protocol_version())
                  : false;
     }
   };
diff --git a/src/components/application_manager/include/application_manager/message.h b/src/components/application_manager/include/application_manager/message.h
index 53b2271de8..4678e88106 100644
--- a/src/components/application_manager/include/application_manager/message.h
+++ b/src/components/application_manager/include/application_manager/message.h
@@ -102,6 +102,9 @@ class Message {
   void set_data_size(size_t data_size);
   void set_payload_size(size_t payload_size);
 
+  static bool is_sufficient_version(ProtocolVersion minVersion,
+                                    ProtocolVersion version);
+
   protocol_handler::MessagePriority Priority() const {
     return priority_;
   }
diff --git a/src/components/application_manager/src/commands/hmi/sdl_activate_app_request.cc b/src/components/application_manager/src/commands/hmi/sdl_activate_app_request.cc
index 1abe38c5fe..a75eba3ed8 100644
--- a/src/components/application_manager/src/commands/hmi/sdl_activate_app_request.cc
+++ b/src/components/application_manager/src/commands/hmi/sdl_activate_app_request.cc
@@ -46,8 +46,8 @@ struct ProtoV4AppsOnDevice : std::unary_function<ApplicationSharedPtr, bool> {
   bool operator()(const ApplicationSharedPtr app) const {
     return app
                ? handle_ == app->device() &&
-                     ProtocolVersion::kV4 <= app->protocol_version() &&
-                     ProtocolVersion::kV5 >= app->protocol_version()
+                     Message::is_sufficient_version(ProtocolVersion::kV4,
+                                                    app->protocol_version())
                : false;
   }
 };
diff --git a/src/components/application_manager/src/commands/mobile/on_hmi_status_notification_from_mobile.cc b/src/components/application_manager/src/commands/mobile/on_hmi_status_notification_from_mobile.cc
index 58fd571d08..4952f01bb2 100644
--- a/src/components/application_manager/src/commands/mobile/on_hmi_status_notification_from_mobile.cc
+++ b/src/components/application_manager/src/commands/mobile/on_hmi_status_notification_from_mobile.cc
@@ -75,8 +75,9 @@ void OnHMIStatusNotificationFromMobile::Run() {
                     << connection_key() << " and handle: " << handle);
 
   if (!is_apps_requested_before &&
-      ProtocolVersion::kV4 <= app->protocol_version() &&
-      ProtocolVersion::kV5 >= app->protocol_version() && app->is_foreground()) {
+      Message::is_sufficient_version(ProtocolVersion::kV4,
+                                     app->protocol_version()) &&
+      app->is_foreground()) {
     // In case this notification will be received from mobile side with
     // foreground level for app on mobile, this should trigger remote
     // apps list query for SDL 4.0+ app
@@ -90,8 +91,8 @@ void OnHMIStatusNotificationFromMobile::Run() {
                   " for handle: "
                       << handle);
 
-    if (ProtocolVersion::kV4 <= app->protocol_version() &&
-        ProtocolVersion::kV5 >= app->protocol_version()) {
+    if (Message::is_sufficient_version(ProtocolVersion::kV4,
+                                       app->protocol_version())) {
       const ApplicationSet& accessor =
           application_manager_.applications().GetData();
 
@@ -99,8 +100,8 @@ void OnHMIStatusNotificationFromMobile::Run() {
       ApplicationSetConstIt it = accessor.begin();
       for (; accessor.end() != it; ++it) {
         if (connection_key() != (*it)->app_id() &&
-            ProtocolVersion::kV4 <= (*it)->protocol_version() &&
-            ProtocolVersion::kV5 >= (*it)->protocol_version() &&
+            Message::is_sufficient_version(ProtocolVersion::kV4,
+                                           (*it)->protocol_version()) &&
             (*it)->is_foreground()) {
           is_another_foreground_sdl4_app = true;
           break;
diff --git a/src/components/application_manager/src/message.cc b/src/components/application_manager/src/message.cc
index 2206cf479f..48333d4e6f 100644
--- a/src/components/application_manager/src/message.cc
+++ b/src/components/application_manager/src/message.cc
@@ -205,4 +205,9 @@ void Message::set_data_size(size_t data_size) {
 void Message::set_payload_size(size_t payload_size) {
   payload_size_ = payload_size;
 }
+
+bool Message::is_sufficient_version(ProtocolVersion minVersion,
+                                    ProtocolVersion version) {
+  return version >= minVersion && version <= ProtocolVersion::kV5;
+}
 }  // namespace application_manager
diff --git a/src/components/application_manager/src/mobile_message_handler.cc b/src/components/application_manager/src/mobile_message_handler.cc
index 5dddf42825..e82873ce83 100644
--- a/src/components/application_manager/src/mobile_message_handler.cc
+++ b/src/components/application_manager/src/mobile_message_handler.cc
@@ -124,8 +124,8 @@ MobileMessageHandler::HandleOutgoingMessageProtocol(
   if (message->protocol_version() == application_manager::kV1) {
     return MobileMessageHandler::HandleOutgoingMessageProtocolV1(message);
   }
-  if ((message->protocol_version() >= application_manager::kV2) &&
-      (message->protocol_version() <= application_manager::kV5)) {
+  if (Message::is_sufficient_version(ProtocolVersion::kV4,
+                                     message->protocol_version())) {
     return MobileMessageHandler::HandleOutgoingMessageProtocolV2(message);
   }
   return NULL;
diff --git a/src/components/config_profile/include/config_profile/profile.h b/src/components/config_profile/include/config_profile/profile.h
index b9b162bcb1..5c4618903f 100644
--- a/src/components/config_profile/include/config_profile/profile.h
+++ b/src/components/config_profile/include/config_profile/profile.h
@@ -130,16 +130,28 @@ class Profile : public protocol_handler::ProtocolHandlerSettings,
   const uint32_t& app_icons_amount_to_remove() const OVERRIDE;
 
   /**
-   * @brief Returns true, if SDL protocol v5 is enabled
+   * @brief Returns true if SDL protocol v5 is enabled
    */
   bool enable_protocol_5() const OVERRIDE;
 
+  /**
+   * @brief Returns the maximum payload size for control services
+   */
   size_t maximum_control_payload_size() const OVERRIDE;
 
+  /**
+   * @brief Returns the maximum payload size for RPC services
+   */
   size_t maximum_rpc_payload_size() const OVERRIDE;
 
+  /**
+   * @brief Returns the maximum payload size for audio services
+   */
   size_t maximum_audio_payload_size() const OVERRIDE;
 
+  /**
+   * @brief Returns the maximum payload size for video services
+   */
   size_t maximum_video_payload_size() const OVERRIDE;
 
   /**
diff --git a/src/components/config_profile/src/profile.cc b/src/components/config_profile/src/profile.cc
index a75eef1fc1..9ea2672448 100644
--- a/src/components/config_profile/src/profile.cc
+++ b/src/components/config_profile/src/profile.cc
@@ -1088,7 +1088,7 @@ void Profile::UpdateValues() {
   LOG_UPDATED_VALUE(
       app_icons_amount_to_remove_, kAppIconsAmountToRemoveKey, kSDL4Section);
 
-  // Enable protocol ver.4 parameter
+  // Enable protocol ver.5 parameter
   std::string enable_protocol_5_value;
   if (ReadValue(&enable_protocol_5_value, kSDL5Section, kEnableProtocol5Key) &&
       0 == strcmp("true", enable_protocol_5_value.c_str())) {
diff --git a/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h b/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
index 562540ff3f..bb6483dd82 100644
--- a/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
+++ b/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
@@ -250,6 +250,21 @@ class ProtocolHandlerImpl
                            uint8_t service_type,
                            bool protection);
 
+  /**
+   * \brief Sends acknowledgement of starting session to mobile application
+   * with session number and hash code for second version of protocol
+   * was started
+   * \param connection_id Identifier of connection within which session
+   * \param session_id ID of session to be sent to mobile application
+   * \param protocol_version Version of protocol used for communication
+   * \param hash_code For second version of protocol: identifier of session
+   * to be sent to
+   * mobile app for using when ending session
+   * \param service_type Type of session: RPC or BULK Data. RPC by default
+   * \param protection Protection flag
+   * \param full_version full protocol version (major.minor.patch) used by the
+   *                     mobile proxy
+   */
   void SendStartSessionAck(ConnectionID connection_id,
                            uint8_t session_id,
                            uint8_t protocol_version,
@@ -274,6 +289,14 @@ class ProtocolHandlerImpl
                             uint8_t protocol_version,
                             uint8_t service_type);
 
+  /**
+   * \brief Sends fail of starting session to mobile application
+   * \param connection_id Identifier of connection within which session
+   * \param session_id ID of session to be sent to mobile application
+   * \param protocol_version Version of protocol used for communication
+   * \param service_type Type of session: RPC or BULK Data. RPC by default
+   * \param rejected_params List of rejected params to send in payload
+   */
   void SendStartSessionNAck(ConnectionID connection_id,
                             uint8_t session_id,
                             uint8_t protocol_version,
diff --git a/src/components/protocol_handler/include/protocol_handler/protocol_packet.h b/src/components/protocol_handler/include/protocol_handler/protocol_packet.h
index eae4a74025..31b4c12ea6 100644
--- a/src/components/protocol_handler/include/protocol_handler/protocol_packet.h
+++ b/src/components/protocol_handler/include/protocol_handler/protocol_packet.h
@@ -63,6 +63,11 @@ class ProtocolPacket {
     uint32_t totalDataBytes;
   };
 
+  /**
+   * \class ProtocolVersion
+   * \brief Used for storing the full protocol version of a service
+   *        (major.minor.patch).
+   */
   class ProtocolVersion {
    public:
     ProtocolVersion();
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index fce8cbfcea..8bdd6f1fb1 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -317,7 +317,9 @@ void ProtocolHandlerImpl::SendStartSessionNAck(
                                            0u,
                                            message_counters_[session_id]++));
 
-  if (rejectedParams.size() > 0) {
+  uint8_t maxProtocolVersion = SupportedSDLProtocolVersion();
+
+  if (maxProtocolVersion >= PROTOCOL_VERSION_5 && rejectedParams.size() > 0) {
     BsonObject payloadObj;
     bson_object_initialize_default(&payloadObj);
     BsonArray rejectedParamsArr;
@@ -389,7 +391,10 @@ void ProtocolHandlerImpl::SendEndSessionNAck(
                                            session_id,
                                            0u,
                                            message_counters_[session_id]++));
-  if (rejectedParams.size() > 0) {
+
+  uint8_t maxProtocolVersion = SupportedSDLProtocolVersion();
+
+  if (maxProtocolVersion >= PROTOCOL_VERSION_5 && rejectedParams.size() > 0) {
     BsonObject payloadObj;
     bson_object_initialize_default(&payloadObj);
     BsonArray rejectedParamsArr;
@@ -1104,9 +1109,7 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageEndSession(
   LOG4CXX_AUTO_TRACE(logger_);
 
   const uint8_t current_session_id = packet.session_id();
-  uint32_t hash_id;
-
-  hash_id = get_hash_id(packet);
+  uint32_t hash_id = get_hash_id(packet);
   const ServiceType service_type = ServiceTypeFromByte(packet.service_type());
 
   const ConnectionID connection_id = packet.connection_id();
@@ -1328,11 +1331,15 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageStartSession(
     }
     ProtocolPacket::ProtocolVersion* fullVersion;
     std::vector<std::string> rejectedParams(0, std::string(""));
+    // Can't check protocol_version because the first packet is v1, but there
+    // could still
+    // be a payload, in which case we can get the real protocol version
     if (packet.data_size() != 0) {
       BsonObject obj = bson_object_from_bytes(packet.data());
       fullVersion = new ProtocolPacket::ProtocolVersion(
           std::string(bson_object_get_string(&obj, "protocolVersion")));
       bson_object_deinitialize(&obj);
+      // Constructed payloads added in Protocol v5
       if (fullVersion->majorVersion < PROTOCOL_VERSION_5) {
         rejectedParams.push_back(std::string("protocolVersion"));
       }
@@ -1427,7 +1434,8 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageHeartBeat(
   if (session_observer_.ProtocolVersionUsed(
           connection_id, packet.session_id(), protocol_version)) {
     // TODO(EZamakhov): investigate message_id for HeartBeatAck
-    if (protocol_version >= PROTOCOL_VERSION_3) {
+    if (protocol_version >= PROTOCOL_VERSION_3 &&
+        protocol_version <= PROTOCOL_VERSION_5) {
       return SendHeartBeatAck(
           connection_id, packet.session_id(), packet.message_id());
     } else {