From 6013ab56daed44ee2330e83aa9fb4852753616c6 Mon Sep 17 00:00:00 2001 From: "Andrii Kalinich (GitHub)" Date: Tue, 21 Apr 2020 14:10:11 -0700 Subject: Fix UpdateAppList/UpdateDeviceList spam (#3339) * Fix UpdateAppList/UpdateDeviceList spam There was noticed a continuous spam of update app and device list requests from SDL to HMI even if thre is no any devices connected and apps registered. After analysis, there was found a couple of places where logic should be updated: - In OnHMIStartedCooperation don't send update app list after refreshing cloud app information as this function able to trigger the same request itself when needed - When update app list timer is expired, don't send update app list if no any apps were registered or unregistered while timer was running - In transport manager, don't raise OnDeviceListUpdated event if no any devices were added/removed - When WebEngine device is created, it makes sense to connect it automatically and set its status to CONNECTED as its internal communication between SDL and HMI - In transport adapter, don't notify listeners via OnConnectionStatusUpdated() callbacks if device connection status was not actually changed All these actions together prevents all possible reasons of UpdateAppList/UpdateDeviceList spam observed before fix. * fixup! Fix UpdateAppList/UpdateDeviceList spam * fixup! fixup! Fix UpdateAppList/UpdateDeviceList spam * fixup! fixup! fixup! Fix UpdateAppList/UpdateDeviceList spam --- .../application_manager/src/application_manager_impl.cc | 9 +++++++-- .../connection_handler/src/connection_handler_impl.cc | 8 ++++++-- .../include/transport_manager/transport_manager_impl.h | 3 ++- .../transport_manager/src/transport_manager_impl.cc | 15 ++++++++++----- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index 5c44282d77..e6705a4d8e 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -898,7 +898,6 @@ void ApplicationManagerImpl::OnHMIStartedCooperation() { resume_controller().ResetLaunchTime(); RefreshCloudAppInformation(); - SendUpdateAppList(); } std::string ApplicationManagerImpl::PolicyIDByIconUrl(const std::string url) { @@ -4085,11 +4084,17 @@ bool ApplicationManagerImpl::IsHMICooperating() const { void ApplicationManagerImpl::OnApplicationListUpdateTimer() { LOG4CXX_DEBUG(logger_, "Application list update timer finished"); + const bool is_new_app_registered = registered_during_timer_execution_; registered_during_timer_execution_ = false; + apps_to_register_list_lock_ptr_->Acquire(); const bool trigger_ptu = apps_size_ != applications_.size(); apps_to_register_list_lock_ptr_->Release(); - SendUpdateAppList(); + + if (is_new_app_registered) { + SendUpdateAppList(); + } + GetPolicyHandler().OnAppsSearchCompleted(trigger_ptu); } diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc index 4b97aa0e0c..1ad938332a 100644 --- a/src/components/connection_handler/src/connection_handler_impl.cc +++ b/src/components/connection_handler/src/connection_handler_impl.cc @@ -1418,8 +1418,12 @@ void ConnectionHandlerImpl::ConnectToAllDevices() { sync_primitives::AutoReadLock lock(device_list_lock_); for (DeviceMap::iterator i = device_list_.begin(); i != device_list_.end(); ++i) { - connection_handler::DeviceHandle device_handle = i->first; - ConnectToDevice(device_handle); + if (transport_manager::webengine_constants::kWebEngineDeviceName == + i->second.user_friendly_name()) { + LOG4CXX_DEBUG(logger_, "No need to connect to web engine device"); + continue; + } + ConnectToDevice(i->first); } } diff --git a/src/components/transport_manager/include/transport_manager/transport_manager_impl.h b/src/components/transport_manager/include/transport_manager/transport_manager_impl.h index a765bb045a..85d96bac55 100644 --- a/src/components/transport_manager/include/transport_manager/transport_manager_impl.h +++ b/src/components/transport_manager/include/transport_manager/transport_manager_impl.h @@ -525,8 +525,9 @@ class TransportManagerImpl /** * @brief Updates total device list with info from specific transport adapter. * @param ta Transport adapter + * @return True if device list has been changed, otherwise - false */ - void UpdateDeviceList(TransportAdapter* ta); + bool UpdateDeviceList(TransportAdapter* ta); }; // class TransportManagerImpl } // namespace transport_manager #endif // SRC_COMPONENTS_TRANSPORT_MANAGER_INCLUDE_TRANSPORT_MANAGER_TRANSPORT_MANAGER_IMPL_H_ diff --git a/src/components/transport_manager/src/transport_manager_impl.cc b/src/components/transport_manager/src/transport_manager_impl.cc index 9d4f6be316..ddff4f3780 100644 --- a/src/components/transport_manager/src/transport_manager_impl.cc +++ b/src/components/transport_manager/src/transport_manager_impl.cc @@ -711,10 +711,8 @@ void TransportManagerImpl::CreateWebEngineDevice() { ws_device->set_keep_on_disconnect(true); - RaiseEvent(&TransportManagerListener::OnDeviceAdded, web_engine_device_info_); - device_list_.push_back( - std::make_pair(web_socket_ta, web_engine_device_info_)); web_socket_ta->AddDevice(ws_device); + OnDeviceListUpdated(web_socket_ta); #endif // WEBSOCKET_SERVER_TRANSPORT_SUPPORT } @@ -723,7 +721,7 @@ const DeviceInfo& TransportManagerImpl::GetWebEngineDeviceInfo() const { return web_engine_device_info_; } -void TransportManagerImpl::UpdateDeviceList(TransportAdapter* ta) { +bool TransportManagerImpl::UpdateDeviceList(TransportAdapter* ta) { LOG4CXX_TRACE(logger_, "enter. TransportAdapter: " << ta); std::set old_devices; std::set new_devices; @@ -775,7 +773,9 @@ void TransportManagerImpl::UpdateDeviceList(TransportAdapter* ta) { ++it) { RaiseEvent(&TransportManagerListener::OnDeviceRemoved, *it); } + LOG4CXX_TRACE(logger_, "exit"); + return added_devices.size() + removed_devices.size() > 0; } void TransportManagerImpl::PostMessage( @@ -1067,7 +1067,12 @@ void TransportManagerImpl::OnDeviceListUpdated(TransportAdapter* ta) { LOG4CXX_ERROR(logger_, "Device list update failed."); return; } - UpdateDeviceList(ta); + + if (!UpdateDeviceList(ta)) { + LOG4CXX_DEBUG(logger_, "Device list was not changed"); + return; + } + std::vector device_infos; device_list_lock_.AcquireForReading(); for (DeviceInfoList::const_iterator it = device_list_.begin(); -- cgit v1.2.1