Address comments

6 files changed, 72 insertions, 44 deletions

diff --git a/src/components/application_manager/include/application_manager/rpc_handler_impl.h b/src/components/application_manager/include/application_manager/rpc_handler_impl.h
index 7f61c7b034..8f1d454c3b 100644
--- a/src/components/application_manager/include/application_manager/rpc_handler_impl.h
+++ b/src/components/application_manager/include/application_manager/rpc_handler_impl.h
@@ -143,6 +143,16 @@ class RPCHandlerImpl : public RPCHandler,
   void SetTelemetryObserver(AMTelemetryObserver* observer) OVERRIDE;
 #endif  // TELEMETRY_MONITOR
 
+  /**
+   * @brief Extracts and validates the syncMsgVersion included in
+   * a RegisterAppInterfaceRequest
+   *
+   * @param output - SmartObject Message received from mobile
+   * @param messageVersion - message version to be updated
+   */
+  void GetMessageVersion(NsSmartDeviceLink::NsSmartObjects::SmartObject& output,
+                         utils::SemanticVersion& message_version);
+
  private:
   void ProcessMessageFromMobile(const std::shared_ptr<Message> message);
   void ProcessMessageFromHMI(const std::shared_ptr<Message> message);
diff --git a/src/components/application_manager/src/rpc_handler_impl.cc b/src/components/application_manager/src/rpc_handler_impl.cc
index 8b405cf61d..bf1ff78d36 100644
--- a/src/components/application_manager/src/rpc_handler_impl.cc
+++ b/src/components/application_manager/src/rpc_handler_impl.cc
@@ -194,6 +194,36 @@ void RPCHandlerImpl::SetTelemetryObserver(AMTelemetryObserver* observer) {
 
 #endif  // TELEMETRY_MONITOR
 
+void RPCHandlerImpl::GetMessageVersion(
+    NsSmartDeviceLink::NsSmartObjects::SmartObject& output,
+    utils::SemanticVersion& message_version) {
+  if (output.keyExists(
+          NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS) &&
+      output[NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS].keyExists(
+          strings::sync_msg_version)) {
+    // SyncMsgVersion exists, check if it is valid.
+    auto sync_msg_version =
+        output[NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS]
+              [strings::sync_msg_version];
+    uint16_t major = 0;
+    uint16_t minor = 0;
+    uint16_t patch = 0;
+    if (sync_msg_version.keyExists(strings::major_version)) {
+      major = sync_msg_version[strings::major_version].asUInt();
+    }
+    if (sync_msg_version.keyExists(strings::minor_version)) {
+      minor = sync_msg_version[strings::minor_version].asUInt();
+    }
+    if (sync_msg_version.keyExists(strings::patch_version)) {
+      patch = sync_msg_version[strings::patch_version].asUInt();
+    }
+    utils::SemanticVersion temp_version(major, minor, patch);
+    if (temp_version.isValid()) {
+      message_version = temp_version;
+    }
+  }
+}
+
 bool RPCHandlerImpl::ConvertMessageToSO(
     const Message& message,
     NsSmartDeviceLink::NsSmartObjects::SmartObject& output) {
@@ -229,19 +259,7 @@ bool RPCHandlerImpl::ConvertMessageToSO(
       } else if (mobile_apis::FunctionID::RegisterAppInterfaceID ==
                  static_cast<mobile_apis::FunctionID::eType>(
                      output[strings::params][strings::function_id].asInt())) {
-        if (output.keyExists(
-                NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS) &&
-                output[NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS]
-                    .keyExists(strings::sync_msg_version)) {
-          // SyncMsgVersion exists, check if it is valid.
-          std::string str_msg_version =
-              output[NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS]
-                    [strings::sync_msg_version].asString();
-          utils::SemanticVersion temp_version(str_msg_version);
-          if (temp_version.isValid()) {
-            msg_version = temp_version;
-          }
-        }
+        GetMessageVersion(output, msg_version);
       }
 
       if (!conversion_result ||
diff --git a/src/components/include/utils/semantic_version.h b/src/components/include/utils/semantic_version.h
index 6c54a11984..7b9ec0eb96 100644
--- a/src/components/include/utils/semantic_version.h
+++ b/src/components/include/utils/semantic_version.h
@@ -37,42 +37,42 @@ namespace utils {
 
 struct SemanticVersion {
   SemanticVersion(uint16_t major = 0, uint16_t minor = 0, uint16_t patch = 0) {
-    majorVersion = major;
-    minorVersion = minor;
-    patchVersion = patch;
+    major_version_ = major;
+    minor_version_ = minor;
+    patch_version_ = patch;
   }
 
   SemanticVersion(const SemanticVersion& other) {
-    majorVersion = other.majorVersion;
-    minorVersion = other.minorVersion;
-    patchVersion = other.patchVersion;
+    major_version_ = other.major_version_;
+    minor_version_ = other.minor_version_;
+    patch_version_ = other.patch_version_;
   }
 
   SemanticVersion(const std::string& versionString)
-      : majorVersion(0), minorVersion(0), patchVersion(0) {
-    unsigned int majorInt, minorInt, patchInt;
+      : major_version_(0), minor_version_(0), patch_version_(0) {
+    unsigned int major_int, minor_int, patch_int;
     int readElements = sscanf(
-        versionString.c_str(), "%u.%u.%u", &majorInt, &minorInt, &patchInt);
+        versionString.c_str(), "%u.%u.%u", &major_int, &minor_int, &patch_int);
     if (readElements != 3) {
       // LOG4CXX_WARN(logger_,
       //             "Error while parsing version string: " << versionString);
     } else {
-      majorVersion = static_cast<uint8_t>(majorInt);
-      minorVersion = static_cast<uint8_t>(minorInt);
-      patchVersion = static_cast<uint8_t>(patchInt);
+      major_version_ = static_cast<uint8_t>(major_int);
+      minor_version_ = static_cast<uint8_t>(minor_int);
+      patch_version_ = static_cast<uint8_t>(patch_int);
     }
   }
 
   static inline int16_t cmp(const SemanticVersion& version1,
                             const SemanticVersion& version2) {
     int16_t diff =
-        static_cast<int16_t>(version1.majorVersion - version2.majorVersion);
+        static_cast<int16_t>(version1.major_version_ - version2.major_version_);
     if (diff == 0) {
-      diff =
-          static_cast<int16_t>(version1.minorVersion - version2.minorVersion);
+      diff = static_cast<int16_t>(version1.minor_version_ -
+                                  version2.minor_version_);
       if (diff == 0) {
-        diff =
-            static_cast<int16_t>(version1.patchVersion - version2.patchVersion);
+        diff = static_cast<int16_t>(version1.patch_version_ -
+                                    version2.patch_version_);
       }
     }
     return diff;
@@ -100,21 +100,21 @@ struct SemanticVersion {
 
   const std::string toString() const {
     std::string result = "";
-    result += std::to_string(majorVersion);
+    result += std::to_string(major_version_);
     result += ".";
-    result += std::to_string(minorVersion);
+    result += std::to_string(minor_version_);
     result += ".";
-    result += std::to_string(patchVersion);
+    result += std::to_string(patch_version_);
     return result;
   }
 
   bool isValid() const {
-    return majorVersion > 0 || minorVersion > 0 || patchVersion > 0;
+    return major_version_ > 0 || minor_version_ > 0 || patch_version_ > 0;
   }
 
-  uint16_t majorVersion = 0;
-  uint16_t minorVersion = 0;
-  uint16_t patchVersion = 0;
+  uint16_t major_version_ = 0;
+  uint16_t minor_version_ = 0;
+  uint16_t patch_version_ = 0;
 };
 }
 
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index 4ff24f9e64..0e49e28397 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -252,7 +252,7 @@ void ProtocolHandlerImpl::SendStartSessionAck(
   const bool proxy_supports_v5_protocol =
       input_protocol_version >= PROTOCOL_VERSION_5 ||
       (ServiceTypeFromByte(service_type) == kRpc &&
-       full_version.majorVersion >= PROTOCOL_VERSION_5);
+       full_version.major_version_ >= PROTOCOL_VERSION_5);
 
   if (kRpc != service_type) {
     // In case if input protocol version os bigger then supported, SDL should
@@ -308,7 +308,7 @@ void ProtocolHandlerImpl::SendStartSessionAck(
 
       // Minimum protocol version supported by both
       utils::SemanticVersion* minVersion =
-          (full_version.majorVersion < PROTOCOL_VERSION_5)
+          (full_version.major_version_ < PROTOCOL_VERSION_5)
               ? &defaultProtocolVersion
               : utils::SemanticVersion::min(full_version,
                                             defaultProtocolVersion);
@@ -1619,7 +1619,7 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageStartSession(
           std::string(bson_object_get_string(&obj, "protocolVersion")));
       bson_object_deinitialize(&obj);
       // Constructed payloads added in Protocol v5
-      if (fullVersion->majorVersion < PROTOCOL_VERSION_5) {
+      if (fullVersion->major_version_ < PROTOCOL_VERSION_5) {
         rejectedParams.push_back(std::string("protocolVersion"));
       }
     } else {
@@ -1674,7 +1674,7 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageStartSession(
         bson_object_get_string(&obj, "protocolVersion"));
     bson_object_deinitialize(&obj);
 
-    if (fullVersion.majorVersion >= PROTOCOL_VERSION_5) {
+    if (fullVersion.major_version_ >= PROTOCOL_VERSION_5) {
       // Start service without protection
       SendStartSessionAck(connection_id,
                           session_id,
@@ -1892,7 +1892,7 @@ void ProtocolHandlerImpl::NotifySessionStarted(
     std::string version_string(version_param == NULL ? "" : version_param);
     fullVersion = std::make_shared<utils::SemanticVersion>(version_string);
     // Constructed payloads added in Protocol v5
-    if (fullVersion->majorVersion < PROTOCOL_VERSION_5) {
+    if (fullVersion->major_version_ < PROTOCOL_VERSION_5) {
       rejected_params.push_back(std::string(strings::protocol_version));
     }
     bson_object_deinitialize(&request_params);
diff --git a/src/components/smart_objects/include/smart_objects/enum_schema_item.h b/src/components/smart_objects/include/smart_objects/enum_schema_item.h
index 46656d1bd9..23532f1bbe 100644
--- a/src/components/smart_objects/include/smart_objects/enum_schema_item.h
+++ b/src/components/smart_objects/include/smart_objects/enum_schema_item.h
@@ -346,7 +346,7 @@ const ElementSignature TEnumSchemaItem<EnumType>::getSignature(
   }
 
   // Could not match msg version to element siganture
-  ElementSignature ret("", "", false);
+  ElementSignature ret;
   return ret;
 }
 
diff --git a/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py b/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
index 27b5a05f98..6546b0ea89 100755
--- a/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
+++ b/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
@@ -920,7 +920,7 @@ class CodeGenerator(object):
             member.history is None):
             return
         if (member.history is not None and member.since is None):
-             raise GenerateError("Error: Missing since version parameter for " + member.name)
+            raise GenerateError("Error: Missing since version parameter for " + member.name)
         if (member.until is not None): 
             raise GenerateError("Error: Until should only exist in history tag for " + member.name)
         if (member.history is None):