Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start the new PTU after a failed retry sequence #3208

Merged
merged 12 commits into from
Feb 28, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,8 @@ class ApplicationManagerImpl

uint32_t apps_size_;

std::atomic<bool> registered_during_timer_execution_;

volatile bool is_stopping_;

std::unique_ptr<CommandHolder> commands_holder_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,8 @@ class PolicyHandler : public PolicyHandlerInterface,
const std::string& policy_app_id,
const std::string& hmi_level) OVERRIDE;

#ifndef EXTERNAL_PROPRIETARY_MODE
void OnPTUTimeOut() OVERRIDE;
#endif

/**
* Gets all allowed module types
* @param app_id unique identifier of application
Expand Down Expand Up @@ -424,6 +423,18 @@ class PolicyHandler : public PolicyHandlerInterface,
*/
uint32_t GetAppIdForSending() const OVERRIDE;

/**
* @brief Add application to PTU queue if no application with
* the same app id exists
* @param new_app_id app id new application
*/
void PushAppIdToPTUQueue(const uint32_t new_app_id);

/**
* @brief Remove the first application from applications queue
*/
void PopAppIdFromPTUQueue();

custom_str::CustomString GetAppName(
const std::string& policy_app_id) OVERRIDE;

Expand Down Expand Up @@ -588,6 +599,14 @@ class PolicyHandler : public PolicyHandlerInterface,
*/
void OnAppsSearchCompleted(const bool trigger_ptu) OVERRIDE;

void OnAddedNewApplicationToAppList(const uint32_t new_app_id,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ydementieiev Move description to iface

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar leaved description only in iface c04c115ce

const std::string& policy_id) OVERRIDE;

/**
* @brief Queue applications for which PTU has not yet been completed
*/
std::set<uint32_t> applications_ptu_queue_;

/**
* @brief OnAppRegisteredOnMobile allows to handle event when application were
* succesfully registered on mobile device.
Expand Down Expand Up @@ -885,6 +904,7 @@ class PolicyHandler : public PolicyHandlerInterface,
typedef std::list<PolicyHandlerObserver*> HandlersCollection;
HandlersCollection listeners_;
mutable sync_primitives::Lock listeners_lock_;
mutable sync_primitives::Lock app_id_queue_lock_;

/**
* @brief Application-to-device links are used for collecting their current
Expand All @@ -899,6 +919,8 @@ class PolicyHandler : public PolicyHandlerInterface,
std::shared_ptr<StatisticManagerImpl> statistic_manager_impl_;
const PolicySettings& settings_;
application_manager::ApplicationManager& application_manager_;
std::string last_registered_policy_app_id_;

friend class AppPermissionDelegate;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ ApplicationManagerImpl::ApplicationManagerImpl(
this, &ApplicationManagerImpl::ClearTimerPool))
, is_low_voltage_(false)
, apps_size_(0)
, registered_during_timer_execution_(false)
, is_stopping_(false) {
std::srand(std::time(nullptr));
AddPolicyObserver(this);
Expand Down Expand Up @@ -3967,7 +3968,7 @@ bool ApplicationManagerImpl::IsHMICooperating() const {

void ApplicationManagerImpl::OnApplicationListUpdateTimer() {
LOG4CXX_DEBUG(logger_, "Application list update timer finished");

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();
Expand Down Expand Up @@ -4429,6 +4430,12 @@ void ApplicationManagerImpl::AddAppToRegisteredAppList(
logger_,
"App with app_id: " << application->app_id()
<< " has been added to registered applications list");
if (application_list_update_timer_.is_running() &&
!registered_during_timer_execution_) {
GetPolicyHandler().OnAddedNewApplicationToAppList(
application->app_id(), application->policy_app_id());
registered_during_timer_execution_ = true;
}
apps_size_ = static_cast<uint32_t>(applications_.size());
}

Expand Down
51 changes: 42 additions & 9 deletions src/components/application_manager/src/policies/policy_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,8 @@ PolicyHandler::PolicyHandler(const PolicySettings& settings,
, last_activated_app_id_(0)
, statistic_manager_impl_(std::make_shared<StatisticManagerImpl>(this))
, settings_(settings)
, application_manager_(application_manager) {}
, application_manager_(application_manager)
, last_registered_policy_app_id_(std::string()) {}

PolicyHandler::~PolicyHandler() {}

Expand Down Expand Up @@ -465,6 +466,24 @@ uint32_t PolicyHandler::GetAppIdForSending() const {
return ChooseRandomAppForPolicyUpdate(apps_with_none_level);
}

void PolicyHandler::PushAppIdToPTUQueue(const uint32_t app_id) {
LOG4CXX_AUTO_TRACE(logger_);
sync_primitives::AutoLock lock(app_id_queue_lock_);
const auto result = applications_ptu_queue_.insert(app_id);
if (result.second) {
policy_manager_->UpdatePTUReadyAppsCount(applications_ptu_queue_.size());
}
}

void PolicyHandler::PopAppIdFromPTUQueue() {
LOG4CXX_AUTO_TRACE(logger_);
sync_primitives::AutoLock lock(app_id_queue_lock_);
if (applications_ptu_queue_.size() > 0) {
applications_ptu_queue_.erase(applications_ptu_queue_.begin());
policy_manager_->UpdatePTUReadyAppsCount(applications_ptu_queue_.size());
}
}

#ifdef EXTERNAL_PROPRIETARY_MODE
PTURetryHandler& PolicyHandler::ptu_retry_handler() const {
LOG4CXX_AUTO_TRACE(logger_);
Expand Down Expand Up @@ -1508,11 +1527,13 @@ void PolicyHandler::OnPermissionsUpdated(const std::string& device_id,
<< policy_app_id << " and connection_key "
<< app->app_id());
}
#ifndef EXTERNAL_PROPRIETARY_MODE

void PolicyHandler::OnPTUTimeOut() {
PopAppIdFromPTUQueue();
#ifndef EXTERNAL_PROPRIETARY_MODE
application_manager_.protocol_handler().ProcessFailedPTU();
}
#endif
}

bool PolicyHandler::SaveSnapshot(const BinaryMessage& pt_string,
std::string& snap_path) {
Expand Down Expand Up @@ -1554,12 +1575,6 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string,
LOG4CXX_AUTO_TRACE(logger_);
POLICY_LIB_CHECK_VOID();
#ifdef PROPRIETARY_MODE
std::string policy_snapshot_full_path;
if (!SaveSnapshot(pt_string, policy_snapshot_full_path)) {
LOG4CXX_ERROR(logger_, "Snapshot processing skipped.");
return;
}

if (PTUIterationType::RetryIteration == iteration_type) {
uint32_t app_id_for_sending = GetAppIdForSending();

Expand All @@ -1569,6 +1584,12 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string,
}

} else {
std::string policy_snapshot_full_path;
if (!SaveSnapshot(pt_string, policy_snapshot_full_path)) {
LOG4CXX_ERROR(logger_, "Snapshot processing skipped.");
return;
}

MessageHelper::SendPolicyUpdate(
policy_snapshot_full_path,
TimeoutExchangeSec(),
Expand Down Expand Up @@ -1922,6 +1943,8 @@ void PolicyHandler::OnPTUFinished(const bool ptu_result) {
LOG4CXX_AUTO_TRACE(logger_);
sync_primitives::AutoLock lock(listeners_lock_);

PopAppIdFromPTUQueue();

std::for_each(
listeners_.begin(),
listeners_.end(),
Expand Down Expand Up @@ -2373,6 +2396,16 @@ void PolicyHandler::OnAppsSearchCompleted(const bool trigger_ptu) {
policy_manager_->OnAppsSearchCompleted(trigger_ptu);
}

void PolicyHandler::OnAddedNewApplicationToAppList(
const uint32_t new_app_id, const std::string& policy_id) {
LOG4CXX_AUTO_TRACE(logger_);
if (policy_id == last_registered_policy_app_id_) {
return;
}
last_registered_policy_app_id_ = policy_id;
PushAppIdToPTUQueue(new_app_id);
}

void PolicyHandler::OnAppRegisteredOnMobile(const std::string& device_id,
const std::string& application_id) {
POLICY_LIB_CHECK_VOID();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,14 @@ class PolicyHandlerInterface : public VehicleDataItemProvider {
*/
virtual void OnAppsSearchCompleted(const bool trigger_ptu) = 0;

/**
* @brief Notify that new application was added to application list
* @param new_app_id app_id for this application
* @param policy_id policy_id for this application
*/
virtual void OnAddedNewApplicationToAppList(const uint32_t new_app_id,
const std::string& policy_id) = 0;

/**
* @brief OnAppRegisteredOnMobile allows to handle event when application were
* succesfully registered on mobile device.
Expand Down Expand Up @@ -700,6 +708,12 @@ class PolicyHandlerInterface : public VehicleDataItemProvider {
const std::string& policy_app_id,
const std::string& hmi_level) = 0;

/**
* @brief OnPTUTimeOut the callback which is performed when PTU timeout
* occurred
*/
virtual void OnPTUTimeOut() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ydementieiev please add a description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/**
* Gets all allowed module types
* @param app_id unique identifier of application
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,12 @@ class PolicyManager : public usage_statistics::StatisticsManager,
*/
virtual void OnAppsSearchCompleted(const bool trigger_ptu) = 0;

/**
* @brief Change applications count ready for PTU
* @param new_app_count new applications count for PTU
*/
virtual void UpdatePTUReadyAppsCount(const uint32_t new_app_count) = 0;

/**
* @brief OnAppRegisteredOnMobile allows to handle event when application were
* succesfully registered on mobile device.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ class PolicyListener {
virtual void OnCertificateUpdated(const std::string& certificate_data) = 0;

/**
* @brief OnPTUTimeOut the callback which signals if PTU timeout occured
* @brief OnPTUTimeOut the callback which is performed when PTU timeout
* occurred
*/
virtual void OnPTUTimeOut() = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,12 @@ class PolicyManager : public usage_statistics::StatisticsManager,
*/
virtual void OnAppsSearchCompleted(const bool trigger_ptu) = 0;

/**
* @brief Change applications count ready for PTU
* @param new_app_count new applications count for PTU
*/
virtual void UpdatePTUReadyAppsCount(const uint32_t new_app_count) = 0;

/**
* @brief Get state of request types for given application
* @param policy_app_id Unique application id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface {
MOCK_CONST_METHOD1(HeartBeatTimeout, uint32_t(const std::string& app_id));
MOCK_METHOD0(OnAppsSearchStarted, void());
MOCK_METHOD1(OnAppsSearchCompleted, void(const bool trigger_ptu));
MOCK_METHOD2(OnAddedNewApplicationToAppList,
void(const uint32_t new_app_id, const std::string& policy_id));
MOCK_METHOD2(OnAppRegisteredOnMobile,
void(const std::string& device_id,
const std::string& application_id));
Expand Down Expand Up @@ -324,6 +326,7 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface {
void(const std::string& device_id,
const std::string& policy_app_id,
const std::string& hmi_level));
MOCK_METHOD0(OnPTUTimeOut, void());
MOCK_CONST_METHOD2(GetModuleTypes,
bool(const std::string& policy_app_id,
std::vector<std::string>* modules));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ class MockPolicyManager : public PolicyManager {
MOCK_METHOD1(SaveUpdateStatusRequired, void(bool is_update_needed));
MOCK_METHOD0(OnAppsSearchStarted, void());
MOCK_METHOD1(OnAppsSearchCompleted, void(const bool trigger_ptu));
MOCK_METHOD1(UpdatePTUReadyAppsCount, void(const uint32_t new_app_count));
MOCK_METHOD2(OnAppRegisteredOnMobile,
void(const std::string& device_id,
const std::string& application_id));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ class MockPolicyManager : public PolicyManager {
MOCK_METHOD1(SaveUpdateStatusRequired, void(bool is_update_needed));
MOCK_METHOD0(OnAppsSearchStarted, void());
MOCK_METHOD1(OnAppsSearchCompleted, void(const bool trigger_ptu));
MOCK_METHOD1(UpdatePTUReadyAppsCount, void(const uint32_t new_app_count));
MOCK_METHOD2(OnAppRegisteredOnMobile,
void(const std::string& device_id,
const std::string& application_id));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,8 @@ class PolicyManagerImpl : public PolicyManager {
*/
void OnAppsSearchCompleted(const bool trigger_ptu) OVERRIDE;

void UpdatePTUReadyAppsCount(const uint32_t new_app_count) OVERRIDE;

/**
* @brief Get state of request types for given application
* @param policy_app_id Unique application id
Expand Down Expand Up @@ -1079,6 +1081,11 @@ class PolicyManagerImpl : public PolicyManager {
bool IsPTValid(std::shared_ptr<policy_table::Table> policy_table,
policy_table::PolicyTableType type) const;

/**
* @brief Check that new applications for PTU were registered
*/
bool HasApplicationForPTU() const;

/**
* @brief Get resulting RPCs permissions for application which started on
* specific device
Expand Down Expand Up @@ -1293,6 +1300,11 @@ class PolicyManagerImpl : public PolicyManager {
*/
uint32_t retry_sequence_index_;

/**
* @brief Applications pending count ready for PTU
*/
uint32_t applications_pending_ptu_count_;

/**
* @brief Lock for guarding retry sequence
*/
Expand Down Expand Up @@ -1334,6 +1346,11 @@ class PolicyManagerImpl : public PolicyManager {
*/
bool trigger_ptu_;

/**
* @brief Flag for notifying that PTU was requested
*/
bool ptu_requested_;

/**
* @brief Flag that indicates whether a PTU sequence (including retries) is in
* progress
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ class UpdateStatusManager {
*/
void OnUpdateTimeoutOccurs();

/**
* @brief Update status for next in queue application
* after previous update been has finished
*/
void OnUpdateForNextInQueue();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ydementieiev please update description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/**
* @brief Update status handler for valid PTU receiving
*/
Expand Down Expand Up @@ -222,6 +228,7 @@ class UpdateStatusManager {
UpdateEvent last_processed_event_;
bool apps_search_in_progress_;
bool app_registered_from_non_consented_device_;
bool last_update_was_failed_;
sync_primitives::Lock apps_search_in_progress_lock_;

class UpdateThreadDelegate : public threads::ThreadDelegate {
Expand Down
Loading