-
Notifications
You must be signed in to change notification settings - Fork 243
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
Fix/incorrect setting service status #3106
base: develop
Are you sure you want to change the base?
Fix/incorrect setting service status #3106
Conversation
7acf7f4
to
ade53aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add documentation, commit, and PR description. Unable to get an idea of this changes.
@@ -828,6 +828,10 @@ class ApplicationManagerImpl | |||
bool result, | |||
std::vector<std::string>& rejected_params) OVERRIDE; | |||
|
|||
bool SetNaviServiceStatus(uint32_t connection_key, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation?
@@ -1866,17 +1849,11 @@ void ApplicationManagerImpl::OnStreamingConfigured( | |||
sync_primitives::AutoLock lock(navi_service_status_lock_); | |||
|
|||
NaviServiceStatusMap::iterator it = navi_service_status_.find(app_id); | |||
if (navi_service_status_.end() == it) { | |||
if (navi_service_status_.end() == it && !result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IGapchuk not obvious what is result variable about. Please rename variable. Give more narative name
bool ApplicationManagerImpl::SetNaviServiceStatus( | ||
uint32_t connection_key, | ||
protocol_handler::ServiceType service_type, | ||
bool is_started) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IGapchuk good practice to use enums for such values.
using namespace protocol_handler; | ||
LOG4CXX_AUTO_TRACE(logger_); | ||
|
||
auto app = application(connection_key); | ||
if (!app) { | ||
LOG4CXX_DEBUG( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IGapchuk I would say this is warning or error
if (navi_service_status_.end() == it) { | ||
LOG4CXX_WARN(logger_, | ||
"No Information about navi service " << service_type); | ||
NaviServiceStatusMap::iterator it = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto
NaviServiceStatusMap::iterator it = | ||
navi_service_status_.find(app->app_id()); | ||
if (navi_service_status_.end() == it && (is_started)) { | ||
std::pair<NaviServiceStatusMap::iterator, bool> res = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use autos
@@ -70,6 +70,10 @@ class ServiceStatusUpdateHandlerListener { | |||
hmi_apis::Common_ServiceEvent::eType service_event, | |||
utils::Optional<hmi_apis::Common_ServiceStatusUpdateReason::eType> | |||
service_update_reason) = 0; | |||
|
|||
virtual bool SetNaviServiceStatus(uint32_t connection_key, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation
ade53aa
to
40a8805
Compare
Fixes #[issue number]
This PR is not ready for review.
Summary
[Summary of PR changes]
CLA