Merge pull request #290 in NAR/sdl-core from bugfix/handle_review to feature/Ford-WiFi

* commit '9d4d335347fe5ccf69c99e21d2e12e9a2bcd0fd7':
  address review comments from OSS maintainer

5 files changed, 20 insertions, 47 deletions

diff --git a/src/components/connection_handler/include/connection_handler/connection.h b/src/components/connection_handler/include/connection_handler/connection.h
index 2b5ba3e82d..a2309c1521 100644
--- a/src/components/connection_handler/include/connection_handler/connection.h
+++ b/src/components/connection_handler/include/connection_handler/connection.h
@@ -79,9 +79,6 @@ struct Service {
       , connection_id(0)
       , is_protected_(false) {}
 
-  DEPRECATED explicit Service(protocol_handler::ServiceType service_type)
-      : service_type(service_type), connection_id(0), is_protected_(false) {}
-
   explicit Service(protocol_handler::ServiceType service_type,
                    transport_manager::ConnectionUID connection_id)
       : service_type(service_type)
@@ -162,12 +159,6 @@ class Connection {
 
   /**
    * @brief Adds session to connection
-   * @return new session id or 0 in case of issues
-   */
-  DEPRECATED uint32_t AddNewSession();
-
-  /**
-   * @brief Adds session to connection
    * @param connection_handle Connection Handle for the session
    * @return new session id or 0 in case of issues
    */
@@ -187,18 +178,6 @@ class Connection {
    * @param session_id session ID
    * @param service_type Type of service
    * @param is_protected protection state
-   * @return TRUE on success, otherwise FALSE
-   */
-  DEPRECATED bool AddNewService(uint8_t session_id,
-                                protocol_handler::ServiceType service_type,
-                                const bool is_protected);
-
-  /**
-   * @brief Adds uprotected service to session or
-   * check protection to service has been started before
-   * @param session_id session ID
-   * @param service_type Type of service
-   * @param is_protected protection state
    * @param connection_id Connection ID associated with the service
    * @return TRUE on success, otherwise FALSE
    */
@@ -219,7 +198,7 @@ class Connection {
    * @brief Removes secondary service from session
    * @param secondary_connection_handle connection identifying services to be
    * removed
-   * \param removed_services_list Returned: List of service types removed
+   * @param removed_services_list Returned: List of service types removed
    * @return the session ID associated with the services removed
    */
   uint8_t RemoveSecondaryServices(
@@ -334,8 +313,8 @@ class Connection {
   ConnectionHandle primary_connection_handle() const;
 
   /**
-   * \brief Sets the primary connection handle
-   * \param primary_connection_handle the primary connection handle to
+   * @brief Sets the primary connection handle
+   * @param primary_connection_handle the primary connection handle to
    * associate with this connection
    */
   void SetPrimaryConnectionHandle(ConnectionHandle primary_connection_handle);
diff --git a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
index 88352d106a..7b12cb541e 100644
--- a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
+++ b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
@@ -54,6 +54,8 @@
 #include "utils/stl_utils.h"
 #include "utils/rwlock.h"
 
+#define DISABLED_SECONDARY 0xFFFFFFFF
+
 /**
  * \namespace connection_handler
  * \brief SmartDeviceLink connection_handler namespace.
diff --git a/src/components/connection_handler/src/connection.cc b/src/components/connection_handler/src/connection.cc
index 363da19be9..103ac113cf 100644
--- a/src/components/connection_handler/src/connection.cc
+++ b/src/components/connection_handler/src/connection.cc
@@ -138,10 +138,6 @@ uint32_t findGap(const std::map<unsigned char, T>& map) {
 }
 }  // namespace
 
-DEPRECATED uint32_t Connection::AddNewSession() {
-  return AddNewSession(0);
-}
-
 uint32_t Connection::AddNewSession(
     const transport_manager::ConnectionUID connection_handle) {
   LOG4CXX_AUTO_TRACE(logger_);
@@ -210,13 +206,6 @@ uint32_t Connection::RemoveSession(uint8_t session_id) {
   return session_id;
 }
 
-DEPRECATED bool Connection::AddNewService(
-    uint8_t session_id,
-    protocol_handler::ServiceType service_type,
-    const bool request_protection) {
-  return AddNewService(session_id, service_type, request_protection, 0);
-}
-
 bool Connection::AddNewService(uint8_t session_id,
                                protocol_handler::ServiceType service_type,
                                const bool request_protection,
diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc
index e9188b9f66..a2883c9137 100644
--- a/src/components/connection_handler/src/connection_handler_impl.cc
+++ b/src/components/connection_handler/src/connection_handler_impl.cc
@@ -1026,10 +1026,11 @@ SessionTransports ConnectionHandlerImpl::SetSecondaryTransportID(
     st = it->second;
 
     // The only time we overwrite an existing entry in the map is if the new
-    // secondary transport ID is 0xFFFFFFFF, which effectively DISABLES the
-    // secondary transport feature for the session, or if the new secondary
+    // secondary transport ID is DISABLED_SECONDARY, which effectively DISABLES
+    // the secondary transport feature for the session, or if the new secondary
     // transport ID is 0, which means a secondary transport has shut down
-    if (st.secondary_transport != 0 && secondary_transport_id != 0xFFFFFFFF &&
+    if (st.secondary_transport != 0 &&
+        secondary_transport_id != DISABLED_SECONDARY &&
         secondary_transport_id != 0) {
       LOG4CXX_WARN(logger_,
                    "SetSecondaryTransportID: session ID "
diff --git a/src/components/connection_handler/test/connection_handler_impl_test.cc b/src/components/connection_handler/test/connection_handler_impl_test.cc
index 45bdf435e7..9ea5bf8637 100644
--- a/src/components/connection_handler/test/connection_handler_impl_test.cc
+++ b/src/components/connection_handler/test/connection_handler_impl_test.cc
@@ -2120,6 +2120,8 @@ TEST_F(ConnectionHandlerTest, StopSecondarySession_NoService) {
   // session ID even if we don't have any services
   EXPECT_CALL(mock_connection_handler_observer,
               OnSecondaryTransportEndedCallback(_));
+  EXPECT_CALL(mock_connection_handler_observer,

+              OnServiceEndedCallback(_, _, _)).Times(0);
 
   connection_handler_->OnSecondaryTransportEnded(uid_, secondary_uid);
 
@@ -2156,10 +2158,10 @@ TEST_F(ConnectionHandlerTest, SetSecondaryTransportID_UpdateSuccess) {
   {
     NonConstDataAccessor<SessionConnectionMap> accessor =
         connection_handler_->session_connection_map();
-    SessionConnectionMap& session_conneciton_map = accessor.GetData();
+    SessionConnectionMap& session_connection_map = accessor.GetData();
     // secondary transport's ID is 0
     SessionTransports st = {primary_uid, secondary_uid};
-    session_conneciton_map[session_id] = st;
+    session_connection_map[session_id] = st;
   }
 
   secondary_uid = 200;
@@ -2176,10 +2178,10 @@ TEST_F(ConnectionHandlerTest, SetSecondaryTransportID_UpdateFailure) {
   {
     NonConstDataAccessor<SessionConnectionMap> accessor =
         connection_handler_->session_connection_map();
-    SessionConnectionMap& session_conneciton_map = accessor.GetData();
+    SessionConnectionMap& session_connection_map = accessor.GetData();
     // secondary transport's ID is already assigned
     SessionTransports st = {primary_uid, secondary_uid};
-    session_conneciton_map[session_id] = st;
+    session_connection_map[session_id] = st;
   }
 
   SessionTransports st =
@@ -2196,12 +2198,12 @@ TEST_F(ConnectionHandlerTest, SetSecondaryTransportID_OverwirteSecondaryUID) {
   {
     NonConstDataAccessor<SessionConnectionMap> accessor =
         connection_handler_->session_connection_map();
-    SessionConnectionMap& session_conneciton_map = accessor.GetData();
+    SessionConnectionMap& session_connection_map = accessor.GetData();
     SessionTransports st = {primary_uid, secondary_uid};
-    session_conneciton_map[session_id] = st;
+    session_connection_map[session_id] = st;
   }
 
-  secondary_uid = 0xFFFFFFFF;
+  secondary_uid = DISABLED_SECONDARY;
   SessionTransports st =
       connection_handler_->SetSecondaryTransportID(session_id, secondary_uid);
   EXPECT_EQ(primary_uid, st.primary_transport);
@@ -2216,9 +2218,9 @@ TEST_F(ConnectionHandlerTest, SetSecondaryTransportID_Failure) {
   {
     NonConstDataAccessor<SessionConnectionMap> accessor =
         connection_handler_->session_connection_map();
-    SessionConnectionMap& session_conneciton_map = accessor.GetData();
+    SessionConnectionMap& session_connection_map = accessor.GetData();
     SessionTransports st = {primary_uid, secondary_uid};
-    session_conneciton_map[session_id] = st;
+    session_connection_map[session_id] = st;
   }
 
   uint8_t invalid_session_id = 10;