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
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 87010ce..4112ccc 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 @@
// 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 @@
// 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 f16489c..1c149d4 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 @@
}
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 8a434c8..474f6af 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 @@
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 @@
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;