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

[SDL-0189] Restructuring OnResetTimeout #3726

Merged
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1011d00
Add Reset Timeout Handler, remove deprecated functionality, create Re…
ydementieiev Apr 9, 2020
97e5527
fixup! Add Reset Timeout Handler, remove deprecated functionality, cr…
Jun 4, 2020
95225be
Merge branch 'develop' into feature/restructuring_OnResetTimeout
ychernysheva Jun 1, 2021
dd59333
Fix compilation errors after merge of develop branch
ychernysheva Jun 2, 2021
91b4dfc
Remove deprecated functions from SubtleAlert request and remove redun…
ychernysheva Jun 2, 2021
cb3cf2a
Add changes to HMI API
ychernysheva Jun 2, 2021
493b960
Increase timeout in case if SIVD need Consent popup
ychernysheva Jun 8, 2021
266abf2
Increase timeout for ButtonPress request
ychernysheva Jun 9, 2021
70a4e4f
Restrict timeout for Subtle Alert with soft buttons
ychernysheva Jun 10, 2021
970427a
update timeout for hmi request
VadymLuchko Jun 10, 2021
1b7415d
Merge pull request #187 from LuxoftSDL/fix/OnResetTimeout_Notification
VadymLuchko Jun 10, 2021
943b5de
Merge pull request #188 from LuxoftSDL/fix/restrict_timeout_for_subtl…
dboltovskyi Jun 10, 2021
57d7846
careful stopping of the request controller
VadymLuchko Jun 17, 2021
c9ae084
Merge pull request #190 from LuxoftSDL/fix/crash_SDL_on_close
dboltovskyi Jun 22, 2021
aef4c59
Fix OnResetTimeout for Alerts with soft buttons (#192)
VadymLuchko Jun 25, 2021
f27c548
Revert double timeouts from core
AKalinich-Luxoft Jun 28, 2021
fd71265
Apply smaller timeout to requests
AKalinich-Luxoft Jun 28, 2021
25c3fb1
Merge pull request #193 from LuxoftSDL/fix/restructuring_onresettimeo…
AKalinich-Luxoft Jun 29, 2021
57e4acc
Address Livio comments
AKalinich-Luxoft Jul 16, 2021
031c65f
Add timeout compensation parameter
AKalinich-Luxoft Jul 29, 2021
d63e933
Fix affected unit tests
AKalinich-Luxoft Jul 30, 2021
55d7500
Address Livio comments
AKalinich-Luxoft Aug 6, 2021
3867df8
Merge remote-tracking branch 'origin/develop' into feature/restructur…
AKalinich-Luxoft Aug 6, 2021
228dbe2
Update rpc_spec
AKalinich-Luxoft Aug 6, 2021
c011a9c
Update rpc_spec one more time
AKalinich-Luxoft Aug 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
#include "application_manager/hmi_interfaces_impl.h"
#include "application_manager/message.h"
#include "application_manager/message_helper.h"
#include "application_manager/request_controller.h"
#include "application_manager/resumption/resume_ctrl.h"
#include "application_manager/rpc_handler.h"
#include "application_manager/rpc_service.h"
Expand Down Expand Up @@ -787,14 +786,14 @@ class ApplicationManagerImpl
*
* @param ptr Reference to shared pointer that point on hmi notification
*/
void addNotification(const CommandSharedPtr ptr);
void AddNotification(const CommandSharedPtr ptr);

/**
* @ Add notification to collection
*
* @param ptr Reference to shared pointer that point on hmi notification
* @param notification Pointer that points to hmi notification
*/
void removeNotification(const commands::Command* notification);
void RemoveNotification(const commands::Command* notification);

/**
* @ Updates request timeout
Expand All @@ -803,7 +802,7 @@ class ApplicationManagerImpl
* @param mobile_correlation_id Correlation ID of the mobile request
* @param new_timeout_value New timeout in milliseconds to be set
*/
void updateRequestTimeout(uint32_t connection_key,
void UpdateRequestTimeout(uint32_t connection_key,
uint32_t mobile_correlation_id,
uint32_t new_timeout_value) OVERRIDE;

Expand Down Expand Up @@ -999,6 +998,18 @@ class ApplicationManagerImpl
return *rpc_handler_;
}

request_controller::RequestTimeoutHandler& get_request_timeout_handler()
const OVERRIDE {
DCHECK(request_timeout_handler_);
return *request_timeout_handler_;
}

request_controller::RequestController& get_request_controller()
const OVERRIDE {
DCHECK(request_ctrl_);
return *request_ctrl_;
}

void SetRPCService(std::unique_ptr<rpc_service::RPCService>& rpc_service) {
rpc_service_ = std::move(rpc_service);
}
Expand Down Expand Up @@ -1602,8 +1613,10 @@ class ApplicationManagerImpl
connection_handler::ConnectionHandler* connection_handler_;
std::unique_ptr<policy::PolicyHandlerInterface> policy_handler_;
protocol_handler::ProtocolHandler* protocol_handler_;
std::unique_ptr<request_controller::RequestTimeoutHandler>
request_timeout_handler_;
std::unique_ptr<request_controller::RequestController> request_ctrl_;
std::unique_ptr<plugin_manager::RPCPluginManager> plugin_manager_;
request_controller::RequestController request_ctrl_;
std::unique_ptr<application_manager::AppServiceManager> app_service_manager_;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,13 @@ class CommandRequestImpl : public CommandImpl,
*/
void AddTimeOutComponentInfoToMessage(
smart_objects::SmartObject& response) const;
/**
* @brief AddRequestToTimeoutHandler checks the request and adds it to
* request_timeout_handler map for tracking
* @param request_to_hmi request to HMI
*/
void AddRequestToTimeoutHandler(
const smart_objects::SmartObject& request_to_hmi) const;
};

} // namespace commands
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
* POSSIBILITY OF SUCH DAMAGE.
*/

#ifndef SRC_COMPONENTS_APPLICATION_MANAGER_INCLUDE_APPLICATION_MANAGER_REQUEST_CONTROLLER_H_
#define SRC_COMPONENTS_APPLICATION_MANAGER_INCLUDE_APPLICATION_MANAGER_REQUEST_CONTROLLER_H_
#ifndef SRC_COMPONENTS_APPLICATION_MANAGER_INCLUDE_APPLICATION_MANAGER_REQUEST_CONTROLLER_IMPL_H_
#define SRC_COMPONENTS_APPLICATION_MANAGER_INCLUDE_APPLICATION_MANAGER_REQUEST_CONTROLLER_IMPL_H_

#include <climits>
#include <list>
Expand All @@ -47,6 +47,7 @@
#include "interfaces/HMI_API.h"
#include "interfaces/MOBILE_API.h"

#include "application_manager/request_controller.h"
#include "application_manager/request_controller_settings.h"
#include "application_manager/request_info.h"
#include "application_manager/request_tracker.h"
Expand All @@ -55,165 +56,60 @@ namespace application_manager {

namespace request_controller {

/**
* @brief RequestController class is used to control currently active mobile
* requests.
*/
class RequestController {
class RequestControllerImpl : public RequestController {

Choose a reason for hiding this comment

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

Please add all the deleted comments as applicable.

Copy link
Contributor

Choose a reason for hiding this comment

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

@atiwari9 please note that RequestControllerImpl is now just a class containing public functions overridden from the interface class. We don't need to duplicate the description of such functions in the impl classes according to our coding style:
https://google.github.io/styleguide/cppguide.html#Function_Comments

When documenting function overrides, focus on the specifics of the override itself, rather than repeating the comment from the overridden function. In many of these cases, the override needs no additional documentation and thus no comment is required.

Choose a reason for hiding this comment

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

@AKalinich-Luxoft , @ychernysheva - Please let me know when the PR is ready to be reviewed after above mentioned changes.

public:
/**
* @brief Result code for addRequest
*/
enum TResult {
SUCCESS = 0,
TOO_MANY_REQUESTS,
TOO_MANY_PENDING_REQUESTS,
NONE_HMI_LEVEL_MANY_REQUESTS,
INVALID_DATA
};

/**
* @brief Thread pool state
*/
enum TPoolState {
UNDEFINED = 0,
STARTED,
STOPPED,
};

// Methods

/**
* @brief Class constructor
*
*/
RequestController(const RequestControlerSettings& settings);
RequestControllerImpl(const RequestControlerSettings& settings,
RequestTimeoutHandler& request_timeout_handler);

/**
* @brief Class destructor
*
*/
virtual ~RequestController();
~RequestControllerImpl();

/**
* @brief Initialize thread pool
*
*/
void InitializeThreadpool();
void Stop() OVERRIDE;

/**
* @brief Destroy thread pool
*
*/
void DestroyThreadpool();
void InitializeThreadpool() OVERRIDE;

/**
* @brief Check if max request amount wasn't exceed and adds request to queue.
*
* @param request Active mobile request
* @param hmi_level Current application hmi_level
*
* @return Result code
*
*/
TResult addMobileRequest(const RequestPtr request,
const mobile_apis::HMILevel::eType& hmi_level);
void DestroyThreadpool() OVERRIDE;

/**
* @brief Store HMI request until response or timeout won't remove it
*
* @param request Active hmi request
* @return Result code
*
*/
TResult addHMIRequest(const RequestPtr request);
TResult AddMobileRequest(
const RequestPtr request,
const mobile_apis::HMILevel::eType& hmi_level) OVERRIDE;

/**
* @ Add notification to collection
*
* @param ptr Reference to shared pointer that point on hmi notification
*/
void addNotification(const RequestPtr ptr);
TResult AddHMIRequest(const RequestPtr request) OVERRIDE;

void AddNotification(const RequestPtr ptr) OVERRIDE;

/**
* @brief Removes request from queue
*
* @param correlation_id Active request correlation ID,
* @param connection_key Active request connection key (0 for HMI requersts)
* @param function_id Active request function id
* @param force_terminate if true, request controller will terminate
* even if not allowed by request
*/
void TerminateRequest(const uint32_t correlation_id,
const uint32_t connection_key,
const int32_t function_id,
bool force_terminate = false);
const bool force_terminate = false) OVERRIDE;

/**
* @brief Removes request from queue
*
* @param mobile_correlation_id Active mobile request correlation ID
*
*/
void OnMobileResponse(const uint32_t mobile_correlation_id,
const uint32_t connection_key,
const int32_t function_id);
const int32_t function_id) OVERRIDE;

/**
* @brief Removes request from queue
*
* @param mobile_correlation_id Active mobile request correlation ID
*
*/
void OnHMIResponse(const uint32_t correlation_id, const int32_t function_id);
void OnHMIResponse(const uint32_t correlation_id,
const int32_t function_id) OVERRIDE;

/**
* @ Add notification to collection
*
* @param ptr Reference to shared pointer that point on hmi notification
*/
void removeNotification(const commands::Command* notification);
void RemoveNotification(const commands::Command* notification) OVERRIDE;

/**
* @brief Removes all requests from queue for specified application
*
* @param app_id Mobile application ID (app_id)
*
*/
void terminateAppRequests(const uint32_t& app_id);
void TerminateAppRequests(const uint32_t app_id) OVERRIDE;

/**
* @brief Terminates all requests from HMI
*/
void terminateAllHMIRequests();
void TerminateAllHMIRequests() OVERRIDE;

/**
* @brief Terminates all requests from Mobile
*/
void terminateAllMobileRequests();
void TerminateAllMobileRequests() OVERRIDE;

/**
* @brief Updates request timeout
*
* @param app_id Connection key of application
* @param mobile_correlation_id Correlation ID of the mobile request
* @param new_timeout_value New timeout to be set in milliseconds
*/
void updateRequestTimeout(const uint32_t& app_id,
const uint32_t& mobile_correlation_id,
const uint32_t& new_timeout);
void UpdateRequestTimeout(const uint32_t app_id,
const uint32_t mobile_correlation_id,
const uint32_t new_timeout) OVERRIDE;

/*
* @brief Function Should be called when Low Voltage is occured
*/
void OnLowVoltage();
void OnLowVoltage() OVERRIDE;

/*
* @brief Function Should be called when Low Voltage is occured
*/
void OnWakeUp();
void OnWakeUp() OVERRIDE;

bool IsLowVoltage();
bool IsLowVoltage() OVERRIDE;

protected:
/**
Expand All @@ -226,8 +122,8 @@ class RequestController {
*/
void NotifyTimer();

void terminateWaitingForExecutionAppRequests(const uint32_t& app_id);
void terminateWaitingForResponseAppRequests(const uint32_t& app_id);
void TerminateWaitingForExecutionAppRequests(const uint32_t app_id);
void TerminateWaitingForResponseAppRequests(const uint32_t app_id);

/**
* @brief Checks whether all constraints are met before adding of request into
Expand All @@ -246,19 +142,19 @@ class RequestController {
* allowed for all applications
* @return True if new request could be added, false otherwise
*/
bool CheckPendingRequestsAmount(const uint32_t& pending_requests_amount);
bool CheckPendingRequestsAmount(const uint32_t pending_requests_amount);

private:
class Worker : public threads::ThreadDelegate {
public:
explicit Worker(RequestController* requestController);
explicit Worker(RequestControllerImpl* request_controller);
virtual ~Worker();
virtual void threadMain();
virtual void exitThreadMain();

protected:
private:
RequestController* request_controller_;
RequestControllerImpl* request_controller_;
sync_primitives::Lock thread_lock_;
volatile bool stop_flag_;
};
Expand Down Expand Up @@ -310,11 +206,12 @@ class RequestController {

bool is_low_voltage_;
const RequestControlerSettings& settings_;
DISALLOW_COPY_AND_ASSIGN(RequestController);
RequestTimeoutHandler& request_timeout_handler_;
DISALLOW_COPY_AND_ASSIGN(RequestControllerImpl);
};

} // namespace request_controller

} // namespace application_manager

#endif // SRC_COMPONENTS_APPLICATION_MANAGER_INCLUDE_APPLICATION_MANAGER_REQUEST_CONTROLLER_H_
#endif // SRC_COMPONENTS_APPLICATION_MANAGER_INCLUDE_APPLICATION_MANAGER_REQUEST_CONTROLLER_IMPL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ struct RequestInfo {
}
uint64_t hash();
static uint64_t GenerateHash(uint32_t var1, uint32_t var2);
static uint32_t HmiConnectionKey;
static constexpr uint32_t kHmiConnectionKey = 0;

protected:
RequestPtr request_;
Expand Down Expand Up @@ -199,7 +199,7 @@ class RequestInfoSet {
* @return founded request or shared_ptr with NULL
*/
RequestInfoPtr Find(const uint32_t connection_key,
const uint32_t correlation_id);
const uint32_t correlation_id) const;

/*
* @brief Get request with smalest end_time_
Expand Down Expand Up @@ -269,7 +269,7 @@ class RequestInfoSet {
TimeSortedRequestInfoSet time_sorted_pending_requests_;
HashSortedRequestInfoSet hash_sorted_pending_requests_;

sync_primitives::Lock pending_requests_lock_;
mutable sync_primitives::Lock pending_requests_lock_;
};

/**
Expand Down
Loading