Updated START_SESSION message interpretation

Removed the dependency on the fact that the core's new ID is always >= existing ID.
New ID and the old ID may not have any such relation.

Bug: 135475397
Fixes: 135475397
Test: Updated unit tests in VmsUtilsTest. Ran the tests on Hawk.

Change-Id: I5a7e72f36a84f5bf2d3c7bf1ccb89d27c5c0da97

3 files changed, 28 insertions, 43 deletions

diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VmsUtils.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VmsUtils.h
index 87010cea0c..4112ccc17a 100644
--- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VmsUtils.h
+++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VmsUtils.h
@@ -109,20 +109,13 @@ struct VmsAvailabilityState {
 
 // An enum to represent the result of parsing START_SESSION message from the VMS service.
 enum VmsSessionStatus {
-    // New server session is received if the new client ID is -1 and the new server ID is not an
-    // invalid ID.
+    // When a new session is received, the client should acknowledge it with the correct
+    // IDs in the START_SESSION message.
     kNewServerSession,
-    // Ack to new client session is received if the new client ID is same as the old one and the new
-    // server ID is not an invalid ID.
-    kAckToNewClientSession,
-    // Error codes:
+    // When an acknowledgement it received, the client can start using the connection.
+    kAckToCurrentSession,
     // Invalid message with either invalid format or unexpected data.
-    kInvalidMessage,
-    // Invalid server ID. New ID should always be greater than or equal to max_of(0, current server
-    // ID)
-    kInvalidServiceId,
-    // Invalid client ID. New ID should always be either -1 or the current client ID.
-    kInvalidClientId
+    kInvalidMessage
 };
 
 // Creates an empty base VMS message with some pre-populated default fields.
@@ -235,7 +228,7 @@ bool hasServiceNewlyStarted(const VehiclePropValue& availability_change);
 // Takes a start session message, current service ID, current client ID; and returns the type/status
 // of the message. It also populates the new service ID with the correct value.
 VmsSessionStatus parseStartSessionMessage(const VehiclePropValue& start_session,
-                                          const int service_id, const int client_id,
+                                          const int current_service_id, const int current_client_id,
                                           int* new_service_id);
 
 }  // namespace vms
diff --git a/automotive/vehicle/2.0/default/common/src/VmsUtils.cpp b/automotive/vehicle/2.0/default/common/src/VmsUtils.cpp
index f16489ce83..1c149d497a 100644
--- a/automotive/vehicle/2.0/default/common/src/VmsUtils.cpp
+++ b/automotive/vehicle/2.0/default/common/src/VmsUtils.cpp
@@ -272,27 +272,28 @@ bool hasServiceNewlyStarted(const VehiclePropValue& availability_change) {
 }
 
 VmsSessionStatus parseStartSessionMessage(const VehiclePropValue& start_session,
-                                          const int service_id, const int client_id,
+                                          const int current_service_id, const int current_client_id,
                                           int* new_service_id) {
     if (isValidVmsMessage(start_session) &&
         parseMessageType(start_session) == VmsMessageType::START_SESSION &&
         start_session.value.int32Values.size() == kSessionIdsSize + 1) {
         *new_service_id = start_session.value.int32Values[1];
         const int new_client_id = start_session.value.int32Values[2];
-        if (*new_service_id < std::max(0, service_id)) {
-            *new_service_id = service_id;
-            return VmsSessionStatus::kInvalidServiceId;
-        }
-        if (new_client_id == -1) {
+        if (new_client_id != current_client_id) {
+            // If the new_client_id = -1, it means the service has newly started.
+            // But if it is not -1 and is different than the current client ID, then
+            // it means that the service did not have the correct client ID. In
+            // both these cases, the client should acknowledge with a START_SESSION
+            // message containing the correct client ID. So here, the status is returned as
+            // kNewServerSession.
             return VmsSessionStatus::kNewServerSession;
+        } else {
+            // kAckToCurrentSession is returned if the new client ID is same as the current one.
+            return VmsSessionStatus::kAckToCurrentSession;
         }
-        if (new_client_id == client_id) {
-            return VmsSessionStatus::kAckToNewClientSession;
-        }
-        *new_service_id = service_id;
-        return VmsSessionStatus::kInvalidClientId;
     }
-    *new_service_id = service_id;
+    // If the message is invalid then persist the old service ID.
+    *new_service_id = current_service_id;
     return VmsSessionStatus::kInvalidMessage;
 }
 
diff --git a/automotive/vehicle/2.0/default/tests/VmsUtils_test.cpp b/automotive/vehicle/2.0/default/tests/VmsUtils_test.cpp
index 8a434c85b8..474f6af3bb 100644
--- a/automotive/vehicle/2.0/default/tests/VmsUtils_test.cpp
+++ b/automotive/vehicle/2.0/default/tests/VmsUtils_test.cpp
@@ -371,25 +371,25 @@ TEST(VmsUtilsTest, startSessionClientNewlyStarted) {
     int new_service_id;
     message->value.int32Values = hidl_vec<int32_t>{toInt(VmsMessageType::START_SESSION), 123, 456};
     EXPECT_EQ(parseStartSessionMessage(*message, -1, 456, &new_service_id),
-              VmsSessionStatus::kAckToNewClientSession);
+              VmsSessionStatus::kAckToCurrentSession);
     EXPECT_EQ(new_service_id, 123);
 }
 
-TEST(VmsUtilsTest, startSessionClientNewlyStartedWithSameServerId) {
+TEST(VmsUtilsTest, startSessionClientNewlyStartedWithSameServerAndClientId) {
     auto message = createBaseVmsMessage(3);
     int new_service_id;
     message->value.int32Values = hidl_vec<int32_t>{toInt(VmsMessageType::START_SESSION), 123, 456};
     EXPECT_EQ(parseStartSessionMessage(*message, 123, 456, &new_service_id),
-              VmsSessionStatus::kAckToNewClientSession);
+              VmsSessionStatus::kAckToCurrentSession);
     EXPECT_EQ(new_service_id, 123);
 }
 
-TEST(VmsUtilsTest, startSessionClientNewlyStartedEdgeCase) {
+TEST(VmsUtilsTest, startSessionWithZeroAsIds) {
     auto message = createBaseVmsMessage(3);
     int new_service_id;
     message->value.int32Values = hidl_vec<int32_t>{toInt(VmsMessageType::START_SESSION), 0, 0};
     EXPECT_EQ(parseStartSessionMessage(*message, 0, 0, &new_service_id),
-              VmsSessionStatus::kAckToNewClientSession);
+              VmsSessionStatus::kAckToCurrentSession);
     EXPECT_EQ(new_service_id, 0);
 }
 
@@ -398,28 +398,19 @@ TEST(VmsUtilsTest, startSessionOldServiceId) {
     int new_service_id;
     message->value.int32Values = hidl_vec<int32_t>{toInt(VmsMessageType::START_SESSION), 120, 456};
     EXPECT_EQ(parseStartSessionMessage(*message, 123, 456, &new_service_id),
-              VmsSessionStatus::kInvalidServiceId);
-    EXPECT_EQ(new_service_id, 123);
+              VmsSessionStatus::kAckToCurrentSession);
+    EXPECT_EQ(new_service_id, 120);
 }
 
-TEST(VmsUtilsTest, startSessionInvalidServiceIdEdgeCase) {
+TEST(VmsUtilsTest, startSessionNegativeServerId) {
     auto message = createBaseVmsMessage(3);
     int new_service_id;
     message->value.int32Values = hidl_vec<int32_t>{toInt(VmsMessageType::START_SESSION), -1, 456};
     EXPECT_EQ(parseStartSessionMessage(*message, -1, 456, &new_service_id),
-              VmsSessionStatus::kInvalidServiceId);
+              VmsSessionStatus::kAckToCurrentSession);
     EXPECT_EQ(new_service_id, -1);
 }
 
-TEST(VmsUtilsTest, startSessionInvalidClientId) {
-    auto message = createBaseVmsMessage(3);
-    int new_service_id;
-    message->value.int32Values = hidl_vec<int32_t>{toInt(VmsMessageType::START_SESSION), 123, 457};
-    EXPECT_EQ(parseStartSessionMessage(*message, 123, 456, &new_service_id),
-              VmsSessionStatus::kInvalidClientId);
-    EXPECT_EQ(new_service_id, 123);
-}
-
 TEST(VmsUtilsTest, startSessionInvalidMessageFormat) {
     auto message = createBaseVmsMessage(2);
     int new_service_id;