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 @@ -1598,6 +1598,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 @@ -422,6 +421,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 PushAppIdToQueue(const uint32_t new_app_id);
ShobhitAd marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief Remove the first application from applications queue
*/
void PopAppIdFromQueue();
ShobhitAd marked this conversation as resolved.
Show resolved Hide resolved

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

Expand Down Expand Up @@ -583,6 +594,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> queue_applications_for_ptu_;
ShobhitAd marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief OnAppRegisteredOnMobile allows to handle event when application were
* succesfully registered on mobile device.
Expand Down Expand Up @@ -880,6 +899,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 @@ -894,6 +914,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 @@ -193,6 +193,7 @@ ApplicationManagerImpl::ApplicationManagerImpl(
this, &ApplicationManagerImpl::OnTimerSendTTSGlobalProperties))
, 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 @@ -3782,7 +3783,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 @@ -4244,6 +4245,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
53 changes: 45 additions & 8 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_("") {}

PolicyHandler::~PolicyHandler() {}

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

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

void PolicyHandler::PopAppIdFromQueue() {
LOG4CXX_AUTO_TRACE(logger_);
sync_primitives::AutoLock lock(app_id_queue_lock_);
if (queue_applications_for_ptu_.size() > 0) {
queue_applications_for_ptu_.erase(queue_applications_for_ptu_.begin());
policy_manager_->OnChangeApplicationCount(
queue_applications_for_ptu_.size());
}
}

#ifdef EXTERNAL_PROPRIETARY_MODE
PTURetryHandler& PolicyHandler::ptu_retry_handler() const {
LOG4CXX_AUTO_TRACE(logger_);
Expand Down Expand Up @@ -1116,7 +1137,7 @@ bool PolicyHandler::ReceiveMessageFromSDK(const std::string& file,
const bool is_ptu_successful =
load_pt_result == PolicyManager::PtProcessingResult::kSuccess;
OnPTUFinished(is_ptu_successful);

PopAppIdFromQueue();
if (is_ptu_successful) {
LOG4CXX_INFO(logger_, "PTU was successful.");
policy_manager_->CleanupUnpairedDevices();
Expand Down Expand Up @@ -1492,11 +1513,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() {
PopAppIdFromQueue();
#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 @@ -1539,9 +1562,11 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string,
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) {
jacobkeeler marked this conversation as resolved.
Show resolved Hide resolved
if (!SaveSnapshot(pt_string, policy_snapshot_full_path)) {
LOG4CXX_ERROR(logger_, "Snapshot processing skipped.");
return;
}
}

if (PTUIterationType::RetryIteration == iteration_type) {
Expand Down Expand Up @@ -1907,7 +1932,9 @@ void PolicyHandler::OnAuthTokenUpdated(const std::string& policy_app_id,
void PolicyHandler::OnPTUFinished(const bool ptu_result) {
LOG4CXX_AUTO_TRACE(logger_);
sync_primitives::AutoLock lock(listeners_lock_);

if (!ptu_result) {
PopAppIdFromQueue();
}
std::for_each(
listeners_.begin(),
listeners_.end(),
Expand Down Expand Up @@ -2205,6 +2232,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;
PushAppIdToQueue(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 @@ -410,6 +410,14 @@ class PolicyHandlerInterface : public VehicleDataItemProvider {
*/
virtual void OnAppsSearchCompleted(const bool trigger_ptu) = 0;

/**
* @brief Notify that new applocation was added to application list
Copy link
Contributor

Choose a reason for hiding this comment

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

applocation typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@ydementieiev still typo...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* @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 @@ -652,6 +660,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 OnChangeApplicationCount(const uint32_t new_app_count) = 0;
ShobhitAd marked this conversation as resolved.
Show resolved Hide resolved

/**
* @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 @@ -121,7 +121,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 OnChangeApplicationCount(const uint32_t new_app_count) = 0;
ShobhitAd marked this conversation as resolved.
Show resolved Hide resolved

/**
* @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 @@ -219,6 +219,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 @@ -316,6 +318,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 @@ -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(OnChangeApplicationCount, 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 @@ -210,6 +210,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(OnChangeApplicationCount, 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,12 @@ class PolicyManagerImpl : public PolicyManager {
*/
void OnAppsSearchCompleted(const bool trigger_ptu) OVERRIDE;

/**
* @brief Change applications count ready for PTU
* @param new_app_count new applications count for PTU
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

@ydementieiev Remove description form derived classes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

void OnChangeApplicationCount(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 @@ -1096,6 +1102,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 @@ -1305,6 +1316,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 @@ -1344,6 +1360,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 @@ -55,7 +55,8 @@ enum UpdateEvent {
kPendingUpdate,
kScheduleManualUpdate,
kOnResetRetrySequence,
kNoEvent
kNoEvent,
kUpdateForNextInQueue
jacobkeeler marked this conversation as resolved.
Show resolved Hide resolved
};

const std::string kUpToDate = "UP_TO_DATE";
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
Loading