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

Fix/incorrect setting service status #3106

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -828,6 +828,10 @@ class ApplicationManagerImpl
bool result,
std::vector<std::string>& rejected_params) OVERRIDE;

bool SetNaviServiceStatus(uint32_t connection_key,
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation?

protocol_handler::ServiceType service_type,
bool is_started) FINAL;

/**
* @brief Callback calls when application starts/stops data streaming
* @param app_id Streaming application id
Expand Down
88 changes: 52 additions & 36 deletions src/components/application_manager/src/application_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1790,23 +1790,6 @@ bool ApplicationManagerImpl::StartNaviService(
LOG4CXX_AUTO_TRACE(logger_);

if (HMILevelAllowsStreaming(app_id, service_type)) {
{
sync_primitives::AutoLock lock(navi_service_status_lock_);

NaviServiceStatusMap::iterator it = navi_service_status_.find(app_id);
if (navi_service_status_.end() == it) {
std::pair<NaviServiceStatusMap::iterator, bool> res =
navi_service_status_.insert(
std::pair<uint32_t, std::pair<bool, bool> >(
app_id, std::make_pair(false, false)));
if (!res.second) {
LOG4CXX_WARN(logger_, "Navi service refused");
return false;
}
it = res.first;
}
}

if (service_type == ServiceType::kMobileNav) {
smart_objects::SmartObject converted_params(smart_objects::SmartType_Map);
ConvertVideoParamsToSO(converted_params, params);
Expand Down Expand Up @@ -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) {
Copy link
Contributor

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

LOG4CXX_WARN(logger_, "Application not found in navi status map");
connection_handler().NotifyServiceStartedResult(app_id, false, empty);
return;
}

// Fill NaviServices map. Set true to first value of pair if
// we've started video service or to second value if we've
// started audio service
service_type == ServiceType::kMobileNav ? it->second.first = true
: it->second.second = true;
}

application(app_id)->StartStreaming(service_type);
Expand All @@ -1889,33 +1866,70 @@ void ApplicationManagerImpl::OnStreamingConfigured(
}
}

void ApplicationManagerImpl::StopNaviService(
uint32_t app_id, protocol_handler::ServiceType service_type) {
bool ApplicationManagerImpl::SetNaviServiceStatus(
uint32_t connection_key,
protocol_handler::ServiceType service_type,
bool is_started) {
Copy link
Contributor

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(
Copy link
Contributor

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

logger_,
"Application with app_id: " << connection_key << " is not found");
return false;
}

{
sync_primitives::AutoLock lock(navi_service_status_lock_);

NaviServiceStatusMap::iterator it = navi_service_status_.find(app_id);
if (navi_service_status_.end() == it) {
LOG4CXX_WARN(logger_,
"No Information about navi service " << service_type);
NaviServiceStatusMap::iterator it =
Copy link
Contributor

Choose a reason for hiding this comment

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

auto

navi_service_status_.find(app->app_id());
if (navi_service_status_.end() == it && (is_started)) {
std::pair<NaviServiceStatusMap::iterator, bool> res =
Copy link
Contributor

Choose a reason for hiding this comment

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

Use autos

navi_service_status_.insert(
std::pair<uint32_t, std::pair<bool, bool> >(
app->app_id(), std::make_pair(false, false)));
if (!res.second) {
LOG4CXX_WARN(logger_, "Navi service refused");
return false;
}
it = res.first;
} else {
// Fill NaviServices map. Set false to first value of pair if
// we've stopped video service or to second value if we've
// stopped audio service
service_type == ServiceType::kMobileNav ? it->second.first = false
: it->second.second = false;
LOG4CXX_DEBUG(logger_,
"Application with app_id: "
<< app->app_id() << " is not found in navi status map");
return false;
}

service_type == ServiceType::kMobileNav ? it->second.first = is_started
: it->second.second = is_started;
LOG4CXX_DEBUG(logger_,
"ServiceType: " << service_type << " is started: "
<< std::boolalpha << is_started);
return true;
}
}

void ApplicationManagerImpl::StopNaviService(
uint32_t app_id, protocol_handler::ServiceType service_type) {
using namespace protocol_handler;
LOG4CXX_AUTO_TRACE(logger_);

ApplicationSharedPtr app = application(app_id);
if (!app) {
LOG4CXX_WARN(logger_, "An application is not registered.");
return;
}

if (!SetNaviServiceStatus(app->app_id(), service_type, false)) {
LOG4CXX_DEBUG(logger_,
"Application with app_id: "
<< app->app_id() << " not found in navi status map");
return;
}

app->StopStreaming(service_type);
}

Expand Down Expand Up @@ -2038,7 +2052,7 @@ void ApplicationManagerImpl::OnServiceEndedCallback(

if (Compare<ServiceType, EQ, ONE>(
type, ServiceType::kMobileNav, ServiceType::kAudio)) {
StopNaviService(session_key, type);
StopNaviService(static_cast<uint32_t>(session_key), type);
}
}

Expand Down Expand Up @@ -3395,6 +3409,8 @@ void ApplicationManagerImpl::EndNaviServices(uint32_t app_id) {
LOG4CXX_ERROR(logger_, "No info about navi servicies for app");
return;
}
LOG4CXX_DEBUG(logger_, "VEDIO_KEK: " << it->second.first);
LOG4CXX_DEBUG(logger_, "AUDIO_KEK: " << it->second.second);
end_video = it->second.first;
end_audio = it->second.second;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ class ServiceStatusUpdateHandler {
const protocol_handler::ServiceType service_type,
const ServiceStatus service_status);

void SetNaviServiceStatus(uint32_t app_id,
protocol_handler::ServiceType service_type,
bool is_opened);

private:
ServiceStatusUpdateHandlerListener* listener_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation

protocol_handler::ServiceType service_type,
bool is_started) = 0;
};

} // namespace protocol_handler
Expand Down
25 changes: 25 additions & 0 deletions src/components/protocol_handler/src/protocol_handler_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,16 @@ void ProtocolHandlerImpl::SendStartSessionAck(
raw_ford_messages_to_mobile_.PostMessage(
impl::RawFordMessageToMobile(ptr, false));

if ((service_type == ServiceType::kAudio) ||
(service_type == ServiceType::kMobileNav)) {
const uint32_t connection_key =
session_observer_.KeyFromPair(connection_id, session_id);
service_status_update_handler_->SetNaviServiceStatus(
connection_key,
static_cast<protocol_handler::ServiceType>(service_type),
true);
}

LOG4CXX_DEBUG(logger_,
"SendStartSessionAck() for connection "
<< connection_id << " for service_type "
Expand Down Expand Up @@ -509,6 +519,13 @@ void ProtocolHandlerImpl::SendStartSessionNAck(
raw_ford_messages_to_mobile_.PostMessage(
impl::RawFordMessageToMobile(ptr, false));

const uint32_t connection_key =
session_observer_.KeyFromPair(connection_id, session_id);
service_status_update_handler_->SetNaviServiceStatus(
connection_key,
static_cast<protocol_handler::ServiceType>(service_type),
false);

LOG4CXX_DEBUG(logger_,
"SendStartSessionNAck() for connection "
<< connection_id << " for service_type "
Expand Down Expand Up @@ -572,6 +589,14 @@ void ProtocolHandlerImpl::SendEndSessionNAck(
raw_ford_messages_to_mobile_.PostMessage(
impl::RawFordMessageToMobile(ptr, false));

const uint32_t connection_key = session_observer_.KeyFromPair(
connection_id, static_cast<uint8_t>(session_id));

service_status_update_handler_->SetNaviServiceStatus(
connection_key,
static_cast<protocol_handler::ServiceType>(service_type),
false);

LOG4CXX_DEBUG(logger_,
"SendEndSessionNAck() for connection "
<< connection_id << " for service_type "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,10 @@ void ServiceStatusUpdateHandler::OnServiceUpdate(
}
}
}

void ServiceStatusUpdateHandler::SetNaviServiceStatus(uint32_t app_id,
ServiceType service_type,
bool is_opened) {
listener_->SetNaviServiceStatus(app_id, service_type, is_opened);
}
} // namespace protocol_handler
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ class MockServiceStatusUpdateHandlerListener
hmi_apis::Common_ServiceType::eType,
hmi_apis::Common_ServiceEvent::eType,
utils::Optional<hmi_apis::Common_ServiceStatusUpdateReason::eType>));

MOCK_METHOD3(SetNaviServiceStatus,
bool(uint32_t connection_key,
protocol_handler::ServiceType service_type,
bool is_started));
};
} // namespace protocol_handler_test
} // namespace components
Expand Down