Merge pull request #1769 from shoamano83/fix/deprecate_service_data_ack

fix: send Service Data ACK frames only for version 2-4

2 files changed, 78 insertions, 21 deletions

diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index 35a3fb032a..23ac2927ef 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -1970,33 +1970,34 @@ void ProtocolHandlerImpl::SendFramesNumber(uint32_t connection_key,
   LOG4CXX_DEBUG(
       logger_, "SendFramesNumber MobileNaviAck for session " << connection_key);
 
-  // TODO(EZamakhov): add protocol version check - to avoid send for
-  // PROTOCOL_VERSION_1
   transport_manager::ConnectionUID connection_id = 0;
   uint8_t session_id = 0;
   session_observer_.PairFromKey(connection_key, &connection_id, &session_id);
   uint8_t protocol_version;
   if (session_observer_.ProtocolVersionUsed(
           connection_id, session_id, protocol_version)) {
-    ProtocolFramePtr ptr(
-        new protocol_handler::ProtocolPacket(connection_id,
-                                             protocol_version,
-                                             PROTECTION_OFF,
-                                             FRAME_TYPE_CONTROL,
-                                             SERVICE_TYPE_NAVI,
-                                             FRAME_DATA_SERVICE_DATA_ACK,
-                                             session_id,
-                                             0,
-                                             message_counters_[session_id]++));
-
-    // Flow control data shall be 4 bytes according Ford Protocol
-    DCHECK(sizeof(number_of_frames) == 4);
-    number_of_frames = LE_TO_BE32(number_of_frames);
-    ptr->set_data(reinterpret_cast<const uint8_t*>(&number_of_frames),
-                  sizeof(number_of_frames));
-    raw_ford_messages_to_mobile_.PostMessage(
-        impl::RawFordMessageToMobile(ptr, false));
-    LOG4CXX_DEBUG(logger_, "SendFramesNumber finished successfully");
+    if (protocol_version > PROTOCOL_VERSION_1 &&
+        protocol_version < PROTOCOL_VERSION_5) {
+      ProtocolFramePtr ptr(new protocol_handler::ProtocolPacket(
+          connection_id,
+          protocol_version,
+          PROTECTION_OFF,
+          FRAME_TYPE_CONTROL,
+          SERVICE_TYPE_NAVI,
+          FRAME_DATA_SERVICE_DATA_ACK,
+          session_id,
+          0,
+          message_counters_[session_id]++));
+
+      // Flow control data shall be 4 bytes according Ford Protocol
+      DCHECK(sizeof(number_of_frames) == 4);
+      number_of_frames = LE_TO_BE32(number_of_frames);
+      ptr->set_data(reinterpret_cast<const uint8_t*>(&number_of_frames),
+                    sizeof(number_of_frames));
+      raw_ford_messages_to_mobile_.PostMessage(
+          impl::RawFordMessageToMobile(ptr, false));
+      LOG4CXX_DEBUG(logger_, "SendFramesNumber finished successfully");
+    }
   } else {
     LOG4CXX_WARN(
         logger_,
diff --git a/src/components/protocol_handler/test/protocol_handler_tm_test.cc b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
index 89b9faedaa..d7850e4df1 100644
--- a/src/components/protocol_handler/test/protocol_handler_tm_test.cc
+++ b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
@@ -65,6 +65,8 @@ using protocol_handler::PROTECTION_OFF;
 using protocol_handler::PROTOCOL_VERSION_1;
 using protocol_handler::PROTOCOL_VERSION_2;
 using protocol_handler::PROTOCOL_VERSION_3;
+using protocol_handler::PROTOCOL_VERSION_4;
+using protocol_handler::PROTOCOL_VERSION_5;
 using protocol_handler::PROTOCOL_VERSION_MAX;
 using protocol_handler::FRAME_TYPE_CONTROL;
 using protocol_handler::FRAME_TYPE_SINGLE;
@@ -79,6 +81,7 @@ using protocol_handler::FRAME_DATA_END_SERVICE_ACK;
 using protocol_handler::FRAME_DATA_END_SERVICE;
 using protocol_handler::FRAME_DATA_HEART_BEAT;
 using protocol_handler::FRAME_DATA_HEART_BEAT_ACK;
+using protocol_handler::FRAME_DATA_SERVICE_DATA_ACK;
 using protocol_handler::FRAME_DATA_SINGLE;
 using protocol_handler::FRAME_DATA_FIRST;
 using protocol_handler::FRAME_DATA_LAST_CONSECUTIVE;
@@ -2143,6 +2146,59 @@ TEST_F(ProtocolHandlerImplTest, SendMessageToMobileApp_SendMultiframeMessage) {
   EXPECT_TRUE(waiter->WaitFor(times, kAsyncExpectationsTimeout));
 }
 
+TEST_F(ProtocolHandlerImplTest, SendServiceDataAck_PreVersion5) {
+  ::utils::SharedPtr<TestAsyncWaiter> waiter =
+      utils::MakeShared<TestAsyncWaiter>();
+  uint32_t times = 0;
+
+  AddSession(waiter, times);
+
+  EXPECT_CALL(session_observer_mock, PairFromKey(connection_key, _, _))
+      .WillOnce(
+          DoAll(SetArgPointee<1>(connection_id), SetArgPointee<2>(session_id)));
+  EXPECT_CALL(session_observer_mock, ProtocolVersionUsed(connection_id, _, _))
+      .WillRepeatedly(
+          DoAll(SetArgReferee<2>(PROTOCOL_VERSION_4), Return(true)));
+
+  EXPECT_CALL(transport_manager_mock,
+              SendMessageToDevice(ExpectedMessage(FRAME_TYPE_CONTROL,
+                                                  FRAME_DATA_SERVICE_DATA_ACK,
+                                                  PROTECTION_OFF,
+                                                  kMobileNav)))
+      .WillOnce(DoAll(NotifyTestAsyncWaiter(waiter), Return(E_SUCCESS)));
+  times++;
+
+  protocol_handler_impl->SendFramesNumber(connection_key, 0);
+
+  EXPECT_TRUE(waiter->WaitFor(times, kAsyncExpectationsTimeout));
+}
+
+TEST_F(ProtocolHandlerImplTest, SendServiceDataAck_AfterVersion5) {
+  ::utils::SharedPtr<TestAsyncWaiter> waiter =
+      utils::MakeShared<TestAsyncWaiter>();
+  uint32_t times = 0;
+
+  AddSession(waiter, times);
+
+  EXPECT_CALL(session_observer_mock, PairFromKey(connection_key, _, _))
+      .WillOnce(
+          DoAll(SetArgPointee<1>(connection_id), SetArgPointee<2>(session_id)));
+  EXPECT_CALL(session_observer_mock, ProtocolVersionUsed(connection_id, _, _))
+      .WillRepeatedly(
+          DoAll(SetArgReferee<2>(PROTOCOL_VERSION_5), Return(true)));
+
+  // It is expected that Service Data ACK is NOT sent for version 5+
+  EXPECT_CALL(transport_manager_mock,
+              SendMessageToDevice(ExpectedMessage(FRAME_TYPE_CONTROL,
+                                                  FRAME_DATA_SERVICE_DATA_ACK,
+                                                  PROTECTION_OFF,
+                                                  kMobileNav))).Times(0);
+
+  protocol_handler_impl->SendFramesNumber(connection_key, 0);
+
+  EXPECT_TRUE(waiter->WaitFor(times, kAsyncExpectationsTimeout));
+}
+
 }  // namespace protocol_handler_test
 }  // namespace components
 }  // namespace test