From 2c37c11a82bd695c35a717352391e75db5f35185 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Mon, 20 May 2024 17:27:19 -0700 Subject: [PATCH] Implement Unsubscribe in vhal proxy. Implement the unsubscribe function in IVehicleHardware. This will stop the server from generating property update events for the specified [propId, areaId]. Test: atest GRPCVehicleHardwareUnitTest GRPCVehicleProxyServerUnitTest Flag: EXEMPT hal change Bug: 328316981 Merged-In: I35f4860eead0c8ec9b192657fe51cc0ff4319383 (cherry-picked from commit: cb00b1f816670c291044087db9f88cb76a2a9b3e) Change-Id: I35f4860eead0c8ec9b192657fe51cc0ff4319383 --- .../aidl/impl/grpc/GRPCVehicleHardware.cpp | 19 +++++++ .../aidl/impl/grpc/GRPCVehicleHardware.h | 2 + .../aidl/impl/grpc/GRPCVehicleProxyServer.cpp | 10 ++++ .../aidl/impl/grpc/GRPCVehicleProxyServer.h | 4 ++ .../aidl/impl/grpc/proto/VehicleServer.proto | 3 ++ .../grpc/test/GRPCVehicleHardwareUnitTest.cpp | 51 ++++++++++++++++++- .../test/GRPCVehicleProxyServerUnitTest.cpp | 19 +++++++ automotive/vehicle/aidl/impl/proto/Android.bp | 2 + .../vehicle/UnsubscribeRequest.proto | 24 +++++++++ 9 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 automotive/vehicle/aidl/impl/proto/android/hardware/automotive/vehicle/UnsubscribeRequest.proto diff --git a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.cpp b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.cpp index 2e5d2e46c3..f44573ac8e 100644 --- a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.cpp +++ b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.cpp @@ -206,6 +206,25 @@ aidlvhal::StatusCode GRPCVehicleHardware::subscribe(aidlvhal::SubscribeOptions o return static_cast(protoStatus.status_code()); } +aidlvhal::StatusCode GRPCVehicleHardware::unsubscribe(int32_t propId, int32_t areaId) { + proto::UnsubscribeRequest request; + ::grpc::ClientContext context; + proto::VehicleHalCallStatus protoStatus; + request.set_prop_id(propId); + request.set_area_id(areaId); + auto grpc_status = mGrpcStub->Unsubscribe(&context, request, &protoStatus); + if (!grpc_status.ok()) { + if (grpc_status.error_code() == ::grpc::StatusCode::UNIMPLEMENTED) { + // This is a legacy sever. Ignore unsubscribe request. + LOG(INFO) << __func__ << ": GRPC Unsubscribe is not supported by the server"; + return aidlvhal::StatusCode::OK; + } + LOG(ERROR) << __func__ << ": GRPC Unsubscribe Failed: " << grpc_status.error_message(); + return aidlvhal::StatusCode::INTERNAL_ERROR; + } + return static_cast(protoStatus.status_code()); +} + aidlvhal::StatusCode GRPCVehicleHardware::updateSampleRate(int32_t propId, int32_t areaId, float sampleRate) { ::grpc::ClientContext context; diff --git a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.h b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.h index bec957cc97..9750f621e9 100644 --- a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.h +++ b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleHardware.h @@ -85,6 +85,8 @@ class GRPCVehicleHardware : public IVehicleHardware { aidlvhal::StatusCode subscribe(aidlvhal::SubscribeOptions options) override; + aidlvhal::StatusCode unsubscribe(int32_t propId, int32_t areaId) override; + bool waitForConnected(std::chrono::milliseconds waitTime); protected: diff --git a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.cpp b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.cpp index 558ab2f250..a6abfa308b 100644 --- a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.cpp +++ b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.cpp @@ -175,6 +175,16 @@ GrpcVehicleProxyServer::GrpcVehicleProxyServer(std::string serverAddr, return ::grpc::Status::OK; } +::grpc::Status GrpcVehicleProxyServer::Unsubscribe(::grpc::ServerContext* context, + const proto::UnsubscribeRequest* request, + proto::VehicleHalCallStatus* status) { + int32_t propId = request->prop_id(); + int32_t areaId = request->area_id(); + const auto status_code = mHardware->unsubscribe(propId, areaId); + status->set_status_code(static_cast(status_code)); + return ::grpc::Status::OK; +} + ::grpc::Status GrpcVehicleProxyServer::CheckHealth(::grpc::ServerContext* context, const ::google::protobuf::Empty*, proto::VehicleHalCallStatus* status) { diff --git a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.h b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.h index 4f0c1e4bcb..dd9e2aad4c 100644 --- a/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.h +++ b/automotive/vehicle/aidl/impl/grpc/GRPCVehicleProxyServer.h @@ -60,6 +60,10 @@ class GrpcVehicleProxyServer : public proto::VehicleServer::Service { ::grpc::Status Subscribe(::grpc::ServerContext* context, const proto::SubscribeRequest* request, proto::VehicleHalCallStatus* status) override; + ::grpc::Status Unsubscribe(::grpc::ServerContext* context, + const proto::UnsubscribeRequest* request, + proto::VehicleHalCallStatus* status) override; + ::grpc::Status CheckHealth(::grpc::ServerContext* context, const ::google::protobuf::Empty*, proto::VehicleHalCallStatus* status) override; diff --git a/automotive/vehicle/aidl/impl/grpc/proto/VehicleServer.proto b/automotive/vehicle/aidl/impl/grpc/proto/VehicleServer.proto index d4b5642c06..732957f9f1 100644 --- a/automotive/vehicle/aidl/impl/grpc/proto/VehicleServer.proto +++ b/automotive/vehicle/aidl/impl/grpc/proto/VehicleServer.proto @@ -22,6 +22,7 @@ import "android/hardware/automotive/vehicle/DumpOptions.proto"; import "android/hardware/automotive/vehicle/DumpResult.proto"; import "android/hardware/automotive/vehicle/SubscribeRequest.proto"; import "android/hardware/automotive/vehicle/StatusCode.proto"; +import "android/hardware/automotive/vehicle/UnsubscribeRequest.proto"; import "android/hardware/automotive/vehicle/VehiclePropConfig.proto"; import "android/hardware/automotive/vehicle/VehiclePropValue.proto"; import "android/hardware/automotive/vehicle/VehiclePropValueRequest.proto"; @@ -43,4 +44,6 @@ service VehicleServer { rpc StartPropertyValuesStream(google.protobuf.Empty) returns (stream VehiclePropValues) {} rpc Subscribe(SubscribeRequest) returns (VehicleHalCallStatus) {} + + rpc Unsubscribe(UnsubscribeRequest) returns (VehicleHalCallStatus) {} } diff --git a/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleHardwareUnitTest.cpp b/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleHardwareUnitTest.cpp index 24f078e524..3bd7e0e56a 100644 --- a/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleHardwareUnitTest.cpp +++ b/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleHardwareUnitTest.cpp @@ -151,7 +151,7 @@ TEST_F(GRPCVehicleHardwareMockServerUnitTest, SubscribeLegacyServer) { EXPECT_CALL(*mGrpcStub, Subscribe(_, _, _)) .WillOnce(Return(::grpc::Status(::grpc::StatusCode::UNIMPLEMENTED, ""))); - aidlvhal::SubscribeOptions options; // Your options here (consider adding sample data) + aidlvhal::SubscribeOptions options; auto status = mHardware->subscribe(options); EXPECT_EQ(status, aidlvhal::StatusCode::OK); @@ -181,4 +181,53 @@ TEST_F(GRPCVehicleHardwareMockServerUnitTest, SubscribeProtoFailure) { EXPECT_EQ(status, aidlvhal::StatusCode::NOT_AVAILABLE_SPEED_LOW); } +TEST_F(GRPCVehicleHardwareMockServerUnitTest, Unsubscribe) { + proto::VehicleHalCallStatus protoStatus; + protoStatus.set_status_code(proto::StatusCode::OK); + proto::UnsubscribeRequest actualRequest; + + EXPECT_CALL(*mGrpcStub, Unsubscribe(_, _, _)) + .WillOnce(DoAll(SaveArg<1>(&actualRequest), SetArgPointee<2>(protoStatus), + Return(::grpc::Status::OK))); + + int32_t propId = 1; + int32_t areaId = 2; + auto status = mHardware->unsubscribe(propId, areaId); + + EXPECT_EQ(status, aidlvhal::StatusCode::OK); + EXPECT_EQ(actualRequest.prop_id(), propId); + EXPECT_EQ(actualRequest.area_id(), areaId); +} + +TEST_F(GRPCVehicleHardwareMockServerUnitTest, UnsubscribeLegacyServer) { + EXPECT_CALL(*mGrpcStub, Unsubscribe(_, _, _)) + .WillOnce(Return(::grpc::Status(::grpc::StatusCode::UNIMPLEMENTED, ""))); + + auto status = mHardware->unsubscribe(1, 2); + + EXPECT_EQ(status, aidlvhal::StatusCode::OK); +} + +TEST_F(GRPCVehicleHardwareMockServerUnitTest, UnsubscribeGrpcFailure) { + EXPECT_CALL(*mGrpcStub, Unsubscribe(_, _, _)) + .WillOnce(Return(::grpc::Status(::grpc::StatusCode::INTERNAL, "GRPC Error"))); + + auto status = mHardware->unsubscribe(1, 2); + + EXPECT_EQ(status, aidlvhal::StatusCode::INTERNAL_ERROR); +} + +TEST_F(GRPCVehicleHardwareMockServerUnitTest, UnsubscribeProtoFailure) { + proto::VehicleHalCallStatus protoStatus; + protoStatus.set_status_code(proto::StatusCode::NOT_AVAILABLE_SPEED_LOW); + + EXPECT_CALL(*mGrpcStub, Unsubscribe(_, _, _)) + .WillOnce(DoAll(SetArgPointee<2>(protoStatus), // Set the output status + Return(::grpc::Status::OK))); + + auto status = mHardware->unsubscribe(1, 2); + + EXPECT_EQ(status, aidlvhal::StatusCode::NOT_AVAILABLE_SPEED_LOW); +} + } // namespace android::hardware::automotive::vehicle::virtualization diff --git a/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleProxyServerUnitTest.cpp b/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleProxyServerUnitTest.cpp index 61dabb9ed2..ca5c2d5a2e 100644 --- a/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleProxyServerUnitTest.cpp +++ b/automotive/vehicle/aidl/impl/grpc/test/GRPCVehicleProxyServerUnitTest.cpp @@ -219,4 +219,23 @@ TEST(GRPCVehicleProxyServerUnitTest, SubscribeNotAvailable) { EXPECT_EQ(returnStatus.status_code(), proto::StatusCode::NOT_AVAILABLE); } +TEST(GRPCVehicleProxyServerUnitTest, Unsubscribe) { + auto mockHardware = std::make_unique(); + // We make sure this is alive inside the function scope. + MockVehicleHardware* mockHardwarePtr = mockHardware.get(); + GrpcVehicleProxyServer server = GrpcVehicleProxyServer("", std::move(mockHardware)); + ::grpc::ServerContext context; + proto::UnsubscribeRequest request; + proto::VehicleHalCallStatus returnStatus; + request.set_prop_id(1); + request.set_area_id(2); + + EXPECT_CALL(*mockHardwarePtr, unsubscribe(1, 2)).WillOnce(Return(aidlvhal::StatusCode::OK)); + + auto grpcStatus = server.Unsubscribe(&context, &request, &returnStatus); + + EXPECT_TRUE(grpcStatus.ok()); + EXPECT_EQ(returnStatus.status_code(), proto::StatusCode::OK); +} + } // namespace android::hardware::automotive::vehicle::virtualization diff --git a/automotive/vehicle/aidl/impl/proto/Android.bp b/automotive/vehicle/aidl/impl/proto/Android.bp index c636bb98d8..b2edf753b8 100644 --- a/automotive/vehicle/aidl/impl/proto/Android.bp +++ b/automotive/vehicle/aidl/impl/proto/Android.bp @@ -52,6 +52,7 @@ genrule { "android/hardware/automotive/vehicle/VehiclePropValueRequest.pb.h", "android/hardware/automotive/vehicle/SubscribeOptions.pb.h", "android/hardware/automotive/vehicle/SubscribeRequest.pb.h", + "android/hardware/automotive/vehicle/UnsubscribeRequest.pb.h", ], } @@ -78,6 +79,7 @@ genrule { "android/hardware/automotive/vehicle/VehiclePropValueRequest.pb.cc", "android/hardware/automotive/vehicle/SubscribeOptions.pb.cc", "android/hardware/automotive/vehicle/SubscribeRequest.pb.cc", + "android/hardware/automotive/vehicle/UnsubscribeRequest.pb.cc", ], } diff --git a/automotive/vehicle/aidl/impl/proto/android/hardware/automotive/vehicle/UnsubscribeRequest.proto b/automotive/vehicle/aidl/impl/proto/android/hardware/automotive/vehicle/UnsubscribeRequest.proto new file mode 100644 index 0000000000..314fbf0f65 --- /dev/null +++ b/automotive/vehicle/aidl/impl/proto/android/hardware/automotive/vehicle/UnsubscribeRequest.proto @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +syntax = "proto3"; + +package android.hardware.automotive.vehicle.proto; + +message UnsubscribeRequest { + int32 prop_id = 1; + int32 area_id = 2; +} \ No newline at end of file