From 0fbc17f23cf4414c636ccc939c9b828e401459b3 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Tue, 26 Dec 2023 15:57:47 -0800 Subject: [PATCH] Make remoteaccess HAL pass VTS. The reference remote access HAL should still pass CTS even when the grpc remote access server does not exist. The reference remote access HAL now allows GRPC_SERVICE_ADDRESS not to be defined. If it is not defined, it will not try to connect a remote server and will work in a fake mode. Test: VtsHalAutomotiveRemoteAccess_TargetTest with an without grpc server running. Bug: 277967402 Change-Id: I89509ac8f8ebe9a268d3a338d6e80c24e39dc512 --- .../remoteaccess/hal/default/Android.bp | 4 +- .../hal/default/include/RemoteAccessService.h | 7 +- .../hal/default/src/RemoteAccessImpl.cpp | 18 +++-- .../hal/default/src/RemoteAccessService.cpp | 68 +++++++++++++++++-- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/automotive/remoteaccess/hal/default/Android.bp b/automotive/remoteaccess/hal/default/Android.bp index 97ed2c12f0..be6a425485 100644 --- a/automotive/remoteaccess/hal/default/Android.bp +++ b/automotive/remoteaccess/hal/default/Android.bp @@ -53,7 +53,9 @@ cc_binary { vintf_fragments: ["remoteaccess-default-service.xml"], init_rc: ["remoteaccess-default-service.rc"], cflags: [ - "-DGRPC_SERVICE_ADDRESS=\"10.0.2.2:50051\"", + // Uncomment this if running on emulator and connecting to a local grpc server + // running on host 127.0.0.1:50051 (TestWakeupClientServerHost) + // "-DGRPC_SERVICE_ADDRESS=\"10.0.2.2:50051\"", ], } diff --git a/automotive/remoteaccess/hal/default/include/RemoteAccessService.h b/automotive/remoteaccess/hal/default/include/RemoteAccessService.h index 6266de8419..8716e48bb9 100644 --- a/automotive/remoteaccess/hal/default/include/RemoteAccessService.h +++ b/automotive/remoteaccess/hal/default/include/RemoteAccessService.h @@ -111,6 +111,8 @@ class RemoteAccessService WakeupClient::StubInterface* mGrpcStub; std::thread mThread; + // Whether the GRPC server exists. Only checked and set during init. + bool mGrpcServerExist = false; std::mutex mLock; std::condition_variable mCv; std::shared_ptr @@ -121,7 +123,7 @@ class RemoteAccessService // A mutex to make sure startTaskLoop does not overlap with stopTaskLoop. std::mutex mStartStopTaskLoopLock; bool mTaskLoopRunning GUARDED_BY(mStartStopTaskLoopLock) = false; - bool mGrpcConnected GUARDED_BY(mLock) = false; + bool mGrpcReadChannelOpen GUARDED_BY(mLock) = false; std::unordered_map mClientIdToTaskCount GUARDED_BY(mLock); // Default wait time before retry connecting to remote access client is 10s. @@ -143,9 +145,10 @@ class RemoteAccessService void debugInjectTask(int fd, std::string_view clientId, std::string_view taskData); void debugInjectTaskNextReboot(int fd, std::string_view clientId, std::string_view taskData, const char* latencyInSecStr); - void updateGrpcConnected(bool connected); + void updateGrpcReadChannelOpen(bool grpcReadChannelOpen); android::base::Result deliverRemoteTaskThroughCallback(const std::string& clientId, std::string_view taskData); + bool isTaskScheduleSupported(); }; } // namespace remoteaccess diff --git a/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp b/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp index d4ba8641b7..28c5cd5cf2 100644 --- a/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp +++ b/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp @@ -30,10 +30,9 @@ constexpr char SERVICE_NAME[] = "android.hardware.automotive.remoteaccess.IRemoteAccess/default"; int main(int /* argc */, char* /* argv */[]) { -#ifndef GRPC_SERVICE_ADDRESS - LOG(ERROR) << "GRPC_SERVICE_ADDRESS is not defined, exiting"; - exit(1); -#endif + android::hardware::automotive::remoteaccess::WakeupClient::StubInterface* grpcStub = nullptr; + +#ifdef GRPC_SERVICE_ADDRESS LOG(INFO) << "Registering RemoteAccessService as service, server: " << GRPC_SERVICE_ADDRESS << "..."; grpc::ChannelArguments grpcargs = {}; @@ -47,11 +46,18 @@ int main(int /* argc */, char* /* argv */[]) { android::netdevice::waitFor({GRPC_SERVICE_IFNAME}, android::netdevice::WaitCondition::PRESENT_AND_UP); LOG(INFO) << "Waiting for interface: " << GRPC_SERVICE_IFNAME << " done"; -#endif +#endif // #ifdef GRPC_SERVICE_IFNAME auto channel = grpc::CreateChannel(GRPC_SERVICE_ADDRESS, grpc::InsecureChannelCredentials()); auto clientStub = android::hardware::automotive::remoteaccess::WakeupClient::NewStub(channel); + + grpcStub = clientStub.get(); + +#else + LOG(INFO) << "GRPC_SERVICE_ADDRESS is not defined, work in fake mode"; +#endif // #ifdef GRPC_SERVICE_ADDRESS + auto service = ndk::SharedRefBase::make< - android::hardware::automotive::remoteaccess::RemoteAccessService>(clientStub.get()); + android::hardware::automotive::remoteaccess::RemoteAccessService>(grpcStub); binder_exception_t err = AServiceManager_addService(service->asBinder().get(), SERVICE_NAME); if (err != EX_NONE) { diff --git a/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp b/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp index 1b42a1f091..dbd5bed7df 100644 --- a/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp +++ b/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp @@ -103,6 +103,10 @@ std::string boolToString(bool x) { RemoteAccessService::RemoteAccessService(WakeupClient::StubInterface* grpcStub) : mGrpcStub(grpcStub) { + if (mGrpcStub != nullptr) { + mGrpcServerExist = true; + } + std::ifstream debugTaskFile; debugTaskFile.open(DEBUG_TASK_FILE, std::ios::in); if (!debugTaskFile.is_open()) { @@ -177,9 +181,9 @@ void RemoteAccessService::maybeStopTaskLoop() { mTaskLoopRunning = false; } -void RemoteAccessService::updateGrpcConnected(bool connected) { +void RemoteAccessService::updateGrpcReadChannelOpen(bool grpcReadChannelOpen) { std::lock_guard lockGuard(mLock); - mGrpcConnected = connected; + mGrpcReadChannelOpen = grpcReadChannelOpen; } Result RemoteAccessService::deliverRemoteTaskThroughCallback(const std::string& clientId, @@ -213,7 +217,7 @@ void RemoteAccessService::runTaskLoop() { mGetRemoteTasksContext.reset(new ClientContext()); reader = mGrpcStub->GetRemoteTasks(mGetRemoteTasksContext.get(), request); } - updateGrpcConnected(true); + updateGrpcReadChannelOpen(true); GetRemoteTasksResponse response; while (reader->Read(&response)) { ALOGI("Receiving one task from remote task client"); @@ -225,7 +229,7 @@ void RemoteAccessService::runTaskLoop() { continue; } } - updateGrpcConnected(false); + updateGrpcReadChannelOpen(false); Status status = reader->Finish(); mGetRemoteTasksContext.reset(); @@ -298,6 +302,11 @@ ScopedAStatus RemoteAccessService::clearRemoteTaskCallback() { } ScopedAStatus RemoteAccessService::notifyApStateChange(const ApState& newState) { + if (!mGrpcServerExist) { + ALOGW("GRPC server does not exist, do nothing"); + return ScopedAStatus::ok(); + } + ClientContext context; NotifyWakeupRequiredRequest request = {}; request.set_iswakeuprequired(newState.isWakeupRequired); @@ -315,19 +324,40 @@ ScopedAStatus RemoteAccessService::notifyApStateChange(const ApState& newState) return ScopedAStatus::ok(); } +bool RemoteAccessService::isTaskScheduleSupported() { + if (!mGrpcServerExist) { + ALOGW("GRPC server does not exist, task scheduling not supported"); + return false; + } + + return true; +} + ScopedAStatus RemoteAccessService::isTaskScheduleSupported(bool* out) { - *out = true; + *out = isTaskScheduleSupported(); return ScopedAStatus::ok(); } ndk::ScopedAStatus RemoteAccessService::getSupportedTaskTypesForScheduling( std::vector* out) { + out->clear(); + if (!isTaskScheduleSupported()) { + ALOGW("Task scheduleing is not supported, return empty task types"); + return ScopedAStatus::ok(); + } + // TODO(b/316233421): support ENTER_GARAGE_MODE type. out->push_back(TaskType::CUSTOM); return ScopedAStatus::ok(); } ScopedAStatus RemoteAccessService::scheduleTask(const ScheduleInfo& scheduleInfo) { + if (!isTaskScheduleSupported()) { + ALOGW("Task scheduleing is not supported, return exception"); + return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT, + "task scheduling is not supported"); + } + ClientContext context; ScheduleTaskRequest request = {}; ScheduleTaskResponse response = {}; @@ -379,6 +409,11 @@ ScopedAStatus RemoteAccessService::scheduleTask(const ScheduleInfo& scheduleInfo ScopedAStatus RemoteAccessService::unscheduleTask(const std::string& clientId, const std::string& scheduleId) { + if (!isTaskScheduleSupported()) { + ALOGW("Task scheduleing is not supported, do nothing"); + return ScopedAStatus::ok(); + } + ClientContext context; UnscheduleTaskRequest request = {}; UnscheduleTaskResponse response = {}; @@ -392,6 +427,11 @@ ScopedAStatus RemoteAccessService::unscheduleTask(const std::string& clientId, } ScopedAStatus RemoteAccessService::unscheduleAllTasks(const std::string& clientId) { + if (!isTaskScheduleSupported()) { + ALOGW("Task scheduleing is not supported, do nothing"); + return ScopedAStatus::ok(); + } + ClientContext context; UnscheduleAllTasksRequest request = {}; UnscheduleAllTasksResponse response = {}; @@ -405,6 +445,12 @@ ScopedAStatus RemoteAccessService::unscheduleAllTasks(const std::string& clientI ScopedAStatus RemoteAccessService::isTaskScheduled(const std::string& clientId, const std::string& scheduleId, bool* out) { + if (!isTaskScheduleSupported()) { + ALOGW("Task scheduleing is not supported, return false"); + *out = false; + return ScopedAStatus::ok(); + } + ClientContext context; IsTaskScheduledRequest request = {}; IsTaskScheduledResponse response = {}; @@ -420,6 +466,12 @@ ScopedAStatus RemoteAccessService::isTaskScheduled(const std::string& clientId, ScopedAStatus RemoteAccessService::getAllPendingScheduledTasks(const std::string& clientId, std::vector* out) { + if (!isTaskScheduleSupported()) { + ALOGW("Task scheduleing is not supported, return empty array"); + out->clear(); + return ScopedAStatus::ok(); + } + ClientContext context; GetAllPendingScheduledTasksRequest request = {}; GetAllPendingScheduledTasksResponse response = {}; @@ -560,9 +612,11 @@ void RemoteAccessService::printCurrentStatus(int fd) { dprintf(fd, "\nRemoteAccess HAL status \n" "Remote task callback registered: %s\n" - "Task receiving GRPC connection established: %s\n" + "GRPC server exist: %s\n" + "GRPC read channel for receiving tasks open: %s\n" "Received task count by clientId: \n%s\n", - boolToString(mRemoteTaskCallback.get()).c_str(), boolToString(mGrpcConnected).c_str(), + boolToString(mRemoteTaskCallback.get()).c_str(), boolToString(mGrpcServerExist).c_str(), + boolToString(mGrpcReadChannelOpen).c_str(), clientIdToTaskCountToStringLocked().c_str()); }