From 85db232d366504c0577c8b3063d5b201d0a439bc Mon Sep 17 00:00:00 2001 From: Sho Amano Date: Fri, 16 Feb 2018 14:22:25 +0900 Subject: fix: remove RIFF header from audio stream for AudioPassThru Please refer to the proposal on https://github.com/smartdevicelink/sdl_evolution/issues/394. The correct format is NOT to include any header. --- .../media_manager/src/audio/audio_stream_sender_thread.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/components/media_manager/src/audio/audio_stream_sender_thread.cc b/src/components/media_manager/src/audio/audio_stream_sender_thread.cc index 9b0057dfe8..e281c760ac 100644 --- a/src/components/media_manager/src/audio/audio_stream_sender_thread.cc +++ b/src/components/media_manager/src/audio/audio_stream_sender_thread.cc @@ -59,6 +59,15 @@ const int32_t AudioStreamSenderThread::kAudioPassThruTimeout = 1000; #endif const uint32_t kMqueueMessageSize = 4095; +// Size of RIFF header contained in a .wav file: 12 bytes for 'RIFF' chunk +// descriptor, 24 bytes for 'fmt ' sub-chunk and 8 bytes for 'data' sub-chunk +// header. +// The correct format of audio stream for AudioPassThru feature is to include +// only audio sample data. Since both pre-recorded file (audio.8bit.wav) and +// GStreamer-generated file contain RIFF header, we will skip it when reading +// the files. +static const uint32_t kRIFFHeaderSize = 44; + CREATE_LOGGERPTR_GLOBAL(logger_, "MediaManager") AudioStreamSenderThread::AudioStreamSenderThread( @@ -67,7 +76,7 @@ AudioStreamSenderThread::AudioStreamSenderThread( application_manager::ApplicationManager& app_mngr) : session_key_(session_key) , fileName_(fileName) - , offset_(0) + , offset_(kRIFFHeaderSize) , shouldBeStoped_(false) , shouldBeStoped_lock_() , shouldBeStoped_cv_() @@ -80,7 +89,7 @@ AudioStreamSenderThread::~AudioStreamSenderThread() {} void AudioStreamSenderThread::threadMain() { LOG4CXX_AUTO_TRACE(logger_); - offset_ = 0; + offset_ = kRIFFHeaderSize; while (false == getShouldBeStopped()) { AutoLock auto_lock(shouldBeStoped_lock_); @@ -123,7 +132,7 @@ void AudioStreamSenderThread::sendAudioChunkToMobile() { } #if !defined(EXTENDED_MEDIA_MODE) // without recording stream restart reading 1-sec file - offset_ = 0; + offset_ = kRIFFHeaderSize; #endif } -- cgit v1.2.1 From 3546d72f9918251ea35f27ecdbf9330ec363a20b Mon Sep 17 00:00:00 2001 From: Sho Amano Date: Fri, 16 Feb 2018 14:59:01 +0900 Subject: fix: make sure to run AudioPassThru in monaural --- .../src/audio/from_mic_to_file_recorder_thread.cc | 30 +++++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/components/media_manager/src/audio/from_mic_to_file_recorder_thread.cc b/src/components/media_manager/src/audio/from_mic_to_file_recorder_thread.cc index 99548e71bd..5c12614662 100644 --- a/src/components/media_manager/src/audio/from_mic_to_file_recorder_thread.cc +++ b/src/components/media_manager/src/audio/from_mic_to_file_recorder_thread.cc @@ -42,6 +42,9 @@ CREATE_LOGGERPTR_GLOBAL(logger_, "MediaManager") GMainLoop* FromMicToFileRecorderThread::loop = NULL; +// As per spec, AudioPassThru recording is in monaural +static const int kNumAudioChannels = 1; + FromMicToFileRecorderThread::FromMicToFileRecorderThread( const std::string& output_file, int32_t duration) : threads::ThreadDelegate() @@ -119,7 +122,8 @@ void FromMicToFileRecorderThread::threadMain() { initArgs(); GstElement* pipeline; - GstElement* alsasrc, *wavenc, *filesink; + GstElement* alsasrc, *audioconvert, *capsfilter, *wavenc, *filesink; + GstCaps* audiocaps; GstBus* bus; const gchar* device = "hw:0,0"; @@ -196,11 +200,18 @@ void FromMicToFileRecorderThread::threadMain() { // Create all of the elements to be added to the pipeline alsasrc = gst_element_factory_make("alsasrc", "alsasrc0"); + audioconvert = gst_element_factory_make("audioconvert", "audioconvert0"); + capsfilter = gst_element_factory_make("capsfilter", "filter0"); wavenc = gst_element_factory_make("wavenc", "wavenc0"); filesink = gst_element_factory_make("filesink", "filesink0"); + // create a capability to downmix the recorded audio to monaural + audiocaps = gst_caps_new_simple( + "audio/x-raw", "channels", G_TYPE_INT, kNumAudioChannels, NULL); + // Assert that all the elements were created - if (!alsasrc || !wavenc || !filesink) { + if (!alsasrc || !audioconvert || !capsfilter || !wavenc || !filesink || + !audiocaps) { g_error("Failed creating one or more of the pipeline elements.\n"); } @@ -209,10 +220,21 @@ void FromMicToFileRecorderThread::threadMain() { g_object_set(G_OBJECT(filesink), "location", outfile, NULL); // Add the elements to the pipeline - gst_bin_add_many(GST_BIN(pipeline), alsasrc, wavenc, filesink, NULL); + gst_bin_add_many(GST_BIN(pipeline), + alsasrc, + audioconvert, + capsfilter, + wavenc, + filesink, + NULL); // Link the elements - gst_element_link_many(alsasrc, wavenc, filesink, NULL); + gst_element_link_many( + alsasrc, audioconvert, capsfilter, wavenc, filesink, NULL); + + // set the capability + g_object_set(G_OBJECT(capsfilter), "caps", audiocaps, NULL); + gst_caps_unref(audiocaps); gst_element_set_state(pipeline, GST_STATE_PLAYING); -- cgit v1.2.1 From 86e9f9d546822d310e7876f451fd9e6b0446c12c Mon Sep 17 00:00:00 2001 From: Sho Amano Date: Fri, 16 Feb 2018 19:24:28 +0900 Subject: Update sample audio file to be monaural ... since the audio stream of AudioPassThru has to be monaural. This file is created by mixing down the original file. --- src/appMain/audio.8bit.wav | Bin 88244 -> 44144 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/src/appMain/audio.8bit.wav b/src/appMain/audio.8bit.wav index cebf6cc0e2..7a3f970a21 100644 Binary files a/src/appMain/audio.8bit.wav and b/src/appMain/audio.8bit.wav differ -- cgit v1.2.1 From d051f63464aa2263993e4459c642e5c02c7f95b4 Mon Sep 17 00:00:00 2001 From: Sho Amano Date: Fri, 16 Feb 2018 15:07:41 +0900 Subject: Update comment fields of AudioPassThru capabilities in MOBILE_API.xml --- src/components/interfaces/MOBILE_API.xml | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/components/interfaces/MOBILE_API.xml b/src/components/interfaces/MOBILE_API.xml index a1c64aecda..3094eb16ba 100644 --- a/src/components/interfaces/MOBILE_API.xml +++ b/src/components/interfaces/MOBILE_API.xml @@ -456,27 +456,42 @@ Describes different sampling options for PerformAudioPassThru. - - - - + + Sampling rate of 8000 Hz. + + + Sampling rate of 16000 Hz. + + + Sampling rate of 22050 Hz. + + + Sampling rate of 44100 Hz. + Describes different quality options for PerformAudioPassThru. - - + + Audio sample is 8 bits wide, unsigned. + + + Audio sample is 16 bits wide, signed, and in little endian. + Describes different audio type options for PerformAudioPassThru. - + + Linear PCM. + Describes different audio type configurations for PerformAudioPassThru. e.g. {8kHz,8-bit,PCM} + The audio is recorded in monaural. -- cgit v1.2.1 From 1f4f032e7a70707e4ce613d5fa42d70a22b20633 Mon Sep 17 00:00:00 2001 From: Sho Amano Date: Fri, 16 Feb 2018 15:23:05 +0900 Subject: Update comment fields of AudioPassThru capabilities in HMI_API.xml --- src/components/interfaces/HMI_API.xml | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/components/interfaces/HMI_API.xml b/src/components/interfaces/HMI_API.xml index ab3933fc0f..790c9d4c06 100644 --- a/src/components/interfaces/HMI_API.xml +++ b/src/components/interfaces/HMI_API.xml @@ -1031,21 +1031,35 @@ Describes different sampling options for PerformAudioPassThru. - - - - + + Sampling rate of 8000 Hz. + + + Sampling rate of 16000 Hz. + + + Sampling rate of 22050 Hz. + + + Sampling rate of 44100 Hz. + Describes different quality options for PerformAudioPassThru. - - + + Audio sample is 8 bits wide, unsigned. + + + Audio sample is 16 bits wide, signed, and in little endian. + Describes different audio type options for PerformAudioPassThru. - + + Linear PCM. + @@ -2121,6 +2135,7 @@ Describes different audio type configurations for PerformAudioPassThru. e.g. 8kHz,8-bit,PCM + The audio is recorded in monaural. -- cgit v1.2.1 From 44007f451d947d97870624f4990c1033d3808276 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Thu, 31 May 2018 14:54:52 -0400 Subject: Begin changing locks over, very segfault-y right now This required modification of the condition variable class as well as every instance of a recursive lock --- .../app_launch/app_launch_data_json.h | 2 +- .../application_manager/application_data_impl.h | 4 +- .../application_manager/application_manager_impl.h | 4 +- .../mobile/create_interaction_choice_set_request.h | 2 +- .../event_engine/event_dispatcher_impl.h | 2 +- .../application_manager/policies/policy_handler.h | 2 +- .../resumption/resumption_data.h | 2 +- .../src/app_launch/app_launch_data_json.cc | 1 - .../src/application_data_impl.cc | 2 - .../src/application_manager_impl.cc | 2 - .../create_interaction_choice_set_request.cc | 3 +- .../src/event_engine/event_dispatcher_impl.cc | 2 +- .../src/policies/policy_handler.cc | 1 - .../src/resumption/resume_ctrl_impl.cc | 1 - .../src/resumption/resumption_data.cc | 2 +- .../test/message_helper/message_helper_test.cc | 14 +- .../include/connection_handler/connection.h | 2 +- .../include/connection_handler/heartbeat_monitor.h | 2 +- .../connection_handler/src/connection.cc | 2 +- .../connection_handler/src/heartbeat_monitor.cc | 1 - .../include/utils/conditional_variable.h | 19 +-- src/components/include/utils/data_accessor.h | 6 +- src/components/include/utils/lock.h | 142 +++++++++------- src/components/include/utils/lock_posix.h | 184 +++++++++++++++++++++ .../policy_regular/include/policy/cache_manager.h | 2 +- .../policy/policy_regular/src/cache_manager.cc | 18 +- src/components/utils/CMakeLists.txt | 5 +- .../utils/src/conditional_variable_posix.cc | 138 +++++++--------- src/components/utils/src/lock_boost.cc | 143 ++++++++++++++++ src/components/utils/src/lock_posix.cc | 4 +- src/components/utils/test/lock_boost_test.cc | 103 ++++++++++++ src/components/utils/test/lock_posix_test.cc | 4 +- 32 files changed, 622 insertions(+), 199 deletions(-) create mode 100644 src/components/include/utils/lock_posix.h create mode 100644 src/components/utils/src/lock_boost.cc create mode 100644 src/components/utils/test/lock_boost_test.cc diff --git a/src/components/application_manager/include/application_manager/app_launch/app_launch_data_json.h b/src/components/application_manager/include/application_manager/app_launch/app_launch_data_json.h index 61117e552b..44b70ee305 100644 --- a/src/components/application_manager/include/application_manager/app_launch/app_launch_data_json.h +++ b/src/components/application_manager/include/application_manager/app_launch/app_launch_data_json.h @@ -141,7 +141,7 @@ class AppLaunchDataJson : public AppLaunchDataImpl { /** * @brief lock to protected common data */ - mutable sync_primitives::Lock app_launch_json_lock_; + mutable sync_primitives::RecursiveLock app_launch_json_lock_; /** * @brief ponter to Last State object diff --git a/src/components/application_manager/include/application_manager/application_data_impl.h b/src/components/application_manager/include/application_manager/application_data_impl.h index dc9be4e1d8..3690508f74 100644 --- a/src/components/application_manager/include/application_manager/application_data_impl.h +++ b/src/components/application_manager/include/application_manager/application_data_impl.h @@ -275,13 +275,13 @@ class DynamicApplicationDataImpl : public virtual Application { std::string display_layout_; CommandsMap commands_; - mutable sync_primitives::Lock commands_lock_; + mutable sync_primitives::RecursiveLock commands_lock_; SubMenuMap sub_menu_; mutable sync_primitives::Lock sub_menu_lock_; ChoiceSetMap choice_set_map_; mutable sync_primitives::Lock choice_set_map_lock_; PerformChoiceSetMap performinteraction_choice_set_map_; - mutable sync_primitives::Lock performinteraction_choice_set_lock_; + mutable sync_primitives::RecursiveLock performinteraction_choice_set_lock_; uint32_t is_perform_interaction_active_; bool is_reset_global_properties_active_; int32_t perform_interaction_mode_; diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h index 286ad87018..985932a172 100644 --- a/src/components/application_manager/include/application_manager/application_manager_impl.h +++ b/src/components/application_manager/include/application_manager/application_manager_impl.h @@ -1698,7 +1698,7 @@ class ApplicationManagerImpl ForbiddenApps forbidden_applications; // Lock for applications list - mutable sync_primitives::Lock applications_list_lock_; + mutable sync_primitives::RecursiveLock applications_list_lock_; mutable sync_primitives::Lock apps_to_register_list_lock_; mutable sync_primitives::Lock subscribed_way_points_apps_lock_; @@ -1799,7 +1799,7 @@ class ApplicationManagerImpl std::vector timer_pool_; sync_primitives::Lock timer_pool_lock_; - sync_primitives::Lock stopping_application_mng_lock_; + sync_primitives::RecursiveLock stopping_application_mng_lock_; StateControllerImpl state_ctrl_; std::auto_ptr app_launch_dto_; std::auto_ptr app_launch_ctrl_; diff --git a/src/components/application_manager/include/application_manager/commands/mobile/create_interaction_choice_set_request.h b/src/components/application_manager/include/application_manager/commands/mobile/create_interaction_choice_set_request.h index c00d310e7d..f79a2e7225 100644 --- a/src/components/application_manager/include/application_manager/commands/mobile/create_interaction_choice_set_request.h +++ b/src/components/application_manager/include/application_manager/commands/mobile/create_interaction_choice_set_request.h @@ -136,7 +136,7 @@ class CreateInteractionChoiceSetRequest : public CommandRequestImpl { volatile bool is_timed_out_; sync_primitives::Lock is_timed_out_lock_; - sync_primitives::Lock vr_commands_lock_; + sync_primitives::RecursiveLock vr_commands_lock_; /* * @brief Sends VR AddCommand request to HMI * diff --git a/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h b/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h index 4cdbb902de..ed923369ac 100644 --- a/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h +++ b/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h @@ -118,7 +118,7 @@ class EventDispatcherImpl : public EventDispatcher { private: // Members section sync_primitives::Lock state_lock_; - sync_primitives::Lock observer_lock_; + sync_primitives::RecursiveLock observer_lock_; EventObserverMap observers_event_; ObserverVector observers_; }; diff --git a/src/components/application_manager/include/application_manager/policies/policy_handler.h b/src/components/application_manager/include/application_manager/policies/policy_handler.h index b4653c6cb2..4be01214ab 100644 --- a/src/components/application_manager/include/application_manager/policies/policy_handler.h +++ b/src/components/application_manager/include/application_manager/policies/policy_handler.h @@ -726,7 +726,7 @@ class PolicyHandler : public PolicyHandlerInterface, std::map app_to_device_link_; // Lock for app to device list - sync_primitives::Lock app_to_device_link_lock_; + sync_primitives::RecursiveLock app_to_device_link_lock_; utils::SharedPtr statistic_manager_impl_; const PolicySettings& settings_; diff --git a/src/components/application_manager/include/application_manager/resumption/resumption_data.h b/src/components/application_manager/include/application_manager/resumption/resumption_data.h index bee2bce570..5ecfaf3c11 100644 --- a/src/components/application_manager/include/application_manager/resumption/resumption_data.h +++ b/src/components/application_manager/include/application_manager/resumption/resumption_data.h @@ -276,7 +276,7 @@ class ResumptionData { ++first; } } - mutable sync_primitives::Lock resumption_lock_; + mutable sync_primitives::RecursiveLock resumption_lock_; const application_manager::ApplicationManager& application_manager_; }; } // namespace resumption diff --git a/src/components/application_manager/src/app_launch/app_launch_data_json.cc b/src/components/application_manager/src/app_launch/app_launch_data_json.cc index 7599dcccb3..99a6416a69 100644 --- a/src/components/application_manager/src/app_launch/app_launch_data_json.cc +++ b/src/components/application_manager/src/app_launch/app_launch_data_json.cc @@ -44,7 +44,6 @@ CREATE_LOGGERPTR_GLOBAL(logger_, "AppLaunch") AppLaunchDataJson::AppLaunchDataJson(const AppLaunchSettings& settings, resumption::LastState& last_state) : AppLaunchDataImpl(settings) - , app_launch_json_lock_(true) , last_state_(last_state) {} AppLaunchDataJson::~AppLaunchDataJson() {} diff --git a/src/components/application_manager/src/application_data_impl.cc b/src/components/application_manager/src/application_data_impl.cc index 226c83dbf6..931b3571fb 100644 --- a/src/components/application_manager/src/application_data_impl.cc +++ b/src/components/application_manager/src/application_data_impl.cc @@ -177,11 +177,9 @@ DynamicApplicationDataImpl::DynamicApplicationDataImpl() , night_color_scheme_(NULL) , display_layout_("") , commands_() - , commands_lock_(true) , sub_menu_() , choice_set_map_() , performinteraction_choice_set_map_() - , performinteraction_choice_set_lock_(true) , is_perform_interaction_active_(false) , is_reset_global_properties_active_(false) , perform_interaction_mode_(-1) {} diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index 3ead8fe65e..7d1b1aaeda 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -141,7 +141,6 @@ ApplicationManagerImpl::ApplicationManagerImpl( const ApplicationManagerSettings& am_settings, const policy::PolicySettings& policy_settings) : settings_(am_settings) - , applications_list_lock_(true) , audio_pass_thru_active_(false) , audio_pass_thru_app_id_(0) , driver_distraction_state_( @@ -168,7 +167,6 @@ ApplicationManagerImpl::ApplicationManagerImpl( , resume_ctrl_(new resumption::ResumeCtrlImpl(*this)) , navi_close_app_timeout_(am_settings.stop_streaming_timeout()) , navi_end_stream_timeout_(am_settings.stop_streaming_timeout()) - , stopping_application_mng_lock_(true) , state_ctrl_(*this) #ifdef TELEMETRY_MONITOR , metric_observer_(NULL) diff --git a/src/components/application_manager/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/src/commands/mobile/create_interaction_choice_set_request.cc index 20387ef230..2ee3fd8bb8 100644 --- a/src/components/application_manager/src/commands/mobile/create_interaction_choice_set_request.cc +++ b/src/components/application_manager/src/commands/mobile/create_interaction_choice_set_request.cc @@ -53,8 +53,7 @@ CreateInteractionChoiceSetRequest::CreateInteractionChoiceSetRequest( , expected_chs_count_(0) , received_chs_count_(0) , error_from_hmi_(false) - , is_timed_out_(false) - , vr_commands_lock_(true) {} + , is_timed_out_(false) {} CreateInteractionChoiceSetRequest::~CreateInteractionChoiceSetRequest() { LOG4CXX_AUTO_TRACE(logger_); diff --git a/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc b/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc index b19a6f9194..505465d001 100644 --- a/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc +++ b/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc @@ -40,7 +40,7 @@ namespace event_engine { using namespace sync_primitives; EventDispatcherImpl::EventDispatcherImpl() - : state_lock_(false), observer_lock_(true), observers_event_() {} + : observers_event_() {} EventDispatcherImpl::~EventDispatcherImpl() {} diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 559b9c0035..4c4b5d47da 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -325,7 +325,6 @@ PolicyHandler::PolicyHandler(const PolicySettings& settings, : AsyncRunner("PolicyHandler async runner thread") , dl_handle_(0) , last_activated_app_id_(0) - , app_to_device_link_lock_(true) , statistic_manager_impl_(utils::MakeShared(this)) , settings_(settings) , application_manager_(application_manager) {} diff --git a/src/components/application_manager/src/resumption/resume_ctrl_impl.cc b/src/components/application_manager/src/resumption/resume_ctrl_impl.cc index e3fd423970..e370d345b8 100644 --- a/src/components/application_manager/src/resumption/resume_ctrl_impl.cc +++ b/src/components/application_manager/src/resumption/resume_ctrl_impl.cc @@ -57,7 +57,6 @@ CREATE_LOGGERPTR_GLOBAL(logger_, "Resumption") ResumeCtrlImpl::ResumeCtrlImpl(ApplicationManager& application_manager) : event_engine::EventObserver(application_manager.event_dispatcher()) - , queue_lock_(false) , restore_hmi_level_timer_( "RsmCtrlRstore", new timer::TimerTaskImpl( diff --git a/src/components/application_manager/src/resumption/resumption_data.cc b/src/components/application_manager/src/resumption/resumption_data.cc index bd5bdbddab..ca049917c4 100644 --- a/src/components/application_manager/src/resumption/resumption_data.cc +++ b/src/components/application_manager/src/resumption/resumption_data.cc @@ -41,7 +41,7 @@ CREATE_LOGGERPTR_GLOBAL(logger_, "Resumption") ResumptionData::ResumptionData( const application_manager::ApplicationManager& application_manager) - : resumption_lock_(true), application_manager_(application_manager) {} + : application_manager_(application_manager) {} smart_objects::SmartObject ResumptionData::GetApplicationCommands( app_mngr::ApplicationConstSharedPtr application) const { diff --git a/src/components/application_manager/test/message_helper/message_helper_test.cc b/src/components/application_manager/test/message_helper/message_helper_test.cc index 289e4f5d05..dc4b3f48bf 100644 --- a/src/components/application_manager/test/message_helper/message_helper_test.cc +++ b/src/components/application_manager/test/message_helper/message_helper_test.cc @@ -236,7 +236,7 @@ TEST(MessageHelperTestCreate, CreateAddCommandRequestToHMI_SendSmartObject_Empty) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); ::application_manager::CommandsMap vis; - DataAccessor data_accessor(vis, true); + DataAccessor data_accessor(vis, sync_primitives::RecursiveLock()); EXPECT_CALL(*appSharedMock, commands_map()).WillOnce(Return(data_accessor)); application_manager_test::MockApplicationManager mock_application_manager; @@ -251,7 +251,7 @@ TEST(MessageHelperTestCreate, CreateAddCommandRequestToHMI_SendSmartObject_Equal) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); CommandsMap vis; - DataAccessor data_accessor(vis, true); + DataAccessor data_accessor(vis, sync_primitives::RecursiveLock()); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -292,7 +292,7 @@ TEST(MessageHelperTestCreate, CreateAddVRCommandRequestFromChoiceToHMI_SendEmptyData_EmptyList) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::ChoiceSetMap vis; - DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, true); + DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, sync_primitives::RecursiveLock()); EXPECT_CALL(*appSharedMock, choice_set_map()).WillOnce(Return(data_accessor)); application_manager_test::MockApplicationManager mock_application_manager; @@ -307,7 +307,7 @@ TEST(MessageHelperTestCreate, CreateAddVRCommandRequestFromChoiceToHMI_SendObject_EqualList) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::ChoiceSetMap vis; - DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, true); + DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, sync_primitives::RecursiveLock()); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -353,7 +353,7 @@ TEST(MessageHelperTestCreate, TEST(MessageHelperTestCreate, CreateAddSubMenuRequestToHMI_SendObject_Equal) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::SubMenuMap vis; - DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, true); + DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, sync_primitives::RecursiveLock()); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -392,7 +392,7 @@ TEST(MessageHelperTestCreate, CreateAddSubMenuRequestToHMI_SendEmptyMap_EmptySmartObjectList) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::SubMenuMap vis; - DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, true); + DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, sync_primitives::RecursiveLock()); EXPECT_CALL(*appSharedMock, sub_menu_map()).WillOnce(Return(data_accessor)); @@ -713,7 +713,7 @@ TEST_F(MessageHelperTest, // Creating data acessor application_manager::VehicleInfoSubscriptions vis; DataAccessor data_accessor( - vis, true); + vis, sync_primitives::RecursiveLock()); // Calls for ApplicationManager EXPECT_CALL(*appSharedMock, app_id()).WillOnce(Return(1u)); EXPECT_CALL(*appSharedMock, SubscribedIVI()).WillOnce(Return(data_accessor)); diff --git a/src/components/connection_handler/include/connection_handler/connection.h b/src/components/connection_handler/include/connection_handler/connection.h index 9b72d60776..d46f574707 100644 --- a/src/components/connection_handler/include/connection_handler/connection.h +++ b/src/components/connection_handler/include/connection_handler/connection.h @@ -304,7 +304,7 @@ class Connection { */ SessionMap session_map_; - mutable sync_primitives::Lock session_map_lock_; + mutable sync_primitives::RecursiveLock session_map_lock_; /** * @brief monitor that closes connection if there is no traffic over it diff --git a/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h b/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h index 4d1d07112c..1a0bfce264 100644 --- a/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h +++ b/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h @@ -111,7 +111,7 @@ class HeartBeatMonitor : public threads::ThreadDelegate { typedef std::map SessionMap; SessionMap sessions_; - sync_primitives::Lock sessions_list_lock_; // recurcive + sync_primitives::RecursiveLock sessions_list_lock_; // recurcive sync_primitives::Lock main_thread_lock_; mutable sync_primitives::Lock heartbeat_timeout_seconds_lock_; sync_primitives::ConditionalVariable heartbeat_monitor_; diff --git a/src/components/connection_handler/src/connection.cc b/src/components/connection_handler/src/connection.cc index 16b88c4164..ae7eb1be18 100644 --- a/src/components/connection_handler/src/connection.cc +++ b/src/components/connection_handler/src/connection.cc @@ -81,7 +81,7 @@ Connection::Connection(ConnectionHandle connection_handle, : connection_handler_(connection_handler) , connection_handle_(connection_handle) , connection_device_handle_(connection_device_handle) - , session_map_lock_(true) + , session_map_lock_() , heartbeat_timeout_(heartbeat_timeout) { LOG4CXX_AUTO_TRACE(logger_); DCHECK(connection_handler_); diff --git a/src/components/connection_handler/src/heartbeat_monitor.cc b/src/components/connection_handler/src/heartbeat_monitor.cc index f3a2322810..50af5a042a 100644 --- a/src/components/connection_handler/src/heartbeat_monitor.cc +++ b/src/components/connection_handler/src/heartbeat_monitor.cc @@ -47,7 +47,6 @@ HeartBeatMonitor::HeartBeatMonitor(uint32_t heartbeat_timeout_mseconds, Connection* connection) : default_heartbeat_timeout_(heartbeat_timeout_mseconds) , connection_(connection) - , sessions_list_lock_(true) , run_(true) {} void HeartBeatMonitor::Process() { diff --git a/src/components/include/utils/conditional_variable.h b/src/components/include/utils/conditional_variable.h index f54a22e993..a29f255dbf 100644 --- a/src/components/include/utils/conditional_variable.h +++ b/src/components/include/utils/conditional_variable.h @@ -32,24 +32,13 @@ #ifndef SRC_COMPONENTS_INCLUDE_UTILS_CONDITIONAL_VARIABLE_H_ #define SRC_COMPONENTS_INCLUDE_UTILS_CONDITIONAL_VARIABLE_H_ -#if defined(OS_POSIX) -#include -#else -#error Please implement conditional variable for your OS -#endif #include +#include +#include "utils/lock.h" #include "utils/macro.h" namespace sync_primitives { -class AutoLock; -class Lock; - -namespace impl { -#if defined(OS_POSIX) -typedef pthread_cond_t PlatformConditionalVariable; -#endif -} // namespace impl /* * Conditional variable wrapper @@ -82,11 +71,11 @@ class ConditionalVariable { // Wait forever or up to milliseconds time limit bool Wait(AutoLock& auto_lock); - bool Wait(Lock& lock); + bool Wait(BaseLock& lock); WaitStatus WaitFor(AutoLock& auto_lock, uint32_t milliseconds); private: - impl::PlatformConditionalVariable cond_var_; + boost::condition_variable_any cond_var_; private: DISALLOW_COPY_AND_ASSIGN(ConditionalVariable); diff --git a/src/components/include/utils/data_accessor.h b/src/components/include/utils/data_accessor.h index 9be28a638b..6239812678 100644 --- a/src/components/include/utils/data_accessor.h +++ b/src/components/include/utils/data_accessor.h @@ -39,9 +39,9 @@ template class DataAccessor { public: - DataAccessor(const T& data, const sync_primitives::Lock& lock) + DataAccessor(const T& data, const sync_primitives::BaseLock& lock) : data_(data) - , lock_(const_cast(lock)) + , lock_(const_cast(lock)) , counter_(new uint32_t(0)) { lock_.Acquire(); } @@ -65,7 +65,7 @@ class DataAccessor { private: void* operator new(size_t size); const T& data_; - sync_primitives::Lock& lock_; + sync_primitives::BaseLock& lock_; utils::SharedPtr counter_; }; diff --git a/src/components/include/utils/lock.h b/src/components/include/utils/lock.h index e615a58f9d..4ace34d71e 100644 --- a/src/components/include/utils/lock.h +++ b/src/components/include/utils/lock.h @@ -32,24 +32,17 @@ #ifndef SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ #define SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ -#if defined(OS_POSIX) -#include -#include -#else -#error Please implement lock for your OS -#endif #include -#include "utils/macro.h" +#include +#include #include "utils/atomic.h" +#include "utils/macro.h" #include "utils/memory_barrier.h" +#include -namespace sync_primitives { +using boost::system::error_code; -namespace impl { -#if defined(OS_POSIX) -typedef pthread_mutex_t PlatformMutex; -#endif -} // namespace impl +namespace sync_primitives { class SpinMutex { public: @@ -78,71 +71,89 @@ class SpinMutex { volatile unsigned int state_; }; -/* Platform-indepenednt NON-RECURSIVE lock (mutex) wrapper - Please use AutoLock to ackquire and (automatically) release it - It eases balancing of multple lock taking/releasing and makes it - Impossible to forget to release the lock: - ... - ConcurentlyAccessedData data_; - sync_primitives::Lock data_lock_; - ... - { - sync_primitives::AutoLock auto_lock(data_lock_); - data_.ReadOrWriteData(); - } // lock is automatically released here -*/ -class Lock { +/* Abstract base class that allows AutoLock to handle both recursive and + * non-recursive locks + */ +class BaseLock { public: - Lock(); - Lock(bool is_recursive); - ~Lock(); - + BaseLock() : lock_taken_(0) {} // Ackquire the lock. Must be called only once on a thread. // Please consider using AutoLock to capture it. - void Acquire(); + virtual void Acquire() = 0; // Release the lock. Must be called only once on a thread after lock. // was acquired. Please consider using AutoLock to automatically release // the lock - void Release(); + virtual void Release() = 0; // Try if lock can be captured and lock it if it was possible. // If it captured, lock must be manually released calling to Release // when protected resource access was finished. // @returns wether lock was captured. - bool Try(); - - private: - impl::PlatformMutex mutex_; + virtual bool Try() = 0; -#ifndef NDEBUG + protected: /** - * @brief Basic debugging aid, a flag that signals wether this lock is - * currently taken - * Allows detection of abandoned and recursively captured mutexes - */ + * @brief Basic debugging aid, a flag that signals wether this lock is + * currently taken + * Allows detection of abandoned and recursively captured mutexes + */ uint32_t lock_taken_; - /** - * @brief Describe if mutex is recurcive or not - */ - bool is_mutex_recursive_; + // Ensures safety in locking + virtual void AssertTakenAndMarkFree() = 0; + virtual void AssertFreeAndMarkTaken() = 0; - void AssertFreeAndMarkTaken(); - void AssertTakenAndMarkFree(); -#else - void AssertFreeAndMarkTaken() {} - void AssertTakenAndMarkFree() {} -#endif +friend class ConditionalVariable; +}; - void Init(bool is_recursive); +/* + * Platform-indepenednt NON-RECURSIVE lock (mutex) wrapper + */ +class Lock : public BaseLock { + public: + Lock(); + ~Lock(); - friend class ConditionalVariable; + virtual void Acquire(); + + virtual void Release(); + + virtual bool Try(); + + private: + virtual void AssertTakenAndMarkFree(); + virtual void AssertFreeAndMarkTaken(); + boost::mutex mutex_; DISALLOW_COPY_AND_ASSIGN(Lock); + friend class ConditionalVariable; +}; + +/* + * Platform-indepenednt RECURSIVE lock (mutex) wrapper + */ +class RecursiveLock : public BaseLock { + public: + RecursiveLock(); + ~RecursiveLock(); + + virtual void Acquire(); + + virtual void Release(); + + virtual bool Try(); + + private: + virtual void AssertTakenAndMarkFree(); + virtual void AssertFreeAndMarkTaken(); + boost::recursive_mutex mutex_; + DISALLOW_COPY_AND_ASSIGN(RecursiveLock); + friend class ConditionalVariable; }; // This class is used to automatically acquire and release the a lock class AutoLock { public: - explicit AutoLock(Lock& lock) : lock_(lock) { + explicit AutoLock(BaseLock& lock) : lock_(lock) { + std::cerr << "lock is at " << &lock << std::endl; lock_.Acquire(); } ~AutoLock() { @@ -150,21 +161,32 @@ class AutoLock { } private: - Lock& GetLock() { + BaseLock& GetLock() { return lock_; } - Lock& lock_; + BaseLock& lock_; private: friend class AutoUnlock; friend class ConditionalVariable; DISALLOW_COPY_AND_ASSIGN(AutoLock); }; - +/* Please use AutoLock to ackquire and (automatically) release it + * It eases balancing of multple lock taking/releasing and makes it + * Impossible to forget to release the lock: + * ... + * ConcurentlyAccessedData data_; + * sync_primitives::Lock data_lock_; + * ... + * { + * sync_primitives::AutoLock auto_lock(data_lock_); + * data_.ReadOrWriteData(); + * } // lock is automatically released here + */ // This class is used to temporarly unlock autolocked lock class AutoUnlock { public: - explicit AutoUnlock(Lock& lock) : lock_(lock) { + explicit AutoUnlock(BaseLock& lock) : lock_(lock) { lock_.Release(); } explicit AutoUnlock(AutoLock& lock) : lock_(lock.GetLock()) { @@ -175,7 +197,7 @@ class AutoUnlock { } private: - Lock& lock_; + BaseLock& lock_; private: DISALLOW_COPY_AND_ASSIGN(AutoUnlock); diff --git a/src/components/include/utils/lock_posix.h b/src/components/include/utils/lock_posix.h new file mode 100644 index 0000000000..ae8b64959f --- /dev/null +++ b/src/components/include/utils/lock_posix.h @@ -0,0 +1,184 @@ +/* + * Copyright (c) 2013, Ford Motor Company + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following + * disclaimer in the documentation and/or other materials provided with the + * distribution. + * + * Neither the name of the Ford Motor Company nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ +#ifndef SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ +#define SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ + +#if defined(OS_POSIX) +#include +#include +#else +#error Please implement lock for your OS +#endif +#include +#include "utils/macro.h" +#include "utils/atomic.h" +#include "utils/memory_barrier.h" + +namespace sync_primitives_posix { + +namespace impl { +#if defined(OS_POSIX) +typedef pthread_mutex_t PlatformMutex; +#endif +} // namespace impl + +class SpinMutex { + public: + SpinMutex() : state_(0) {} + void Lock() { + // Comment below add exception for lint error + // Reason: FlexeLint doesn't know about compiler's built-in instructions + /*lint -e1055*/ + if (atomic_post_set(&state_) == 0) { + return; + } + for (;;) { + sched_yield(); + /*lint -e1055*/ + if (state_ == 0 && atomic_post_set(&state_) == 0) { + return; + } + } + } + void Unlock() { + state_ = 0; + } + ~SpinMutex() {} + + private: + volatile unsigned int state_; +}; + +/* Platform-indepenednt NON-RECURSIVE lock (mutex) wrapper + Please use AutoLock to ackquire and (automatically) release it + It eases balancing of multple lock taking/releasing and makes it + Impossible to forget to release the lock: + ... + ConcurentlyAccessedData data_; + sync_primitives::Lock data_lock_; + ... + { + sync_primitives::AutoLock auto_lock(data_lock_); + data_.ReadOrWriteData(); + } // lock is automatically released here +*/ +class Lock { + public: + Lock(); + Lock(bool is_recursive); + ~Lock(); + + // Ackquire the lock. Must be called only once on a thread. + // Please consider using AutoLock to capture it. + void Acquire(); + // Release the lock. Must be called only once on a thread after lock. + // was acquired. Please consider using AutoLock to automatically release + // the lock + void Release(); + // Try if lock can be captured and lock it if it was possible. + // If it captured, lock must be manually released calling to Release + // when protected resource access was finished. + // @returns wether lock was captured. + bool Try(); + + private: + impl::PlatformMutex mutex_; + +#ifndef NDEBUG + /** + * @brief Basic debugging aid, a flag that signals wether this lock is + * currently taken + * Allows detection of abandoned and recursively captured mutexes + */ + uint32_t lock_taken_; + + /** + * @brief Describe if mutex is recurcive or not + */ + bool is_mutex_recursive_; + + void AssertFreeAndMarkTaken(); + void AssertTakenAndMarkFree(); +#else + void AssertFreeAndMarkTaken() {} + void AssertTakenAndMarkFree() {} +#endif + + void Init(bool is_recursive); + + friend class ConditionalVariable; + DISALLOW_COPY_AND_ASSIGN(Lock); +}; + +// This class is used to automatically acquire and release the a lock +class AutoLock { + public: + explicit AutoLock(Lock& lock) : lock_(lock) { + lock_.Acquire(); + } + ~AutoLock() { + lock_.Release(); + } + + private: + Lock& GetLock() { + return lock_; + } + Lock& lock_; + + private: + friend class AutoUnlock; + friend class ConditionalVariable; + DISALLOW_COPY_AND_ASSIGN(AutoLock); +}; + +// This class is used to temporarly unlock autolocked lock +class AutoUnlock { + public: + explicit AutoUnlock(Lock& lock) : lock_(lock) { + lock_.Release(); + } + explicit AutoUnlock(AutoLock& lock) : lock_(lock.GetLock()) { + lock_.Release(); + } + ~AutoUnlock() { + lock_.Acquire(); + } + + private: + Lock& lock_; + + private: + DISALLOW_COPY_AND_ASSIGN(AutoUnlock); +}; +} // namespace sync_primitives +#endif // SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ diff --git a/src/components/policy/policy_regular/include/policy/cache_manager.h b/src/components/policy/policy_regular/include/policy/cache_manager.h index 8c0acd44d2..464aa711bd 100644 --- a/src/components/policy/policy_regular/include/policy/cache_manager.h +++ b/src/components/policy/policy_regular/include/policy/cache_manager.h @@ -756,7 +756,7 @@ class CacheManager : public CacheManagerInterface { typedef std::set UnpairedDevices; UnpairedDevices is_unpaired_; - mutable sync_primitives::Lock cache_lock_; + mutable sync_primitives::RecursiveLock cache_lock_; sync_primitives::Lock unpaired_lock_; typedef std::map AppCalculatedPermissions; diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc index 6a142374d5..b10cd51e8b 100644 --- a/src/components/policy/policy_regular/src/cache_manager.cc +++ b/src/components/policy/policy_regular/src/cache_manager.cc @@ -33,18 +33,18 @@ #include "policy/cache_manager.h" #include -#include -#include #include +#include +#include #include -#include "utils/file_system.h" -#include "json/reader.h" #include "json/features.h" +#include "json/reader.h" #include "json/writer.h" -#include "utils/logger.h" #include "utils/date_time.h" +#include "utils/file_system.h" #include "utils/gen_hash.h" +#include "utils/logger.h" #include "utils/macro.h" #include "utils/threads/thread.h" #include "utils/threads/thread_delegate.h" @@ -102,8 +102,7 @@ CacheManager::CacheManager() : CacheManagerInterface() , pt_(new policy_table::Table) , backup_(new SQLPTRepresentation()) - , update_required(false) - , cache_lock_(true) { + , update_required(false) { LOG4CXX_AUTO_TRACE(logger_); backuper_ = new BackgroundBackuper(this); backup_thread_ = threads::CreateThread("Backup thread", backuper_); @@ -1635,8 +1634,9 @@ void CacheManager::MergeFG(const policy_table::PolicyTable& new_pt, void CacheManager::MergeAP(const policy_table::PolicyTable& new_pt, policy_table::PolicyTable& pt) { LOG4CXX_AUTO_TRACE(logger_); - pt.app_policies_section.device = const_cast( - new_pt).app_policies_section.device; + pt.app_policies_section.device = + const_cast(new_pt) + .app_policies_section.device; pt.app_policies_section.apps[kDefaultId] = const_cast(new_pt) diff --git a/src/components/utils/CMakeLists.txt b/src/components/utils/CMakeLists.txt index 51835c125a..46acf76e89 100644 --- a/src/components/utils/CMakeLists.txt +++ b/src/components/utils/CMakeLists.txt @@ -101,7 +101,7 @@ set(PATHS collect_sources(SOURCES "${PATHS}" "${EXCLUDE_PATHS}") if (CMAKE_SYSTEM_NAME STREQUAL "QNX") - # --- QDB Wrapper + # --- QDB Wrapper add_subdirectory(./src/qdb_wrapper) else () # --- SQLite Wrapper @@ -109,10 +109,11 @@ else () endif () if(${CMAKE_SYSTEM_NAME} MATCHES "Linux") - list(APPEND LIBRARIES dl pthread ${RTLIB}) + list(APPEND LIBRARIES dl pthread ${RTLIB}) endif() add_library("Utils" ${SOURCES}) +list(APPEND LIBRARIES -lboost_system -lboost_filesystem -lboost_thread) target_link_libraries("Utils" ${LIBRARIES}) if(ENABLE_LOG) diff --git a/src/components/utils/src/conditional_variable_posix.cc b/src/components/utils/src/conditional_variable_posix.cc index 50ebc74556..6b2115ad80 100644 --- a/src/components/utils/src/conditional_variable_posix.cc +++ b/src/components/utils/src/conditional_variable_posix.cc @@ -41,108 +41,98 @@ namespace { const long kNanosecondsPerSecond = 1000000000; const long kMillisecondsPerSecond = 1000; const long kNanosecondsPerMillisecond = 1000000; -} +} // namespace namespace sync_primitives { CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") -ConditionalVariable::ConditionalVariable() { - pthread_condattr_t attrs; - int initialized = pthread_condattr_init(&attrs); - if (initialized != 0) - LOG4CXX_ERROR(logger_, - "Failed to initialize " - "conditional variable attributes"); - pthread_condattr_setclock(&attrs, CLOCK_MONOTONIC); - initialized = pthread_cond_init(&cond_var_, &attrs); - if (initialized != 0) - LOG4CXX_ERROR(logger_, - "Failed to initialize " - "conditional variable"); - int rv = pthread_condattr_destroy(&attrs); - if (rv != 0) - LOG4CXX_ERROR(logger_, - "Failed to destroy " - "conditional variable attributes"); -} +ConditionalVariable::ConditionalVariable() {} -ConditionalVariable::~ConditionalVariable() { - pthread_cond_destroy(&cond_var_); -} +ConditionalVariable::~ConditionalVariable() {} void ConditionalVariable::NotifyOne() { - int signaled = pthread_cond_signal(&cond_var_); - if (signaled != 0) - LOG4CXX_ERROR(logger_, "Failed to signal conditional variable"); + cond_var_.notify_one(); } void ConditionalVariable::Broadcast() { - int signaled = pthread_cond_broadcast(&cond_var_); - if (signaled != 0) - LOG4CXX_ERROR(logger_, "Failed to broadcast conditional variable"); + cond_var_.notify_all(); } -bool ConditionalVariable::Wait(Lock& lock) { - lock.AssertTakenAndMarkFree(); - int wait_status = pthread_cond_wait(&cond_var_, &lock.mutex_); - lock.AssertFreeAndMarkTaken(); - if (wait_status != 0) { - LOG4CXX_ERROR(logger_, "Failed to wait for conditional variable"); +bool ConditionalVariable::Wait(BaseLock& lock) { + // NOTE this grossness is due to boost mutex and recursive mutex not sharing a + // superclass + + try { + lock.AssertTakenAndMarkFree(); + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + cond_var_.wait(test_lock->mutex_); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + cond_var_.wait(test_rec_lock->mutex_); + } else { + // unknown + LOG4CXX_ERROR(logger_, "Unknown lock type!"); + return false; + } + lock.AssertFreeAndMarkTaken(); + } catch (std::exception err) { + LOG4CXX_ERROR( + logger_, + "Failed to wait for conditional variable, exception:" << err.what()); return false; } + return true; } bool ConditionalVariable::Wait(AutoLock& auto_lock) { - Lock& lock = auto_lock.GetLock(); - lock.AssertTakenAndMarkFree(); - int wait_status = pthread_cond_wait(&cond_var_, &lock.mutex_); - lock.AssertFreeAndMarkTaken(); - if (wait_status != 0) { - LOG4CXX_ERROR(logger_, "Failed to wait for conditional variable"); - return false; - } - return true; + BaseLock& lock = auto_lock.GetLock(); + return Wait(lock); } ConditionalVariable::WaitStatus ConditionalVariable::WaitFor( AutoLock& auto_lock, uint32_t milliseconds) { - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - timespec wait_interval; - wait_interval.tv_sec = now.tv_sec + (milliseconds / kMillisecondsPerSecond); - wait_interval.tv_nsec = - now.tv_nsec + - (milliseconds % kMillisecondsPerSecond) * kNanosecondsPerMillisecond; - wait_interval.tv_sec += wait_interval.tv_nsec / kNanosecondsPerSecond; - wait_interval.tv_nsec %= kNanosecondsPerSecond; - Lock& lock = auto_lock.GetLock(); - lock.AssertTakenAndMarkFree(); - int timedwait_status = - pthread_cond_timedwait(&cond_var_, &lock.mutex_, &wait_interval); - lock.AssertFreeAndMarkTaken(); + BaseLock& lock = auto_lock.GetLock(); + WaitStatus wait_status = kNoTimeout; - switch (timedwait_status) { - case 0: { - wait_status = kNoTimeout; - break; - } - case EINTR: { - wait_status = kNoTimeout; - break; + lock.AssertTakenAndMarkFree(); + try { + bool timeout = true; + + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + // cond_var_.wait(test_lock->mutex_); + timeout = cond_var_.timed_wait( + test_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + // cond_var_.wait(test_rec_lock->mutex_); + timeout = cond_var_.timed_wait( + test_rec_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); + } else { + // unknown + LOG4CXX_ERROR(logger_, "Unknown lock type!"); } - case ETIMEDOUT: { + + if (!timeout) { wait_status = kTimeout; - break; - } - default: { - LOG4CXX_ERROR( - logger_, - "Failed to timewait for conditional variable timedwait_status: " - << timedwait_status); } + } catch (boost::thread_interrupted inter) { + wait_status = kNoTimeout; + + } catch (std::exception err) { + LOG4CXX_ERROR( + logger_, + "Failed to timewait for conditional variable timedwait_status: " + << err.what()); } + return wait_status; } diff --git a/src/components/utils/src/lock_boost.cc b/src/components/utils/src/lock_boost.cc new file mode 100644 index 0000000000..67f8b5d2db --- /dev/null +++ b/src/components/utils/src/lock_boost.cc @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2013, Ford Motor Company + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following + * disclaimer in the documentation and/or other materials provided with the + * distribution. + * + * Neither the name of the Ford Motor Company nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include +#include +#include +#include +#include "utils/lock.h" +#include "utils/logger.h" + +namespace sync_primitives { + +CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") + +Lock::Lock() {} + +Lock::~Lock() { + if (lock_taken_ > 0) { + LOG4CXX_ERROR(logger_, "Destroying non-released regular mutex " << &mutex_); + } +} + +void Lock::Acquire() { + try { + mutex_.lock(); + } catch (std::exception err) { + LOG4CXX_FATAL(logger_, + "Failed to acquire mutex " << &mutex_ << ": " << err.what()); + NOTREACHED(); + } + AssertFreeAndMarkTaken(); +} + +void Lock::Release() { + AssertTakenAndMarkFree(); + + mutex_.unlock(); +} + +bool Lock::Try() { + bool status = mutex_.try_lock(); + if (status) { + lock_taken_++; + } + return status; +} + +void Lock::AssertFreeAndMarkTaken() { + if (lock_taken_ != 0) { + LOG4CXX_ERROR(logger_, "Locking already taken not recursive mutex"); + NOTREACHED(); + } + + lock_taken_++; +} + +void Lock::AssertTakenAndMarkFree() { + if (lock_taken_ == 0) { + LOG4CXX_ERROR(logger_, "Unlocking a mutex that is not taken"); + NOTREACHED(); + } + lock_taken_--; +} + +// Recursive lock looks the same on the surface, some code duplication is +// necessary since they don't have a shared parent superclass +RecursiveLock::RecursiveLock() {} + +RecursiveLock::~RecursiveLock() { + if (lock_taken_ > 0) { + LOG4CXX_ERROR(logger_, + "Destroying non-released recursive mutex " << &mutex_); + } +} + +void RecursiveLock::Acquire() { + try { + mutex_.lock(); + } catch (std::exception err) { + LOG4CXX_FATAL(logger_, + "Failed to acquire mutex " << &mutex_ << ": " << err.what()); + NOTREACHED(); + } + + AssertFreeAndMarkTaken(); +} // namespace sync_primitives_boost + +void RecursiveLock::Release() { + AssertTakenAndMarkFree(); + mutex_.unlock(); +} + +bool RecursiveLock::Try() { + bool status = mutex_.try_lock(); + if (status) { + lock_taken_++; + } + return status; +} + +void RecursiveLock::AssertFreeAndMarkTaken() { + lock_taken_++; +} + +void RecursiveLock::AssertTakenAndMarkFree() { + if (lock_taken_ == 0) { + LOG4CXX_ERROR(logger_, "Unlocking a recursive mutex that is not taken"); + NOTREACHED(); + } + lock_taken_--; +} + +} // namespace sync_primitives diff --git a/src/components/utils/src/lock_posix.cc b/src/components/utils/src/lock_posix.cc index 9b90ad20b9..0f795bb669 100644 --- a/src/components/utils/src/lock_posix.cc +++ b/src/components/utils/src/lock_posix.cc @@ -30,7 +30,7 @@ * POSSIBILITY OF SUCH DAMAGE. */ -#include "utils/lock.h" +#include "utils/lock_posix.h" #include #include #include @@ -38,7 +38,7 @@ #include #include "utils/logger.h" -namespace sync_primitives { +namespace sync_primitives_posix { CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") diff --git a/src/components/utils/test/lock_boost_test.cc b/src/components/utils/test/lock_boost_test.cc new file mode 100644 index 0000000000..0da735a990 --- /dev/null +++ b/src/components/utils/test/lock_boost_test.cc @@ -0,0 +1,103 @@ +/* + * Copyright (c) 2015, Ford Motor Company + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following + * disclaimer in the documentation and/or other materials provided with the + * distribution. + * + * Neither the name of the Ford Motor Company nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include "gtest/gtest.h" +#include "utils/lock.h" + +namespace test { +namespace components { +namespace utils_test { + +using sync_primitives::Lock; +using sync_primitives::RecursiveLock; + +TEST(LockBoostTest, TestNonRecursive) { + // Create Lock object + Lock test_mutex; + // Lock mutex + test_mutex.Acquire(); + // Check if created mutex is non-recursive + EXPECT_FALSE(test_mutex.Try()); + // Release mutex before destroy + test_mutex.Release(); +} + +TEST(LockBoostTest, TestRecursive) { + // Create Lock object + RecursiveLock test_mutex; + // Lock mutex + test_mutex.Acquire(); + // Check if created mutex is recursive + EXPECT_TRUE(test_mutex.Try()); + // Release mutex before destroy + test_mutex.Release(); + test_mutex.Release(); +} + +TEST(LockBoostTest, ReleaseMutex_ExpectMutexReleased) { + // Create Lock object (non-recursive mutex) + Lock test_mutex; + // Lock mutex + test_mutex.Acquire(); + // Release mutex + test_mutex.Release(); + // Try to lock it again. If released expect true + EXPECT_TRUE(test_mutex.Try()); + test_mutex.Release(); +} + +TEST(LockBoostTest, TryLockNonRecursiveMutex_ExpectMutexNotLockedTwice) { + // Create Lock object (non-recursive mutex) + Lock test_mutex; + // Lock mutex + test_mutex.Try(); + // Try to lock it again. If locked expect false + EXPECT_FALSE(test_mutex.Try()); + test_mutex.Release(); +} + +TEST(LockBoostTest, TryLockRecursiveMutex_ExpectMutexLockedTwice) { + // Create Lock object (recursive mutex) + RecursiveLock test_mutex; + // Lock mutex + test_mutex.Try(); + // Try to lock it again. Expect true and internal counter increase + EXPECT_TRUE(test_mutex.Try()); + // Release mutex twice as was locked twice. + // Every Release() will decrement internal counter + test_mutex.Release(); + test_mutex.Release(); +} + +} // namespace utils_test +} // namespace components +} // namespace test diff --git a/src/components/utils/test/lock_posix_test.cc b/src/components/utils/test/lock_posix_test.cc index a78659ab31..04f2f5fa34 100644 --- a/src/components/utils/test/lock_posix_test.cc +++ b/src/components/utils/test/lock_posix_test.cc @@ -31,13 +31,13 @@ */ #include "gtest/gtest.h" -#include "utils/lock.h" +#include "utils/lock_posix.h" namespace test { namespace components { namespace utils_test { -using sync_primitives::Lock; +using sync_primitives_posix::Lock; TEST(LockPosixTest, DefaultCtorTest_ExpectNonRecursiveMutexCreated) { // Create Lock object -- cgit v1.2.1 From 09c941dd2ffd4f98ba706895a67133318c3c5007 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Tue, 5 Jun 2018 11:07:54 -0400 Subject: dump current implementation, currently stuck on bigger SDL lock scoping bug --- .../test/hmi_language_handler_test.cc | 2 +- .../test/message_helper/message_helper_test.cc | 60 +++++++++++++------- src/components/include/utils/data_accessor.h | 2 + src/components/include/utils/lock.h | 28 +++++---- .../utils/src/conditional_variable_posix.cc | 55 ++++++++++-------- src/components/utils/src/lock_boost.cc | 66 ++++++++++++++-------- .../utils/test/conditional_variable_test.cc | 7 ++- src/components/utils/test/message_queue_test.cc | 3 + 8 files changed, 140 insertions(+), 83 deletions(-) diff --git a/src/components/application_manager/test/hmi_language_handler_test.cc b/src/components/application_manager/test/hmi_language_handler_test.cc index 2020da5052..c0138710d4 100644 --- a/src/components/application_manager/test/hmi_language_handler_test.cc +++ b/src/components/application_manager/test/hmi_language_handler_test.cc @@ -129,11 +129,11 @@ class HmiLanguageHandlerTest : public ::testing::Test { } } + ::sync_primitives::Lock app_set_lock_; MockApplicationManager app_manager_; MockHMICapabilities hmi_capabilities_; MockEventDispatcher event_dispatcher_; SharedPtr hmi_language_handler_; - ::sync_primitives::Lock app_set_lock_; resumption_test::MockLastState last_state_; }; diff --git a/src/components/application_manager/test/message_helper/message_helper_test.cc b/src/components/application_manager/test/message_helper/message_helper_test.cc index dc4b3f48bf..aef80866b6 100644 --- a/src/components/application_manager/test/message_helper/message_helper_test.cc +++ b/src/components/application_manager/test/message_helper/message_helper_test.cc @@ -33,18 +33,17 @@ #include #include -#include "gmock/gmock.h" -#include "utils/macro.h" -#include "utils/make_shared.h" -#include "application_manager/policies/policy_handler.h" +#include "application_manager/event_engine/event_dispatcher.h" #include "application_manager/mock_application.h" -#include "utils/custom_string.h" -#include "policy/mock_policy_settings.h" -#include "application_manager/policies/policy_handler.h" #include "application_manager/mock_application_manager.h" -#include "application_manager/event_engine/event_dispatcher.h" -#include "application_manager/state_controller.h" +#include "application_manager/policies/policy_handler.h" #include "application_manager/resumption/resume_ctrl.h" +#include "application_manager/state_controller.h" +#include "gmock/gmock.h" +#include "policy/mock_policy_settings.h" +#include "utils/custom_string.h" +#include "utils/macro.h" +#include "utils/make_shared.h" #ifdef EXTERNAL_PROPRIETARY_MODE #include "policy/policy_external/include/policy/policy_types.h" @@ -64,12 +63,12 @@ typedef utils::SharedPtr MockApplicationSharedPtr; typedef std::vector StringArray; typedef utils::SharedPtr ApplicationSharedPtr; +using testing::_; using testing::AtLeast; -using testing::ReturnRefOfCopy; -using testing::ReturnRef; using testing::Return; +using testing::ReturnRef; +using testing::ReturnRefOfCopy; using testing::SaveArg; -using testing::_; TEST(MessageHelperTestCreate, CreateBlockedByPoliciesResponse_SmartObject_Equal) { @@ -234,9 +233,13 @@ TEST(MessageHelperTestCreate, CreateShowRequestToHMI_SendSmartObject_Equal) { TEST(MessageHelperTestCreate, CreateAddCommandRequestToHMI_SendSmartObject_Empty) { + sync_primitives::RecursiveLock access_lock; + MockApplicationSharedPtr appSharedMock = utils::MakeShared(); ::application_manager::CommandsMap vis; - DataAccessor data_accessor(vis, sync_primitives::RecursiveLock()); + + DataAccessor data_accessor(vis, + access_lock); EXPECT_CALL(*appSharedMock, commands_map()).WillOnce(Return(data_accessor)); application_manager_test::MockApplicationManager mock_application_manager; @@ -249,9 +252,11 @@ TEST(MessageHelperTestCreate, TEST(MessageHelperTestCreate, CreateAddCommandRequestToHMI_SendSmartObject_Equal) { + sync_primitives::RecursiveLock access_lock; + MockApplicationSharedPtr appSharedMock = utils::MakeShared(); CommandsMap vis; - DataAccessor data_accessor(vis, sync_primitives::RecursiveLock()); + DataAccessor data_accessor(vis, access_lock); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -290,9 +295,12 @@ TEST(MessageHelperTestCreate, TEST(MessageHelperTestCreate, CreateAddVRCommandRequestFromChoiceToHMI_SendEmptyData_EmptyList) { + sync_primitives::RecursiveLock access_lock; MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::ChoiceSetMap vis; - DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, sync_primitives::RecursiveLock()); + + DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, + access_lock); EXPECT_CALL(*appSharedMock, choice_set_map()).WillOnce(Return(data_accessor)); application_manager_test::MockApplicationManager mock_application_manager; @@ -305,9 +313,12 @@ TEST(MessageHelperTestCreate, TEST(MessageHelperTestCreate, CreateAddVRCommandRequestFromChoiceToHMI_SendObject_EqualList) { + sync_primitives::RecursiveLock access_lock; MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::ChoiceSetMap vis; - DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, sync_primitives::RecursiveLock()); + + DataAccessor< ::application_manager::ChoiceSetMap> data_accessor(vis, + access_lock); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -351,9 +362,12 @@ TEST(MessageHelperTestCreate, } TEST(MessageHelperTestCreate, CreateAddSubMenuRequestToHMI_SendObject_Equal) { + sync_primitives::RecursiveLock access_lock; MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::SubMenuMap vis; - DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, sync_primitives::RecursiveLock()); + + DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, + access_lock); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -390,9 +404,12 @@ TEST(MessageHelperTestCreate, CreateAddSubMenuRequestToHMI_SendObject_Equal) { TEST(MessageHelperTestCreate, CreateAddSubMenuRequestToHMI_SendEmptyMap_EmptySmartObjectList) { + sync_primitives::RecursiveLock access_lock; MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::SubMenuMap vis; - DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, sync_primitives::RecursiveLock()); + + DataAccessor< ::application_manager::SubMenuMap> data_accessor(vis, + access_lock); EXPECT_CALL(*appSharedMock, sub_menu_map()).WillOnce(Return(data_accessor)); @@ -708,12 +725,14 @@ TEST_F(MessageHelperTest, VerifySoftButtonString_CorrectStrings_True) { TEST_F(MessageHelperTest, GetIVISubscriptionRequests_ValidApplication_HmiRequestNotEmpty) { + sync_primitives::RecursiveLock access_lock; // Creating sharedPtr to MockApplication MockApplicationSharedPtr appSharedMock = utils::MakeShared(); // Creating data acessor application_manager::VehicleInfoSubscriptions vis; + DataAccessor data_accessor( - vis, sync_primitives::RecursiveLock()); + vis, access_lock); // Calls for ApplicationManager EXPECT_CALL(*appSharedMock, app_id()).WillOnce(Return(1u)); EXPECT_CALL(*appSharedMock, SubscribedIVI()).WillOnce(Return(data_accessor)); @@ -937,7 +956,8 @@ TEST_F(MessageHelperTest, SubscribeApplicationToSoftButton_CallFromApp) { size_t function_id = 1; // EXPECT_CALL(*appSharedPtr, - SubscribeToSoftButtons(function_id, SoftButtonID())).Times(1); + SubscribeToSoftButtons(function_id, SoftButtonID())) + .Times(1); MessageHelper::SubscribeApplicationToSoftButton( message_params, appSharedPtr, function_id); } diff --git a/src/components/include/utils/data_accessor.h b/src/components/include/utils/data_accessor.h index 6239812678..7ad2252a65 100644 --- a/src/components/include/utils/data_accessor.h +++ b/src/components/include/utils/data_accessor.h @@ -52,6 +52,8 @@ class DataAccessor { } ~DataAccessor() { + // std::cerr << "destructing accessor, counter is " << *counter_ << "for lock " + // << &lock_ << "\n"; if (0 == *counter_) { lock_.Release(); } else { diff --git a/src/components/include/utils/lock.h b/src/components/include/utils/lock.h index 4ace34d71e..d167f5bb54 100644 --- a/src/components/include/utils/lock.h +++ b/src/components/include/utils/lock.h @@ -35,10 +35,10 @@ #include #include #include +#include #include "utils/atomic.h" #include "utils/macro.h" #include "utils/memory_barrier.h" -#include using boost::system::error_code; @@ -76,7 +76,8 @@ class SpinMutex { */ class BaseLock { public: - BaseLock() : lock_taken_(0) {} + BaseLock() {} + virtual ~BaseLock() {} // Ackquire the lock. Must be called only once on a thread. // Please consider using AutoLock to capture it. virtual void Acquire() = 0; @@ -91,18 +92,11 @@ class BaseLock { virtual bool Try() = 0; protected: - /** - * @brief Basic debugging aid, a flag that signals wether this lock is - * currently taken - * Allows detection of abandoned and recursively captured mutexes - */ - uint32_t lock_taken_; - // Ensures safety in locking virtual void AssertTakenAndMarkFree() = 0; virtual void AssertFreeAndMarkTaken() = 0; -friend class ConditionalVariable; + friend class ConditionalVariable; }; /* @@ -120,6 +114,12 @@ class Lock : public BaseLock { virtual bool Try(); private: + /** + * @brief Basic debugging aid, a flag that signals wether this lock is + * currently taken + * Allows detection of abandoned and recursively captured mutexes + */ + uint32_t lock_taken_; virtual void AssertTakenAndMarkFree(); virtual void AssertFreeAndMarkTaken(); boost::mutex mutex_; @@ -142,6 +142,12 @@ class RecursiveLock : public BaseLock { virtual bool Try(); private: + /** + * @brief Basic debugging aid, a flag that signals wether this lock is + * currently taken + * Allows detection of abandoned and recursively captured mutexes + */ + uint32_t lock_taken_; virtual void AssertTakenAndMarkFree(); virtual void AssertFreeAndMarkTaken(); boost::recursive_mutex mutex_; @@ -153,7 +159,7 @@ class RecursiveLock : public BaseLock { class AutoLock { public: explicit AutoLock(BaseLock& lock) : lock_(lock) { - std::cerr << "lock is at " << &lock << std::endl; + // std::cerr << "lock is at " << &lock << std::endl; lock_.Acquire(); } ~AutoLock() { diff --git a/src/components/utils/src/conditional_variable_posix.cc b/src/components/utils/src/conditional_variable_posix.cc index 6b2115ad80..109e2bac9b 100644 --- a/src/components/utils/src/conditional_variable_posix.cc +++ b/src/components/utils/src/conditional_variable_posix.cc @@ -63,28 +63,32 @@ bool ConditionalVariable::Wait(BaseLock& lock) { // NOTE this grossness is due to boost mutex and recursive mutex not sharing a // superclass - try { - lock.AssertTakenAndMarkFree(); - // What kind of lock are we ? - if (Lock* test_lock = dynamic_cast(&lock)) { - // Regular lock - cond_var_.wait(test_lock->mutex_); - } else if (RecursiveLock* test_rec_lock = - dynamic_cast(&lock)) { - // Recursive lock - cond_var_.wait(test_rec_lock->mutex_); - } else { - // unknown - LOG4CXX_ERROR(logger_, "Unknown lock type!"); - return false; - } + lock.AssertTakenAndMarkFree(); + // try { + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + cond_var_.wait(test_lock->mutex_); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + cond_var_.wait(test_rec_lock->mutex_); + } else { + // unknown lock.AssertFreeAndMarkTaken(); - } catch (std::exception err) { - LOG4CXX_ERROR( - logger_, - "Failed to wait for conditional variable, exception:" << err.what()); + + LOG4CXX_ERROR(logger_, "Unknown lock type!"); return false; } + // } catch (std::exception err) { + // lock.AssertFreeAndMarkTaken(); + // + // LOG4CXX_ERROR( + // logger_, + // "Failed to wait for conditional variable, exception:" << err.what()); + // return false; + // } + lock.AssertFreeAndMarkTaken(); return true; } @@ -125,13 +129,14 @@ ConditionalVariable::WaitStatus ConditionalVariable::WaitFor( } } catch (boost::thread_interrupted inter) { wait_status = kNoTimeout; - - } catch (std::exception err) { - LOG4CXX_ERROR( - logger_, - "Failed to timewait for conditional variable timedwait_status: " - << err.what()); } + // catch (std::exception err) { + // LOG4CXX_ERROR( + // logger_, + // "Failed to timewait for conditional variable timedwait_status: " + // << err.what()); + // } + lock.AssertFreeAndMarkTaken(); return wait_status; } diff --git a/src/components/utils/src/lock_boost.cc b/src/components/utils/src/lock_boost.cc index 67f8b5d2db..e3c4a8cb4f 100644 --- a/src/components/utils/src/lock_boost.cc +++ b/src/components/utils/src/lock_boost.cc @@ -42,26 +42,33 @@ namespace sync_primitives { CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") -Lock::Lock() {} +Lock::Lock() : lock_taken_(0) {} Lock::~Lock() { + // std::cerr << "destructing lock " << &mutex_ << " with lock count " + // << lock_taken_ << '\n'; if (lock_taken_ > 0) { - LOG4CXX_ERROR(logger_, "Destroying non-released regular mutex " << &mutex_); + LOG4CXX_FATAL(logger_, "Destroying non-released regular mutex " << &mutex_); } } void Lock::Acquire() { - try { - mutex_.lock(); - } catch (std::exception err) { - LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " << err.what()); - NOTREACHED(); - } + // try { + mutex_.lock(); + // std::cerr << "acquiring lock " << &mutex_ << " with lock count " + // << lock_taken_ << '\n'; + // } catch (std::exception err) { + // LOG4CXX_FATAL(logger_, + // "Failed to acquire mutex " << &mutex_ << ": " << + // err.what()); + // NOTREACHED(); + // } AssertFreeAndMarkTaken(); } void Lock::Release() { + // std::cerr << "releasing lock " << &mutex_ << " with lock count " + // << lock_taken_ << '\n'; AssertTakenAndMarkFree(); mutex_.unlock(); @@ -69,15 +76,17 @@ void Lock::Release() { bool Lock::Try() { bool status = mutex_.try_lock(); + // std::cerr << "trying lock " << &mutex_ << " with lock count " << lock_taken_ + // << " status was " << status << '\n'; if (status) { - lock_taken_++; + AssertFreeAndMarkTaken(); } return status; } void Lock::AssertFreeAndMarkTaken() { if (lock_taken_ != 0) { - LOG4CXX_ERROR(logger_, "Locking already taken not recursive mutex"); + LOG4CXX_FATAL(logger_, "Locking already taken not recursive mutex"); NOTREACHED(); } @@ -86,7 +95,7 @@ void Lock::AssertFreeAndMarkTaken() { void Lock::AssertTakenAndMarkFree() { if (lock_taken_ == 0) { - LOG4CXX_ERROR(logger_, "Unlocking a mutex that is not taken"); + LOG4CXX_FATAL(logger_, "Unlocking a mutex that is not taken"); NOTREACHED(); } lock_taken_--; @@ -94,36 +103,45 @@ void Lock::AssertTakenAndMarkFree() { // Recursive lock looks the same on the surface, some code duplication is // necessary since they don't have a shared parent superclass -RecursiveLock::RecursiveLock() {} +RecursiveLock::RecursiveLock() : lock_taken_(0) {} RecursiveLock::~RecursiveLock() { + // std::cerr << "destructing recursive lock " << &mutex_ << " with lock count " + // << lock_taken_ << '\n'; if (lock_taken_ > 0) { - LOG4CXX_ERROR(logger_, + LOG4CXX_FATAL(logger_, "Destroying non-released recursive mutex " << &mutex_); } } void RecursiveLock::Acquire() { - try { - mutex_.lock(); - } catch (std::exception err) { - LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " << err.what()); - NOTREACHED(); - } + // try { + mutex_.lock(); + // std::cerr << "acquiring recursive lock " << &mutex_ << " with lock count " + // << lock_taken_ << '\n'; + // } catch (std::exception err) { + // LOG4CXX_FATAL(logger_, + // "Failed to acquire mutex " << &mutex_ << ": " << + // err.what()); + // NOTREACHED(); + // } AssertFreeAndMarkTaken(); -} // namespace sync_primitives_boost +} void RecursiveLock::Release() { + std::cerr << "releasing recursive lock " << &mutex_ << " with lock count " + << lock_taken_ << '\n'; AssertTakenAndMarkFree(); mutex_.unlock(); } bool RecursiveLock::Try() { bool status = mutex_.try_lock(); + // std::cerr << "trying recursive lock " << &mutex_ << " with lock count " + // << lock_taken_ << " status was " << status << '\n'; if (status) { - lock_taken_++; + AssertFreeAndMarkTaken(); } return status; } @@ -134,7 +152,7 @@ void RecursiveLock::AssertFreeAndMarkTaken() { void RecursiveLock::AssertTakenAndMarkFree() { if (lock_taken_ == 0) { - LOG4CXX_ERROR(logger_, "Unlocking a recursive mutex that is not taken"); + LOG4CXX_FATAL(logger_, "Unlocking a recursive mutex that is not taken"); NOTREACHED(); } lock_taken_--; diff --git a/src/components/utils/test/conditional_variable_test.cc b/src/components/utils/test/conditional_variable_test.cc index 524d53cafa..290ff3434f 100644 --- a/src/components/utils/test/conditional_variable_test.cc +++ b/src/components/utils/test/conditional_variable_test.cc @@ -30,14 +30,14 @@ * POSSIBILITY OF SUCH DAMAGE. */ -#include #include +#include #include "gtest/gtest.h" +#include "utils/conditional_variable.h" #include "utils/lock.h" #include "utils/macro.h" -#include "utils/conditional_variable.h" namespace test { namespace components { @@ -102,6 +102,7 @@ TEST_F(ConditionalVariableTest, cond_var_.WaitFor(test_lock, 2000); std::string last_value("changed again by thread 1"); EXPECT_EQ(last_value, test_value_); + pthread_join(thread1, NULL); } TEST_F(ConditionalVariableTest, @@ -116,6 +117,8 @@ TEST_F(ConditionalVariableTest, ASSERT_FALSE(thread_created) << "thread2 is not created!"; check_counter(); EXPECT_EQ(2u, counter_); + pthread_join(thread1, NULL); + pthread_join(thread2, NULL); } TEST_F( diff --git a/src/components/utils/test/message_queue_test.cc b/src/components/utils/test/message_queue_test.cc index ddc1aa1d81..9aa70933d6 100644 --- a/src/components/utils/test/message_queue_test.cc +++ b/src/components/utils/test/message_queue_test.cc @@ -158,6 +158,8 @@ TEST_F(MessageQueueTest, ASSERT_EQ(test_val_1, test_line); // Check the size of queue after 1 element was removed ASSERT_EQ(0u, test_queue.size()); + pthread_join(thread1, NULL); + pthread_join(thread2, NULL); } TEST_F(MessageQueueTest, @@ -169,6 +171,7 @@ TEST_F(MessageQueueTest, test_queue.wait(); check_value = true; ASSERT_TRUE(check_value); + pthread_join(thread1, NULL); } } // namespace utils_test -- cgit v1.2.1 From 24d8f16ce9c4ba5f2c09f68fff66e0f46383b6c1 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 18 Jul 2018 13:44:53 -0400 Subject: Cleanup, add some cmake support --- src/3rd_party/CMakeLists.txt | 4 +- .../connection_handler/connection_handler_impl.h | 2 +- .../connection_handler/src/connection.cc | 2 +- .../src/connection_handler_impl.cc | 2 +- src/components/include/utils/lock.h | 3 + src/components/include/utils/lock_posix.h | 184 --------------------- src/components/utils/CMakeLists.txt | 4 +- src/components/utils/src/lock_posix.cc | 149 ----------------- src/components/utils/test/lock_posix_test.cc | 123 -------------- 9 files changed, 11 insertions(+), 462 deletions(-) delete mode 100644 src/components/include/utils/lock_posix.h delete mode 100644 src/components/utils/src/lock_posix.cc delete mode 100644 src/components/utils/test/lock_posix_test.cc diff --git a/src/3rd_party/CMakeLists.txt b/src/3rd_party/CMakeLists.txt index ae9267e5bb..b72d86f902 100644 --- a/src/3rd_party/CMakeLists.txt +++ b/src/3rd_party/CMakeLists.txt @@ -227,9 +227,9 @@ if (HMIADAPTER STREQUAL "messagebroker") URL https://dl.bintray.com/boostorg/release/1.66.0/source/boost_1_66_0.tar.gz DOWNLOAD_DIR ${BOOST_LIB_SOURCE_DIRECTORY} SOURCE_DIR ${BOOST_LIB_SOURCE_DIRECTORY} - CONFIGURE_COMMAND ./bootstrap.sh --with-libraries=system --prefix=${3RD_PARTY_INSTALL_PREFIX} + CONFIGURE_COMMAND ./bootstrap.sh --with-libraries=system,thread --prefix=${3RD_PARTY_INSTALL_PREFIX} BUILD_COMMAND ./b2 - INSTALL_COMMAND ${BOOST_INSTALL_COMMAND} --with-system --prefix=${3RD_PARTY_INSTALL_PREFIX} > boost_install.log + INSTALL_COMMAND ${BOOST_INSTALL_COMMAND} --with-system --with-thread --prefix=${3RD_PARTY_INSTALL_PREFIX} > boost_install.log INSTALL_DIR ${3RD_PARTY_INSTALL_PREFIX} BUILD_IN_SOURCE true ) diff --git a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h index 121d5ed08c..730d7ebe32 100644 --- a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h +++ b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h @@ -640,7 +640,7 @@ class ConnectionHandlerImpl * @brief session/connection map */ SessionConnectionMap session_connection_map_; - mutable std::shared_ptr + mutable std::shared_ptr session_connection_map_lock_ptr_; /** diff --git a/src/components/connection_handler/src/connection.cc b/src/components/connection_handler/src/connection.cc index b8399b2c9b..08b37b9026 100644 --- a/src/components/connection_handler/src/connection.cc +++ b/src/components/connection_handler/src/connection.cc @@ -81,7 +81,7 @@ Connection::Connection(ConnectionHandle connection_handle, : connection_handler_(connection_handler) , connection_handle_(connection_handle) , connection_device_handle_(connection_device_handle) - , session_map_lock_(true) + , session_map_lock_() , primary_connection_handle_(0) , heartbeat_timeout_(heartbeat_timeout) { LOG4CXX_AUTO_TRACE(logger_); diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc index 270b38d724..9aa84989d9 100644 --- a/src/components/connection_handler/src/connection_handler_impl.cc +++ b/src/components/connection_handler/src/connection_handler_impl.cc @@ -69,7 +69,7 @@ ConnectionHandlerImpl::ConnectionHandlerImpl( , transport_manager_(tm) , protocol_handler_(NULL) , session_connection_map_lock_ptr_( - std::make_shared(true)) + std::make_shared()) , connection_list_lock_() , connection_handler_observer_lock_() , connection_list_deleter_(&connection_list_) diff --git a/src/components/include/utils/lock.h b/src/components/include/utils/lock.h index 1b27a4e915..bfa1ef1770 100644 --- a/src/components/include/utils/lock.h +++ b/src/components/include/utils/lock.h @@ -159,6 +159,9 @@ class RecursiveLock : public BaseLock { // This class is used to automatically acquire and release the a lock class AutoLock { public: + explicit AutoLock(const std::shared_ptr& lock) : lock_(*lock) { + lock_.Acquire(); + } explicit AutoLock(BaseLock& lock) : lock_(lock) { // std::cerr << "lock is at " << &lock << std::endl; lock_.Acquire(); diff --git a/src/components/include/utils/lock_posix.h b/src/components/include/utils/lock_posix.h deleted file mode 100644 index ae8b64959f..0000000000 --- a/src/components/include/utils/lock_posix.h +++ /dev/null @@ -1,184 +0,0 @@ -/* - * Copyright (c) 2013, Ford Motor Company - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * Redistributions of source code must retain the above copyright notice, this - * list of conditions and the following disclaimer. - * - * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following - * disclaimer in the documentation and/or other materials provided with the - * distribution. - * - * Neither the name of the Ford Motor Company nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ -#ifndef SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ -#define SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ - -#if defined(OS_POSIX) -#include -#include -#else -#error Please implement lock for your OS -#endif -#include -#include "utils/macro.h" -#include "utils/atomic.h" -#include "utils/memory_barrier.h" - -namespace sync_primitives_posix { - -namespace impl { -#if defined(OS_POSIX) -typedef pthread_mutex_t PlatformMutex; -#endif -} // namespace impl - -class SpinMutex { - public: - SpinMutex() : state_(0) {} - void Lock() { - // Comment below add exception for lint error - // Reason: FlexeLint doesn't know about compiler's built-in instructions - /*lint -e1055*/ - if (atomic_post_set(&state_) == 0) { - return; - } - for (;;) { - sched_yield(); - /*lint -e1055*/ - if (state_ == 0 && atomic_post_set(&state_) == 0) { - return; - } - } - } - void Unlock() { - state_ = 0; - } - ~SpinMutex() {} - - private: - volatile unsigned int state_; -}; - -/* Platform-indepenednt NON-RECURSIVE lock (mutex) wrapper - Please use AutoLock to ackquire and (automatically) release it - It eases balancing of multple lock taking/releasing and makes it - Impossible to forget to release the lock: - ... - ConcurentlyAccessedData data_; - sync_primitives::Lock data_lock_; - ... - { - sync_primitives::AutoLock auto_lock(data_lock_); - data_.ReadOrWriteData(); - } // lock is automatically released here -*/ -class Lock { - public: - Lock(); - Lock(bool is_recursive); - ~Lock(); - - // Ackquire the lock. Must be called only once on a thread. - // Please consider using AutoLock to capture it. - void Acquire(); - // Release the lock. Must be called only once on a thread after lock. - // was acquired. Please consider using AutoLock to automatically release - // the lock - void Release(); - // Try if lock can be captured and lock it if it was possible. - // If it captured, lock must be manually released calling to Release - // when protected resource access was finished. - // @returns wether lock was captured. - bool Try(); - - private: - impl::PlatformMutex mutex_; - -#ifndef NDEBUG - /** - * @brief Basic debugging aid, a flag that signals wether this lock is - * currently taken - * Allows detection of abandoned and recursively captured mutexes - */ - uint32_t lock_taken_; - - /** - * @brief Describe if mutex is recurcive or not - */ - bool is_mutex_recursive_; - - void AssertFreeAndMarkTaken(); - void AssertTakenAndMarkFree(); -#else - void AssertFreeAndMarkTaken() {} - void AssertTakenAndMarkFree() {} -#endif - - void Init(bool is_recursive); - - friend class ConditionalVariable; - DISALLOW_COPY_AND_ASSIGN(Lock); -}; - -// This class is used to automatically acquire and release the a lock -class AutoLock { - public: - explicit AutoLock(Lock& lock) : lock_(lock) { - lock_.Acquire(); - } - ~AutoLock() { - lock_.Release(); - } - - private: - Lock& GetLock() { - return lock_; - } - Lock& lock_; - - private: - friend class AutoUnlock; - friend class ConditionalVariable; - DISALLOW_COPY_AND_ASSIGN(AutoLock); -}; - -// This class is used to temporarly unlock autolocked lock -class AutoUnlock { - public: - explicit AutoUnlock(Lock& lock) : lock_(lock) { - lock_.Release(); - } - explicit AutoUnlock(AutoLock& lock) : lock_(lock.GetLock()) { - lock_.Release(); - } - ~AutoUnlock() { - lock_.Acquire(); - } - - private: - Lock& lock_; - - private: - DISALLOW_COPY_AND_ASSIGN(AutoUnlock); -}; -} // namespace sync_primitives -#endif // SRC_COMPONENTS_INCLUDE_UTILS_LOCK_H_ diff --git a/src/components/utils/CMakeLists.txt b/src/components/utils/CMakeLists.txt index 6449f123a4..0eb64ae8c2 100644 --- a/src/components/utils/CMakeLists.txt +++ b/src/components/utils/CMakeLists.txt @@ -114,8 +114,10 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Linux") endif() add_library("Utils" ${SOURCES}) -list(APPEND LIBRARIES -lboost_system -lboost_filesystem -lboost_thread) +list(APPEND LIBRARIES -lboost_system -lboost_thread) target_link_libraries("Utils" ${LIBRARIES}) +add_dependencies("Utils" Boost) + if(ENABLE_LOG) add_dependencies("Utils" install-3rd_party_logger) diff --git a/src/components/utils/src/lock_posix.cc b/src/components/utils/src/lock_posix.cc deleted file mode 100644 index 2807de1d0a..0000000000 --- a/src/components/utils/src/lock_posix.cc +++ /dev/null @@ -1,149 +0,0 @@ -/* - * Copyright (c) 2013, Ford Motor Company - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * Redistributions of source code must retain the above copyright notice, this - * list of conditions and the following disclaimer. - * - * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following - * disclaimer in the documentation and/or other materials provided with the - * distribution. - * - * Neither the name of the Ford Motor Company nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -#include "utils/lock_posix.h" -#include -#include -#include -#include -#include -#include "utils/logger.h" - -namespace sync_primitives_posix { - -CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") - -Lock::Lock() -#ifndef NDEBUG - : lock_taken_(0) - , is_mutex_recursive_(false) -#endif // NDEBUG -{ - Init(false); -} - -Lock::Lock(bool is_recursive) -#ifndef NDEBUG - : lock_taken_(0) - , is_mutex_recursive_(is_recursive) -#endif // NDEBUG -{ - Init(is_recursive); -} - -Lock::~Lock() { -#ifndef NDEBUG - if (lock_taken_ > 0) { - LOG4CXX_FATAL(logger_, "Destroying non-released mutex " << &mutex_); - NOTREACHED(); - } -#endif - int32_t status = pthread_mutex_destroy(&mutex_); - if (status != 0) { - LOG4CXX_FATAL(logger_, - "Failed to destroy mutex " << &mutex_ << ": " - << strerror(status)); - NOTREACHED(); - } -} - -void Lock::Acquire() { - const int32_t status = pthread_mutex_lock(&mutex_); - if (status != 0) { - LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " - << strerror(status)); - NOTREACHED(); - } else { - AssertFreeAndMarkTaken(); - } -} - -void Lock::Release() { - AssertTakenAndMarkFree(); - const int32_t status = pthread_mutex_unlock(&mutex_); - if (status != 0) { - LOG4CXX_FATAL(logger_, - "Failed to unlock mutex" << &mutex_ << ": " - << strerror(status)); - NOTREACHED(); - } -} - -bool Lock::Try() { - const int32_t status = pthread_mutex_trylock(&mutex_); - if (status == 0) { -#ifndef NDEBUG - lock_taken_++; -#endif - return true; - } - return false; -} - -#ifndef NDEBUG -void Lock::AssertFreeAndMarkTaken() { - if ((lock_taken_ > 0) && !is_mutex_recursive_) { - LOG4CXX_FATAL(logger_, "Locking already taken not recursive mutex"); - NOTREACHED(); - } - lock_taken_++; -} -void Lock::AssertTakenAndMarkFree() { - if (lock_taken_ == 0) { - LOG4CXX_FATAL(logger_, "Unlocking a mutex that is not taken"); - NOTREACHED(); - } - lock_taken_--; -} -#endif - -void Lock::Init(bool is_recursive) { - pthread_mutexattr_t attr; - pthread_mutexattr_init(&attr); - - const int32_t mutex_type = - is_recursive ? PTHREAD_MUTEX_RECURSIVE : PTHREAD_MUTEX_ERRORCHECK; - - pthread_mutexattr_settype(&attr, mutex_type); - const int32_t status = pthread_mutex_init(&mutex_, &attr); - - pthread_mutexattr_destroy(&attr); - - if (status != 0) { - LOG4CXX_FATAL(logger_, - "Failed to initialize mutex. " << std::strerror(status)); - DCHECK(status != 0); - } -} - -} // namespace sync_primitives diff --git a/src/components/utils/test/lock_posix_test.cc b/src/components/utils/test/lock_posix_test.cc deleted file mode 100644 index 04f2f5fa34..0000000000 --- a/src/components/utils/test/lock_posix_test.cc +++ /dev/null @@ -1,123 +0,0 @@ -/* - * Copyright (c) 2015, Ford Motor Company - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * Redistributions of source code must retain the above copyright notice, this - * list of conditions and the following disclaimer. - * - * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following - * disclaimer in the documentation and/or other materials provided with the - * distribution. - * - * Neither the name of the Ford Motor Company nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -#include "gtest/gtest.h" -#include "utils/lock_posix.h" - -namespace test { -namespace components { -namespace utils_test { - -using sync_primitives_posix::Lock; - -TEST(LockPosixTest, DefaultCtorTest_ExpectNonRecursiveMutexCreated) { - // Create Lock object - Lock test_mutex; - // Lock mutex - test_mutex.Acquire(); - // Check if created mutex is non-recursive - EXPECT_FALSE(test_mutex.Try()); - // Release mutex before destroy - test_mutex.Release(); -} - -TEST(LockPosixTest, CtorTestWithFalseArgument_ExpectNonRecursiveMutexCreated) { - // Create Lock object - Lock test_mutex(false); - // Lock mutex - test_mutex.Acquire(); - // Check if created mutex is non-recursive - EXPECT_FALSE(test_mutex.Try()); - // Release mutex before destroy - test_mutex.Release(); -} - -TEST(LockPosixTest, CtorTestWithTrueArgument_ExpectRecursiveMutexCreated) { - // Create Lock object - Lock test_mutex(true); - // Lock mutex - test_mutex.Acquire(); - // Check if created mutex is recursive - EXPECT_TRUE(test_mutex.Try()); - // Release mutex before destroy - test_mutex.Release(); - test_mutex.Release(); -} - -TEST(LockPosixTest, AcquireMutex_ExpectMutexLocked) { - // Create Lock object (non-recursive mutex) - Lock test_mutex; - // Lock mutex - test_mutex.Acquire(); - // Try to lock it again. If locked expect false - EXPECT_FALSE(test_mutex.Try()); - test_mutex.Release(); -} - -TEST(LockPosixTest, ReleaseMutex_ExpectMutexReleased) { - // Create Lock object (non-recursive mutex) - Lock test_mutex; - // Lock mutex - test_mutex.Acquire(); - // Release mutex - test_mutex.Release(); - // Try to lock it again. If released expect true - EXPECT_TRUE(test_mutex.Try()); - test_mutex.Release(); -} - -TEST(LockPosixTest, TryLockNonRecursiveMutex_ExpectMutexNotLockedTwice) { - // Create Lock object (non-recursive mutex) - Lock test_mutex; - // Lock mutex - test_mutex.Try(); - // Try to lock it again. If locked expect false - EXPECT_FALSE(test_mutex.Try()); - test_mutex.Release(); -} - -TEST(LockPosixTest, TryLockRecursiveMutex_ExpectMutexLockedTwice) { - // Create Lock object (recursive mutex) - Lock test_mutex(true); - // Lock mutex - test_mutex.Try(); - // Try to lock it again. Expect true and internal counter increase - EXPECT_TRUE(test_mutex.Try()); - // Release mutex twice as was locked twice. - // Every Release() will decrement internal counter - test_mutex.Release(); - test_mutex.Release(); -} - -} // namespace utils_test -} // namespace components -} // namespace test -- cgit v1.2.1 From f77b9f7348fb823253912e07d04bca0bba936e05 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 18 Jul 2018 14:09:53 -0400 Subject: ayyy it builds --- .../include/application_manager/application_data_impl.h | 4 ++-- .../application_manager/src/application_manager_impl.cc | 2 +- .../test/message_helper/message_helper_test.cc | 8 ++++---- src/components/policy/policy_regular/src/cache_manager.cc | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/application_manager/include/application_manager/application_data_impl.h b/src/components/application_manager/include/application_manager/application_data_impl.h index fbf6d87d0e..3f60040db9 100644 --- a/src/components/application_manager/include/application_manager/application_data_impl.h +++ b/src/components/application_manager/include/application_manager/application_data_impl.h @@ -279,9 +279,9 @@ class DynamicApplicationDataImpl : public virtual Application { SubMenuMap sub_menu_; mutable std::shared_ptr sub_menu_lock_ptr_; ChoiceSetMap choice_set_map_; - mutable std::shared_ptr choice_set_map_lock_ptr_; + mutable std::shared_ptr choice_set_map_lock_ptr_; PerformChoiceSetMap performinteraction_choice_set_map_; - mutable std::shared_ptr + mutable std::shared_ptr performinteraction_choice_set_lock_ptr_; uint32_t is_perform_interaction_active_; bool is_reset_global_properties_active_; diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index b91072598b..2c9f5d2f6f 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -165,7 +165,7 @@ ApplicationManagerImpl::ApplicationManagerImpl( , resume_ctrl_(new resumption::ResumeCtrlImpl(*this)) , navi_close_app_timeout_(am_settings.stop_streaming_timeout()) , navi_end_stream_timeout_(am_settings.stop_streaming_timeout()) - , stopping_application_mng_lock_(true) + , stopping_application_mng_lock_() , state_ctrl_(*this) , application_list_update_timer_( "AM ListUpdater", diff --git a/src/components/application_manager/test/message_helper/message_helper_test.cc b/src/components/application_manager/test/message_helper/message_helper_test.cc index b1871a6107..e7b7b7337f 100644 --- a/src/components/application_manager/test/message_helper/message_helper_test.cc +++ b/src/components/application_manager/test/message_helper/message_helper_test.cc @@ -312,7 +312,7 @@ TEST(MessageHelperTestCreate, MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::ChoiceSetMap vis; DataAccessor< ::application_manager::ChoiceSetMap> data_accessor( - vis, std::make_shared(true)); + vis, std::make_shared()); EXPECT_CALL(*appSharedMock, choice_set_map()).WillOnce(Return(data_accessor)); application_manager_test::MockApplicationManager mock_application_manager; @@ -328,7 +328,7 @@ TEST(MessageHelperTestCreate, MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::ChoiceSetMap vis; DataAccessor< ::application_manager::ChoiceSetMap> data_accessor( - vis, std::make_shared(true)); + vis, std::make_shared()); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -375,7 +375,7 @@ TEST(MessageHelperTestCreate, CreateAddSubMenuRequestToHMI_SendObject_Equal) { MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::SubMenuMap vis; DataAccessor< ::application_manager::SubMenuMap> data_accessor( - vis, std::make_shared(true)); + vis, std::make_shared()); smart_objects::SmartObjectSPtr smartObjectPtr = utils::MakeShared(); @@ -415,7 +415,7 @@ TEST(MessageHelperTestCreate, MockApplicationSharedPtr appSharedMock = utils::MakeShared(); application_manager::SubMenuMap vis; DataAccessor< ::application_manager::SubMenuMap> data_accessor( - vis, std::make_shared(true)); + vis, std::make_shared()); EXPECT_CALL(*appSharedMock, sub_menu_map()).WillOnce(Return(data_accessor)); diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc index aee5c02d4c..bfca801f74 100644 --- a/src/components/policy/policy_regular/src/cache_manager.cc +++ b/src/components/policy/policy_regular/src/cache_manager.cc @@ -103,7 +103,7 @@ CacheManager::CacheManager() : CacheManagerInterface() , pt_(new policy_table::Table) , backup_(new SQLPTRepresentation()) - , update_required(false) + , update_required(false){ LOG4CXX_AUTO_TRACE(logger_); backuper_ = new BackgroundBackuper(this); backup_thread_ = threads::CreateThread("Backup thread", backuper_); -- cgit v1.2.1 From baccac8a8b4ad7f9eca9130cf24b0ca35d6cfd4d Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 18 Jul 2018 14:59:02 -0400 Subject: Cleanup, add more error checking --- src/components/utils/src/lock_boost.cc | 49 +++++++++++----------------------- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/src/components/utils/src/lock_boost.cc b/src/components/utils/src/lock_boost.cc index e3c4a8cb4f..a76bd02a78 100644 --- a/src/components/utils/src/lock_boost.cc +++ b/src/components/utils/src/lock_boost.cc @@ -32,7 +32,6 @@ #include #include -#include #include #include #include "utils/lock.h" @@ -45,39 +44,30 @@ CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") Lock::Lock() : lock_taken_(0) {} Lock::~Lock() { - // std::cerr << "destructing lock " << &mutex_ << " with lock count " - // << lock_taken_ << '\n'; if (lock_taken_ > 0) { LOG4CXX_FATAL(logger_, "Destroying non-released regular mutex " << &mutex_); } } void Lock::Acquire() { - // try { + try { mutex_.lock(); - // std::cerr << "acquiring lock " << &mutex_ << " with lock count " - // << lock_taken_ << '\n'; - // } catch (std::exception err) { - // LOG4CXX_FATAL(logger_, - // "Failed to acquire mutex " << &mutex_ << ": " << - // err.what()); - // NOTREACHED(); - // } + } catch (std::exception err) { + LOG4CXX_FATAL(logger_, + "Failed to acquire mutex " << &mutex_ << ": " << + err.what()); + NOTREACHED(); + } AssertFreeAndMarkTaken(); } void Lock::Release() { - // std::cerr << "releasing lock " << &mutex_ << " with lock count " - // << lock_taken_ << '\n'; AssertTakenAndMarkFree(); - mutex_.unlock(); } bool Lock::Try() { bool status = mutex_.try_lock(); - // std::cerr << "trying lock " << &mutex_ << " with lock count " << lock_taken_ - // << " status was " << status << '\n'; if (status) { AssertFreeAndMarkTaken(); } @@ -89,7 +79,6 @@ void Lock::AssertFreeAndMarkTaken() { LOG4CXX_FATAL(logger_, "Locking already taken not recursive mutex"); NOTREACHED(); } - lock_taken_++; } @@ -106,8 +95,6 @@ void Lock::AssertTakenAndMarkFree() { RecursiveLock::RecursiveLock() : lock_taken_(0) {} RecursiveLock::~RecursiveLock() { - // std::cerr << "destructing recursive lock " << &mutex_ << " with lock count " - // << lock_taken_ << '\n'; if (lock_taken_ > 0) { LOG4CXX_FATAL(logger_, "Destroying non-released recursive mutex " << &mutex_); @@ -115,31 +102,25 @@ RecursiveLock::~RecursiveLock() { } void RecursiveLock::Acquire() { - // try { + try { mutex_.lock(); - // std::cerr << "acquiring recursive lock " << &mutex_ << " with lock count " - // << lock_taken_ << '\n'; - // } catch (std::exception err) { - // LOG4CXX_FATAL(logger_, - // "Failed to acquire mutex " << &mutex_ << ": " << - // err.what()); - // NOTREACHED(); - // } - + } catch (std::exception err) { + LOG4CXX_FATAL(logger_, + "Failed to acquire mutex " << &mutex_ << ": " << + err.what()); + NOTREACHED(); + } AssertFreeAndMarkTaken(); } void RecursiveLock::Release() { - std::cerr << "releasing recursive lock " << &mutex_ << " with lock count " - << lock_taken_ << '\n'; + AssertTakenAndMarkFree(); mutex_.unlock(); } bool RecursiveLock::Try() { bool status = mutex_.try_lock(); - // std::cerr << "trying recursive lock " << &mutex_ << " with lock count " - // << lock_taken_ << " status was " << status << '\n'; if (status) { AssertFreeAndMarkTaken(); } -- cgit v1.2.1 From 1257372af4cc6d6b350f4a91ec823f9c00161858 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 18 Jul 2018 15:01:15 -0400 Subject: style fix --- .../include/application_manager/application_manager_impl.h | 3 ++- .../src/app_launch/app_launch_data_json.cc | 3 +-- .../application_manager/src/application_data_impl.cc | 3 ++- .../application_manager/src/application_manager_impl.cc | 3 ++- .../src/event_engine/event_dispatcher_impl.cc | 3 +-- src/components/policy/policy_regular/src/cache_manager.cc | 2 +- src/components/utils/src/lock_boost.cc | 11 ++++------- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h index 79d5dda867..4fee30eb8a 100644 --- a/src/components/application_manager/include/application_manager/application_manager_impl.h +++ b/src/components/application_manager/include/application_manager/application_manager_impl.h @@ -1450,7 +1450,8 @@ class ApplicationManagerImpl ForbiddenApps forbidden_applications; // Lock for applications list - mutable std::shared_ptr applications_list_lock_ptr_; + mutable std::shared_ptr + applications_list_lock_ptr_; mutable std::shared_ptr apps_to_register_list_lock_ptr_; mutable sync_primitives::Lock subscribed_way_points_apps_lock_; diff --git a/src/components/application_manager/src/app_launch/app_launch_data_json.cc b/src/components/application_manager/src/app_launch/app_launch_data_json.cc index 99a6416a69..f7d24c26b1 100644 --- a/src/components/application_manager/src/app_launch/app_launch_data_json.cc +++ b/src/components/application_manager/src/app_launch/app_launch_data_json.cc @@ -43,8 +43,7 @@ CREATE_LOGGERPTR_GLOBAL(logger_, "AppLaunch") AppLaunchDataJson::AppLaunchDataJson(const AppLaunchSettings& settings, resumption::LastState& last_state) - : AppLaunchDataImpl(settings) - , last_state_(last_state) {} + : AppLaunchDataImpl(settings), last_state_(last_state) {} AppLaunchDataJson::~AppLaunchDataJson() {} diff --git a/src/components/application_manager/src/application_data_impl.cc b/src/components/application_manager/src/application_data_impl.cc index 356c75dc77..3cfbeb2602 100644 --- a/src/components/application_manager/src/application_data_impl.cc +++ b/src/components/application_manager/src/application_data_impl.cc @@ -184,7 +184,8 @@ DynamicApplicationDataImpl::DynamicApplicationDataImpl() , choice_set_map_() , choice_set_map_lock_ptr_(std::make_shared()) , performinteraction_choice_set_map_() - , performinteraction_choice_set_lock_ptr_(std::make_shared()) + , performinteraction_choice_set_lock_ptr_( + std::make_shared()) , is_perform_interaction_active_(false) , is_reset_global_properties_active_(false) , perform_interaction_mode_(-1) {} diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index 2c9f5d2f6f..3174e27607 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -143,7 +143,8 @@ ApplicationManagerImpl::ApplicationManagerImpl( const ApplicationManagerSettings& am_settings, const policy::PolicySettings& policy_settings) : settings_(am_settings) - , applications_list_lock_ptr_(std::make_shared()) + , applications_list_lock_ptr_( + std::make_shared()) , apps_to_register_list_lock_ptr_(std::make_shared()) , audio_pass_thru_active_(false) , audio_pass_thru_app_id_(0) diff --git a/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc b/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc index 505465d001..f1e5bf9735 100644 --- a/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc +++ b/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc @@ -39,8 +39,7 @@ namespace application_manager { namespace event_engine { using namespace sync_primitives; -EventDispatcherImpl::EventDispatcherImpl() - : observers_event_() {} +EventDispatcherImpl::EventDispatcherImpl() : observers_event_() {} EventDispatcherImpl::~EventDispatcherImpl() {} diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc index bfca801f74..64a16e5e6e 100644 --- a/src/components/policy/policy_regular/src/cache_manager.cc +++ b/src/components/policy/policy_regular/src/cache_manager.cc @@ -103,7 +103,7 @@ CacheManager::CacheManager() : CacheManagerInterface() , pt_(new policy_table::Table) , backup_(new SQLPTRepresentation()) - , update_required(false){ + , update_required(false) { LOG4CXX_AUTO_TRACE(logger_); backuper_ = new BackgroundBackuper(this); backup_thread_ = threads::CreateThread("Backup thread", backuper_); diff --git a/src/components/utils/src/lock_boost.cc b/src/components/utils/src/lock_boost.cc index a76bd02a78..c22ac467b5 100644 --- a/src/components/utils/src/lock_boost.cc +++ b/src/components/utils/src/lock_boost.cc @@ -51,11 +51,10 @@ Lock::~Lock() { void Lock::Acquire() { try { - mutex_.lock(); + mutex_.lock(); } catch (std::exception err) { LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " << - err.what()); + "Failed to acquire mutex " << &mutex_ << ": " << err.what()); NOTREACHED(); } AssertFreeAndMarkTaken(); @@ -103,18 +102,16 @@ RecursiveLock::~RecursiveLock() { void RecursiveLock::Acquire() { try { - mutex_.lock(); + mutex_.lock(); } catch (std::exception err) { LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " << - err.what()); + "Failed to acquire mutex " << &mutex_ << ": " << err.what()); NOTREACHED(); } AssertFreeAndMarkTaken(); } void RecursiveLock::Release() { - AssertTakenAndMarkFree(); mutex_.unlock(); } -- cgit v1.2.1 From adba01da38c865370d378eb2f0d237543a5b12d4 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 18 Jul 2018 15:08:12 -0400 Subject: rename cond var implementation file --- .../utils/src/conditional_variable_boost.cc | 144 +++++++++++++++++++++ .../utils/src/conditional_variable_posix.cc | 144 --------------------- 2 files changed, 144 insertions(+), 144 deletions(-) create mode 100644 src/components/utils/src/conditional_variable_boost.cc delete mode 100644 src/components/utils/src/conditional_variable_posix.cc diff --git a/src/components/utils/src/conditional_variable_boost.cc b/src/components/utils/src/conditional_variable_boost.cc new file mode 100644 index 0000000000..109e2bac9b --- /dev/null +++ b/src/components/utils/src/conditional_variable_boost.cc @@ -0,0 +1,144 @@ +/* + * Copyright (c) 2013, Ford Motor Company + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following + * disclaimer in the documentation and/or other materials provided with the + * distribution. + * + * Neither the name of the Ford Motor Company nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ +#include "utils/conditional_variable.h" + +#include +#include + +#include "utils/lock.h" +#include "utils/logger.h" + +namespace { +const long kNanosecondsPerSecond = 1000000000; +const long kMillisecondsPerSecond = 1000; +const long kNanosecondsPerMillisecond = 1000000; +} // namespace + +namespace sync_primitives { + +CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") + +ConditionalVariable::ConditionalVariable() {} + +ConditionalVariable::~ConditionalVariable() {} + +void ConditionalVariable::NotifyOne() { + cond_var_.notify_one(); +} + +void ConditionalVariable::Broadcast() { + cond_var_.notify_all(); +} + +bool ConditionalVariable::Wait(BaseLock& lock) { + // NOTE this grossness is due to boost mutex and recursive mutex not sharing a + // superclass + + lock.AssertTakenAndMarkFree(); + // try { + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + cond_var_.wait(test_lock->mutex_); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + cond_var_.wait(test_rec_lock->mutex_); + } else { + // unknown + lock.AssertFreeAndMarkTaken(); + + LOG4CXX_ERROR(logger_, "Unknown lock type!"); + return false; + } + // } catch (std::exception err) { + // lock.AssertFreeAndMarkTaken(); + // + // LOG4CXX_ERROR( + // logger_, + // "Failed to wait for conditional variable, exception:" << err.what()); + // return false; + // } + lock.AssertFreeAndMarkTaken(); + + return true; +} + +bool ConditionalVariable::Wait(AutoLock& auto_lock) { + BaseLock& lock = auto_lock.GetLock(); + return Wait(lock); +} + +ConditionalVariable::WaitStatus ConditionalVariable::WaitFor( + AutoLock& auto_lock, uint32_t milliseconds) { + BaseLock& lock = auto_lock.GetLock(); + + WaitStatus wait_status = kNoTimeout; + lock.AssertTakenAndMarkFree(); + try { + bool timeout = true; + + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + // cond_var_.wait(test_lock->mutex_); + timeout = cond_var_.timed_wait( + test_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + // cond_var_.wait(test_rec_lock->mutex_); + timeout = cond_var_.timed_wait( + test_rec_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); + } else { + // unknown + LOG4CXX_ERROR(logger_, "Unknown lock type!"); + } + + if (!timeout) { + wait_status = kTimeout; + } + } catch (boost::thread_interrupted inter) { + wait_status = kNoTimeout; + } + // catch (std::exception err) { + // LOG4CXX_ERROR( + // logger_, + // "Failed to timewait for conditional variable timedwait_status: " + // << err.what()); + // } + lock.AssertFreeAndMarkTaken(); + + return wait_status; +} + +} // namespace sync_primitives diff --git a/src/components/utils/src/conditional_variable_posix.cc b/src/components/utils/src/conditional_variable_posix.cc deleted file mode 100644 index 109e2bac9b..0000000000 --- a/src/components/utils/src/conditional_variable_posix.cc +++ /dev/null @@ -1,144 +0,0 @@ -/* - * Copyright (c) 2013, Ford Motor Company - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * Redistributions of source code must retain the above copyright notice, this - * list of conditions and the following disclaimer. - * - * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following - * disclaimer in the documentation and/or other materials provided with the - * distribution. - * - * Neither the name of the Ford Motor Company nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ -#include "utils/conditional_variable.h" - -#include -#include - -#include "utils/lock.h" -#include "utils/logger.h" - -namespace { -const long kNanosecondsPerSecond = 1000000000; -const long kMillisecondsPerSecond = 1000; -const long kNanosecondsPerMillisecond = 1000000; -} // namespace - -namespace sync_primitives { - -CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") - -ConditionalVariable::ConditionalVariable() {} - -ConditionalVariable::~ConditionalVariable() {} - -void ConditionalVariable::NotifyOne() { - cond_var_.notify_one(); -} - -void ConditionalVariable::Broadcast() { - cond_var_.notify_all(); -} - -bool ConditionalVariable::Wait(BaseLock& lock) { - // NOTE this grossness is due to boost mutex and recursive mutex not sharing a - // superclass - - lock.AssertTakenAndMarkFree(); - // try { - // What kind of lock are we ? - if (Lock* test_lock = dynamic_cast(&lock)) { - // Regular lock - cond_var_.wait(test_lock->mutex_); - } else if (RecursiveLock* test_rec_lock = - dynamic_cast(&lock)) { - // Recursive lock - cond_var_.wait(test_rec_lock->mutex_); - } else { - // unknown - lock.AssertFreeAndMarkTaken(); - - LOG4CXX_ERROR(logger_, "Unknown lock type!"); - return false; - } - // } catch (std::exception err) { - // lock.AssertFreeAndMarkTaken(); - // - // LOG4CXX_ERROR( - // logger_, - // "Failed to wait for conditional variable, exception:" << err.what()); - // return false; - // } - lock.AssertFreeAndMarkTaken(); - - return true; -} - -bool ConditionalVariable::Wait(AutoLock& auto_lock) { - BaseLock& lock = auto_lock.GetLock(); - return Wait(lock); -} - -ConditionalVariable::WaitStatus ConditionalVariable::WaitFor( - AutoLock& auto_lock, uint32_t milliseconds) { - BaseLock& lock = auto_lock.GetLock(); - - WaitStatus wait_status = kNoTimeout; - lock.AssertTakenAndMarkFree(); - try { - bool timeout = true; - - // What kind of lock are we ? - if (Lock* test_lock = dynamic_cast(&lock)) { - // Regular lock - // cond_var_.wait(test_lock->mutex_); - timeout = cond_var_.timed_wait( - test_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); - } else if (RecursiveLock* test_rec_lock = - dynamic_cast(&lock)) { - // Recursive lock - // cond_var_.wait(test_rec_lock->mutex_); - timeout = cond_var_.timed_wait( - test_rec_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); - } else { - // unknown - LOG4CXX_ERROR(logger_, "Unknown lock type!"); - } - - if (!timeout) { - wait_status = kTimeout; - } - } catch (boost::thread_interrupted inter) { - wait_status = kNoTimeout; - } - // catch (std::exception err) { - // LOG4CXX_ERROR( - // logger_, - // "Failed to timewait for conditional variable timedwait_status: " - // << err.what()); - // } - lock.AssertFreeAndMarkTaken(); - - return wait_status; -} - -} // namespace sync_primitives -- cgit v1.2.1 From c96b488f0a5c6f114016ab9c013a347b5257d7d9 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 18 Jul 2018 15:14:34 -0400 Subject: maybe final cleanup --- .../utils/src/conditional_variable_boost.cc | 51 +++++++++++----------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/src/components/utils/src/conditional_variable_boost.cc b/src/components/utils/src/conditional_variable_boost.cc index 109e2bac9b..f1c23c13a4 100644 --- a/src/components/utils/src/conditional_variable_boost.cc +++ b/src/components/utils/src/conditional_variable_boost.cc @@ -64,30 +64,30 @@ bool ConditionalVariable::Wait(BaseLock& lock) { // superclass lock.AssertTakenAndMarkFree(); - // try { - // What kind of lock are we ? - if (Lock* test_lock = dynamic_cast(&lock)) { - // Regular lock - cond_var_.wait(test_lock->mutex_); - } else if (RecursiveLock* test_rec_lock = - dynamic_cast(&lock)) { - // Recursive lock - cond_var_.wait(test_rec_lock->mutex_); - } else { - // unknown + try { + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + cond_var_.wait(test_lock->mutex_); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + cond_var_.wait(test_rec_lock->mutex_); + } else { + // unknown + lock.AssertFreeAndMarkTaken(); + + LOG4CXX_ERROR(logger_, "Unknown lock type!"); + return false; + } + } catch (std::exception err) { lock.AssertFreeAndMarkTaken(); - LOG4CXX_ERROR(logger_, "Unknown lock type!"); + LOG4CXX_ERROR( + logger_, + "Failed to wait for conditional variable, exception:" << err.what()); return false; } - // } catch (std::exception err) { - // lock.AssertFreeAndMarkTaken(); - // - // LOG4CXX_ERROR( - // logger_, - // "Failed to wait for conditional variable, exception:" << err.what()); - // return false; - // } lock.AssertFreeAndMarkTaken(); return true; @@ -129,13 +129,12 @@ ConditionalVariable::WaitStatus ConditionalVariable::WaitFor( } } catch (boost::thread_interrupted inter) { wait_status = kNoTimeout; + } catch (std::exception err) { + LOG4CXX_ERROR( + logger_, + "Failed to timewait for conditional variable timedwait_status: " + << err.what()); } - // catch (std::exception err) { - // LOG4CXX_ERROR( - // logger_, - // "Failed to timewait for conditional variable timedwait_status: " - // << err.what()); - // } lock.AssertFreeAndMarkTaken(); return wait_status; -- cgit v1.2.1 From f58f40ec9630827124dbfffa8083f0cf2536f592 Mon Sep 17 00:00:00 2001 From: conlain-k Date: Fri, 20 Jul 2018 09:52:33 -0400 Subject: add boost thread component requirement to cmake --- src/3rd_party/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rd_party/CMakeLists.txt b/src/3rd_party/CMakeLists.txt index b72d86f902..6b21ce036c 100644 --- a/src/3rd_party/CMakeLists.txt +++ b/src/3rd_party/CMakeLists.txt @@ -210,7 +210,7 @@ else() endif() if (HMIADAPTER STREQUAL "messagebroker") - find_package(Boost 1.66.0 COMPONENTS system) + find_package(Boost 1.66.0 COMPONENTS system thread) set(BOOST_LIB_SOURCE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/boost_src) set(BOOST_LIBS_DIRECTORY ${3RD_PARTY_INSTALL_PREFIX}/lib) SET_PROPERTY(GLOBAL PROPERTY GLOBAL_BOOST_LIBS ${BOOST_LIBS_DIRECTORY}) -- cgit v1.2.1 From 750dad21868304cbe06b2039e5a8f0f561caa924 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Fri, 20 Jul 2018 15:51:03 -0400 Subject: Add multithread and move tests into main build --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 73bbfbeb2a..f56f77902d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,8 +24,7 @@ script: - ./tools/infrastructure/check_style.sh - mkdir build && cd build - cmake ../ -DBUILD_TESTS=ON - - make install && sudo ldconfig -after_success: + - make install-3rd_party && make -j `nproc` install && sudo ldconfig - make test env: global: -- cgit v1.2.1 From 4fdca461ea2678385bd30743473dcb758f4da80b Mon Sep 17 00:00:00 2001 From: jacobkeeler Date: Mon, 23 Jul 2018 16:26:10 -0400 Subject: Fix EXTENDED_MEDIA_MODE build Also fixed a typo --- .../media_manager/src/audio/a2dp_source_player_adapter.cc | 12 ++++++------ src/components/media_manager/src/media_manager_impl.cc | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/media_manager/src/audio/a2dp_source_player_adapter.cc b/src/components/media_manager/src/audio/a2dp_source_player_adapter.cc index 5e9d6ab0ba..91e3c5465d 100644 --- a/src/components/media_manager/src/audio/a2dp_source_player_adapter.cc +++ b/src/components/media_manager/src/audio/a2dp_source_player_adapter.cc @@ -91,19 +91,19 @@ void A2DPSourcePlayerAdapter::StartActivity(int32_t application_key) { if (application_key != current_application_) { current_application_ = application_key; - uint32_t device_id = 0; + transport_manager::DeviceHandle device_id = 0; session_observer_.GetDataOnSessionKey(application_key, 0, NULL, &device_id); - std::string mac_adddress; - session_observer_.GetDataOnDeviceID(device_id, NULL, NULL, &mac_adddress); + std::string mac_address; + session_observer_.GetDataOnDeviceID(device_id, NULL, NULL, &mac_address); - // TODO(PK): Convert mac_adddress to the + // TODO(PK): Convert mac_address to the // following format : "bluez_source.XX_XX_XX_XX_XX_XX" if needed // before passing to the A2DPSourcePlayerThread constructor A2DPSourcePlayerThread* delegate = - new A2DPSourcePlayerAdapter::A2DPSourcePlayerThread(mac_adddress); + new A2DPSourcePlayerAdapter::A2DPSourcePlayerThread(mac_address); threads::Thread* new_activity = - threads::CreateThread(mac_adddress.c_str(), delegate); + threads::CreateThread(mac_address.c_str(), delegate); sources_[application_key] = Pair(new_activity, delegate); new_activity->start(); } diff --git a/src/components/media_manager/src/media_manager_impl.cc b/src/components/media_manager/src/media_manager_impl.cc index 15b58aee1f..ec88e910d3 100644 --- a/src/components/media_manager/src/media_manager_impl.cc +++ b/src/components/media_manager/src/media_manager_impl.cc @@ -116,7 +116,7 @@ void MediaManagerImpl::Init() { #if defined(EXTENDED_MEDIA_MODE) LOG4CXX_INFO(logger_, "Called Init with default configuration."); - from_mic_recorder_ = std::make_shared(); + from_mic_recorder_ = new FromMicRecorderAdapter(); #endif if ("socket" == settings().video_server_type()) { -- cgit v1.2.1 From 5c4d3aa87ab16f894132038700e819595df8e9fc Mon Sep 17 00:00:00 2001 From: jacobkeeler Date: Mon, 23 Jul 2018 16:52:38 -0400 Subject: Fix style --- .../commands/mobile/dial_number_request_test.cc | 65 +++++++++++----------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/dial_number_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/dial_number_request_test.cc index 8e95f43752..5d6c6dc89e 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/dial_number_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/dial_number_request_test.cc @@ -139,48 +139,45 @@ TEST_F(DialNumberRequestTest, Run_SUCCESS) { } TEST_F(DialNumberRequestTest, OnEvent_UnknownEvent_UNSUCCESS) { - MessageSharedPtr command_msg(CreateMessage(smart_objects::SmartType_Map)); - (*command_msg)[am::strings::params][am::strings::connection_key] = - kConnectionKey; + MessageSharedPtr command_msg(CreateMessage(smart_objects::SmartType_Map)); + (*command_msg)[am::strings::params][am::strings::connection_key] = + kConnectionKey; - DialNumberRequestPtr - command(CreateCommand(command_msg)); + DialNumberRequestPtr command(CreateCommand(command_msg)); - MockAppPtr app(CreateMockApp()); - EXPECT_CALL(app_mngr_, application(kConnectionKey)).WillOnce(Return(app)); + MockAppPtr app(CreateMockApp()); + EXPECT_CALL(app_mngr_, application(kConnectionKey)).WillOnce(Return(app)); - Event event(hmi_apis::FunctionID::INVALID_ENUM); - EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)).Times(0); + Event event(hmi_apis::FunctionID::INVALID_ENUM); + EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)).Times(0); command->on_event(event); } TEST_F(DialNumberRequestTest, OnEvent_SUCCESS) { - MessageSharedPtr event_msg(CreateMessage(smart_objects::SmartType_Map)); - (*event_msg)[am::strings::params][am::hmi_response::code] = - mobile_apis::Result::SUCCESS; - (*event_msg)[am::strings::params][am::strings::info] = "test_info"; - - Event event(hmi_apis::FunctionID::BasicCommunication_DialNumber); - event.set_smart_object(*event_msg); - - MockAppPtr app(CreateMockApp()); - EXPECT_CALL(app_mngr_, application(kConnectionKey)) - .WillRepeatedly(Return(app)); - ON_CALL(app_mngr_, - GetRPCService()).WillByDefault(ReturnRef(mock_rpc_service_)); - EXPECT_CALL( - mock_rpc_service_, - ManageMobileCommand(MobileResultCodeIs(mobile_apis::Result::SUCCESS), - _)); - - MessageSharedPtr command_msg(CreateMessage(smart_objects::SmartType_Map)); - (*command_msg)[am::strings::params][am::strings::connection_key] = - kConnectionKey; - - DialNumberRequestPtr - command(CreateCommand(command_msg)); - command->on_event(event); + MessageSharedPtr event_msg(CreateMessage(smart_objects::SmartType_Map)); + (*event_msg)[am::strings::params][am::hmi_response::code] = + mobile_apis::Result::SUCCESS; + (*event_msg)[am::strings::params][am::strings::info] = "test_info"; + + Event event(hmi_apis::FunctionID::BasicCommunication_DialNumber); + event.set_smart_object(*event_msg); + + MockAppPtr app(CreateMockApp()); + EXPECT_CALL(app_mngr_, application(kConnectionKey)) + .WillRepeatedly(Return(app)); + ON_CALL(app_mngr_, GetRPCService()) + .WillByDefault(ReturnRef(mock_rpc_service_)); + EXPECT_CALL( + mock_rpc_service_, + ManageMobileCommand(MobileResultCodeIs(mobile_apis::Result::SUCCESS), _)); + + MessageSharedPtr command_msg(CreateMessage(smart_objects::SmartType_Map)); + (*command_msg)[am::strings::params][am::strings::connection_key] = + kConnectionKey; + + DialNumberRequestPtr command(CreateCommand(command_msg)); + command->on_event(event); } } // namespace dial_number_request -- cgit v1.2.1 From 0849c2a539587ae1b020599c0baed5e6403ce3b0 Mon Sep 17 00:00:00 2001 From: conlain-k Date: Mon, 23 Jul 2018 17:13:50 -0400 Subject: Only run tests on successful build --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f56f77902d..7f7df359b4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,8 +24,7 @@ script: - ./tools/infrastructure/check_style.sh - mkdir build && cd build - cmake ../ -DBUILD_TESTS=ON - - make install-3rd_party && make -j `nproc` install && sudo ldconfig - - make test + - make install-3rd_party && make -j `nproc` install && sudo ldconfig && make test env: global: - LC_CTYPE=en_US.UTF-8 -- cgit v1.2.1 From 01a61648226fd0df15188bc4bdd60d1f4a6f0709 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Tue, 24 Jul 2018 09:56:37 -0400 Subject: Make sure boost target is always there, utils needs it now --- src/3rd_party/CMakeLists.txt | 54 ++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/3rd_party/CMakeLists.txt b/src/3rd_party/CMakeLists.txt index 6b21ce036c..6c70e8988b 100644 --- a/src/3rd_party/CMakeLists.txt +++ b/src/3rd_party/CMakeLists.txt @@ -209,34 +209,34 @@ else() ) endif() -if (HMIADAPTER STREQUAL "messagebroker") - find_package(Boost 1.66.0 COMPONENTS system thread) - set(BOOST_LIB_SOURCE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/boost_src) - set(BOOST_LIBS_DIRECTORY ${3RD_PARTY_INSTALL_PREFIX}/lib) - SET_PROPERTY(GLOBAL PROPERTY GLOBAL_BOOST_LIBS ${BOOST_LIBS_DIRECTORY}) - set(BOOST_INCLUDE_DIRECTORY ${3RD_PARTY_INSTALL_PREFIX}/include ) - if (NOT ${Boost_FOUND}) - message(STATUS "Did not find boost. Downloading and installing boost 1.66") - set(BOOST_INSTALL_COMMAND ./b2 install) - if (${3RD_PARTY_INSTALL_PREFIX} MATCHES "/usr/local") - set(BOOST_INSTALL_COMMAND sudo ./b2 install) - endif() - include(ExternalProject) - ExternalProject_Add( - Boost - URL https://dl.bintray.com/boostorg/release/1.66.0/source/boost_1_66_0.tar.gz - DOWNLOAD_DIR ${BOOST_LIB_SOURCE_DIRECTORY} - SOURCE_DIR ${BOOST_LIB_SOURCE_DIRECTORY} - CONFIGURE_COMMAND ./bootstrap.sh --with-libraries=system,thread --prefix=${3RD_PARTY_INSTALL_PREFIX} - BUILD_COMMAND ./b2 - INSTALL_COMMAND ${BOOST_INSTALL_COMMAND} --with-system --with-thread --prefix=${3RD_PARTY_INSTALL_PREFIX} > boost_install.log - INSTALL_DIR ${3RD_PARTY_INSTALL_PREFIX} - BUILD_IN_SOURCE true - ) - - set(BOOST_INCLUDE_DIR ${BOOST_ROOT_DIR}/Boost-prefix/src/Boost) - set(BOOST_LIB_DIR ${BOOST_ROOT_DIR}/Boost-prefix/src/Boost/stage/lib/) +find_package(Boost 1.66.0 COMPONENTS system thread) +set(BOOST_LIB_SOURCE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/boost_src) +set(BOOST_LIBS_DIRECTORY ${3RD_PARTY_INSTALL_PREFIX}/lib) +SET_PROPERTY(GLOBAL PROPERTY GLOBAL_BOOST_LIBS ${BOOST_LIBS_DIRECTORY}) +set(BOOST_INCLUDE_DIRECTORY ${3RD_PARTY_INSTALL_PREFIX}/include ) +if (NOT ${Boost_FOUND}) + message(STATUS "Did not find boost. Downloading and installing boost 1.66") + set(BOOST_INSTALL_COMMAND ./b2 install) + if (${3RD_PARTY_INSTALL_PREFIX} MATCHES "/usr/local") + set(BOOST_INSTALL_COMMAND sudo ./b2 install) endif() + include(ExternalProject) + ExternalProject_Add( + Boost + URL https://dl.bintray.com/boostorg/release/1.66.0/source/boost_1_66_0.tar.gz + DOWNLOAD_DIR ${BOOST_LIB_SOURCE_DIRECTORY} + SOURCE_DIR ${BOOST_LIB_SOURCE_DIRECTORY} + CONFIGURE_COMMAND ./bootstrap.sh --with-libraries=system,thread --prefix=${3RD_PARTY_INSTALL_PREFIX} + BUILD_COMMAND ./b2 + INSTALL_COMMAND ${BOOST_INSTALL_COMMAND} --with-system --with-thread --prefix=${3RD_PARTY_INSTALL_PREFIX} > boost_install.log + INSTALL_DIR ${3RD_PARTY_INSTALL_PREFIX} + BUILD_IN_SOURCE true + ) + + set(BOOST_INCLUDE_DIR ${BOOST_ROOT_DIR}/Boost-prefix/src/Boost) + set(BOOST_LIB_DIR ${BOOST_ROOT_DIR}/Boost-prefix/src/Boost/stage/lib/) +else() +add_custom_target(Boost) # empty target, Boost is already installed endif() add_custom_target(install-3rd_party -- cgit v1.2.1 From f8a549cdf0e1b52ef66f1ca703a1d07d4c99f645 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Tue, 24 Jul 2018 14:31:17 -0400 Subject: Remove exception-handling code, extra constructors Note that locks will now crash on error via an uncaught Boost exception This is because SDL prohibits exceptions, but Boost uses them internally --- .../src/application_manager_impl.cc | 1 - .../connection_handler/src/connection.cc | 1 - .../utils/src/conditional_variable_boost.cc | 87 +++++++++------------- src/components/utils/src/lock_boost.cc | 16 +--- 4 files changed, 36 insertions(+), 69 deletions(-) diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index a0be3d1939..9b936bdf41 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -166,7 +166,6 @@ ApplicationManagerImpl::ApplicationManagerImpl( , resume_ctrl_(new resumption::ResumeCtrlImpl(*this)) , navi_close_app_timeout_(am_settings.stop_streaming_timeout()) , navi_end_stream_timeout_(am_settings.stop_streaming_timeout()) - , stopping_application_mng_lock_() , state_ctrl_(*this) , application_list_update_timer_( "AM ListUpdater", diff --git a/src/components/connection_handler/src/connection.cc b/src/components/connection_handler/src/connection.cc index 08b37b9026..5a35919c6c 100644 --- a/src/components/connection_handler/src/connection.cc +++ b/src/components/connection_handler/src/connection.cc @@ -81,7 +81,6 @@ Connection::Connection(ConnectionHandle connection_handle, : connection_handler_(connection_handler) , connection_handle_(connection_handle) , connection_device_handle_(connection_device_handle) - , session_map_lock_() , primary_connection_handle_(0) , heartbeat_timeout_(heartbeat_timeout) { LOG4CXX_AUTO_TRACE(logger_); diff --git a/src/components/utils/src/conditional_variable_boost.cc b/src/components/utils/src/conditional_variable_boost.cc index f1c23c13a4..c07b8284ea 100644 --- a/src/components/utils/src/conditional_variable_boost.cc +++ b/src/components/utils/src/conditional_variable_boost.cc @@ -64,29 +64,18 @@ bool ConditionalVariable::Wait(BaseLock& lock) { // superclass lock.AssertTakenAndMarkFree(); - try { - // What kind of lock are we ? - if (Lock* test_lock = dynamic_cast(&lock)) { - // Regular lock - cond_var_.wait(test_lock->mutex_); - } else if (RecursiveLock* test_rec_lock = - dynamic_cast(&lock)) { - // Recursive lock - cond_var_.wait(test_rec_lock->mutex_); - } else { - // unknown - lock.AssertFreeAndMarkTaken(); - - LOG4CXX_ERROR(logger_, "Unknown lock type!"); - return false; - } - } catch (std::exception err) { - lock.AssertFreeAndMarkTaken(); - - LOG4CXX_ERROR( - logger_, - "Failed to wait for conditional variable, exception:" << err.what()); - return false; + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + cond_var_.wait(test_lock->mutex_); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + cond_var_.wait(test_rec_lock->mutex_); + } else { + // unknown, give up the lock + LOG4CXX_ERROR(logger_, "Unknown lock type!"); + NOTREACHED(); } lock.AssertFreeAndMarkTaken(); @@ -104,36 +93,28 @@ ConditionalVariable::WaitStatus ConditionalVariable::WaitFor( WaitStatus wait_status = kNoTimeout; lock.AssertTakenAndMarkFree(); - try { - bool timeout = true; - - // What kind of lock are we ? - if (Lock* test_lock = dynamic_cast(&lock)) { - // Regular lock - // cond_var_.wait(test_lock->mutex_); - timeout = cond_var_.timed_wait( - test_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); - } else if (RecursiveLock* test_rec_lock = - dynamic_cast(&lock)) { - // Recursive lock - // cond_var_.wait(test_rec_lock->mutex_); - timeout = cond_var_.timed_wait( - test_rec_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); - } else { - // unknown - LOG4CXX_ERROR(logger_, "Unknown lock type!"); - } - - if (!timeout) { - wait_status = kTimeout; - } - } catch (boost::thread_interrupted inter) { - wait_status = kNoTimeout; - } catch (std::exception err) { - LOG4CXX_ERROR( - logger_, - "Failed to timewait for conditional variable timedwait_status: " - << err.what()); + bool timeout = true; + + // What kind of lock are we ? + if (Lock* test_lock = dynamic_cast(&lock)) { + // Regular lock + // cond_var_.wait(test_lock->mutex_); + timeout = cond_var_.timed_wait( + test_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); + } else if (RecursiveLock* test_rec_lock = + dynamic_cast(&lock)) { + // Recursive lock + // cond_var_.wait(test_rec_lock->mutex_); + timeout = cond_var_.timed_wait( + test_rec_lock->mutex_, boost::posix_time::milliseconds(milliseconds)); + } else { + // this is an unknown lock, we have an issue + LOG4CXX_ERROR(logger_, "Unknown lock type!"); + NOTREACHED(); + } + + if (!timeout) { + wait_status = kTimeout; } lock.AssertFreeAndMarkTaken(); diff --git a/src/components/utils/src/lock_boost.cc b/src/components/utils/src/lock_boost.cc index c22ac467b5..2299bcf77d 100644 --- a/src/components/utils/src/lock_boost.cc +++ b/src/components/utils/src/lock_boost.cc @@ -50,13 +50,7 @@ Lock::~Lock() { } void Lock::Acquire() { - try { - mutex_.lock(); - } catch (std::exception err) { - LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " << err.what()); - NOTREACHED(); - } + mutex_.lock(); AssertFreeAndMarkTaken(); } @@ -101,13 +95,7 @@ RecursiveLock::~RecursiveLock() { } void RecursiveLock::Acquire() { - try { - mutex_.lock(); - } catch (std::exception err) { - LOG4CXX_FATAL(logger_, - "Failed to acquire mutex " << &mutex_ << ": " << err.what()); - NOTREACHED(); - } + mutex_.lock(); AssertFreeAndMarkTaken(); } -- cgit v1.2.1 From 3463f9a85b5fe5a30ee1edf540f077855170e0bf Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Thu, 26 Jul 2018 10:48:50 -0400 Subject: make videostreamingstate optional --- src/components/interfaces/MOBILE_API.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/interfaces/MOBILE_API.xml b/src/components/interfaces/MOBILE_API.xml index 97bc33ceae..09fe843ef3 100644 --- a/src/components/interfaces/MOBILE_API.xml +++ b/src/components/interfaces/MOBILE_API.xml @@ -6115,7 +6115,7 @@ See SystemContext - + See VideoStreamingState. If it is NOT_STREAMABLE, the app must stop streaming video to SDL. -- cgit v1.2.1 From 62d2e5847389eaf6b949e2e724ddf98cab486fc1 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Mon, 18 Jun 2018 16:47:11 +0200 Subject: removed image check in RPC --- .../src/commands/mobile/add_command_request.cc | 13 ---------- .../create_interaction_choice_set_request.cc | 28 ---------------------- .../commands/mobile/perform_interaction_request.cc | 10 -------- .../src/commands/mobile/send_location_request.cc | 13 ---------- .../mobile/set_global_properties_request.cc | 26 -------------------- .../commands/mobile/show_constant_tbt_request.cc | 20 ---------------- .../src/commands/mobile/show_request.cc | 26 -------------------- .../commands/mobile/update_turn_list_request.cc | 16 ------------- 8 files changed, 152 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc index fc836aae5f..ab9baea7ef 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc @@ -90,19 +90,6 @@ void AddCommandRequest::Run() { return; } - if ((*message_)[strings::msg_params].keyExists(strings::cmd_icon)) { - mobile_apis::Result::eType verification_result = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::cmd_icon], - app, - application_manager_); - - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR( - logger_, "MessageHelper::VerifyImage return " << verification_result); - SendResponse(false, verification_result); - return; - } - } if (!((*message_)[strings::msg_params].keyExists(strings::cmd_id))) { LOG4CXX_ERROR(logger_, "INVALID_DATA"); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc index d2d39dcd77..89ae0c1aac 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc @@ -78,34 +78,6 @@ void CreateInteractionChoiceSetRequest::Run() { SendResponse(false, mobile_apis::Result::APPLICATION_NOT_REGISTERED); return; } - for (uint32_t i = 0; - i < (*message_)[strings::msg_params][strings::choice_set].length(); - ++i) { - Result::eType verification_result_image = Result::SUCCESS; - Result::eType verification_result_secondary_image = Result::SUCCESS; - if ((*message_)[strings::msg_params][strings::choice_set][i].keyExists( - strings::image)) { - verification_result_image = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::choice_set][i] - [strings::image], - app, - application_manager_); - } - if ((*message_)[strings::msg_params][strings::choice_set][i].keyExists( - strings::secondary_image)) { - verification_result_secondary_image = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::choice_set][i] - [strings::secondary_image], - app, - application_manager_); - } - if (verification_result_image == Result::INVALID_DATA || - verification_result_secondary_image == Result::INVALID_DATA) { - LOG4CXX_ERROR(logger_, "Image verification failed."); - SendResponse(false, Result::INVALID_DATA); - return; - } - } choice_set_id_ = (*message_)[strings::msg_params][strings::interaction_choice_set_id] diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc index 8710f3c3d6..fb3516c24c 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc @@ -164,16 +164,6 @@ void PerformInteractionRequest::Run() { return; } - if (msg_params.keyExists(strings::vr_help)) { - if (mobile_apis::Result::SUCCESS != - MessageHelper::VerifyImageVrHelpItems( - msg_params[strings::vr_help], app, application_manager_)) { - LOG4CXX_ERROR(logger_, - "Verification of " << strings::vr_help << " failed."); - SendResponse(false, mobile_apis::Result::INVALID_DATA); - return; - } - } if (IsWhiteSpaceExist()) { LOG4CXX_ERROR(logger_, diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc index ab8de5923a..e9e11b3f24 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc @@ -117,19 +117,6 @@ void SendLocationRequest::Run() { return; } - if (msg_params.keyExists(strings::location_image)) { - mobile_apis::Result::eType verification_result = - mobile_apis::Result::SUCCESS; - verification_result = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::location_image], - app, - application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR(logger_, "VerifyImage INVALID_DATA!"); - SendResponse(false, verification_result); - return; - } - } SmartObject request_msg_params = SmartObject(smart_objects::SmartType_Map); request_msg_params = msg_params; diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc index b4b4e84f49..0e8c4fcbfc 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc @@ -88,32 +88,6 @@ void SetGlobalPropertiesRequest::Run() { return; } - mobile_apis::Result::eType verification_result = mobile_apis::Result::SUCCESS; - - if ((*message_)[strings::msg_params].keyExists(strings::menu_icon)) { - verification_result = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::menu_icon], - app, - application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR( - logger_, "MessageHelper::VerifyImage return " << verification_result); - SendResponse(false, verification_result); - return; - } - } - // Check for image file(s) in vrHelpItem - if ((*message_)[strings::msg_params].keyExists(strings::vr_help)) { - if (mobile_apis::Result::SUCCESS != - MessageHelper::VerifyImageVrHelpItems( - (*message_)[strings::msg_params][strings::vr_help], - app, - application_manager_)) { - LOG4CXX_ERROR(logger_, "MessageHelper::VerifyImage return INVALID_DATA!"); - SendResponse(false, mobile_apis::Result::INVALID_DATA); - return; - } - } if (IsWhiteSpaceExist()) { LOG4CXX_ERROR(logger_, "White spaces found"); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc index c2eb067e18..1e5767258d 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc @@ -101,26 +101,6 @@ void ShowConstantTBTRequest::Run() { return; } - mobile_apis::Result::eType verification_result = mobile_apis::Result::SUCCESS; - if (msg_params.keyExists(strings::turn_icon)) { - verification_result = MessageHelper::VerifyImage( - msg_params[strings::turn_icon], app, application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR(logger_, "VerifyImage INVALID_DATA!"); - SendResponse(false, verification_result); - return; - } - } - - if (msg_params.keyExists(strings::next_turn_icon)) { - verification_result = MessageHelper::VerifyImage( - msg_params[strings::next_turn_icon], app, application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR(logger_, "VerifyImage INVALID_DATA!"); - SendResponse(false, verification_result); - return; - } - } msg_params[strings::app_id] = app->app_id(); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc index 0fdf6d83cf..25b96d6aab 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc @@ -135,32 +135,6 @@ void ShowRequest::Run() { return; } - mobile_apis::Result::eType verification_result = mobile_apis::Result::SUCCESS; - if (((*message_)[strings::msg_params].keyExists(strings::graphic)) && - ((*message_)[strings::msg_params][strings::graphic][strings::value] - .asString()).length()) { - verification_result = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::graphic], - app, - application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR(logger_, "Image verification failed."); - SendResponse(false, verification_result); - return; - } - } - - if ((*message_)[strings::msg_params].keyExists(strings::secondary_graphic)) { - verification_result = MessageHelper::VerifyImage( - (*message_)[strings::msg_params][strings::secondary_graphic], - app, - application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { - LOG4CXX_ERROR(logger_, "Image verification failed."); - SendResponse(false, verification_result); - return; - } - } smart_objects::SmartObject msg_params = smart_objects::SmartObject(smart_objects::SmartType_Map); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc index 69379135b5..f28656514f 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc @@ -95,22 +95,6 @@ void UpdateTurnListRequest::Run() { return; } - if ((*message_)[strings::msg_params].keyExists(strings::turn_list)) { - smart_objects::SmartObject& turn_list_array = - ((*message_)[strings::msg_params][strings::turn_list]); - for (uint32_t i = 0; i < turn_list_array.length(); ++i) { - if ((turn_list_array[i].keyExists(strings::turn_icon)) && - (mobile_apis::Result::SUCCESS != - MessageHelper::VerifyImage(turn_list_array[i][strings::turn_icon], - app, - application_manager_))) { - LOG4CXX_ERROR(logger_, - "MessageHelper::VerifyImage return INVALID_DATA"); - SendResponse(false, mobile_apis::Result::INVALID_DATA); - return; - } - } - } smart_objects::SmartObject msg_params = smart_objects::SmartObject(smart_objects::SmartType_Map); -- cgit v1.2.1 From 25e00ebf5cb3dfa127a1f56a6bdead72ed0a10a9 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Mon, 18 Jun 2018 16:47:40 +0200 Subject: UT fix after removing image check --- .../commands/mobile/add_command_request_test.cc | 20 ++++++ .../commands/mobile/send_location_request_test.cc | 14 +---- .../commands/mobile/set_global_properties_test.cc | 73 +++++++++++++++++++++- .../test/commands/mobile/show_test.cc | 10 --- .../mobile/update_turn_list_request_test.cc | 7 --- 5 files changed, 93 insertions(+), 31 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc index ea1a0fafed..fedb4966f5 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc @@ -263,6 +263,26 @@ TEST_F(AddCommandRequestTest, Run_ImageVerificationFailed_EXPECT_INVALID_DATA) { request_ptr->Run(); } +TEST_F(AddCommandRequestTest, Run_ImageVerificationFailed_EXPECT_WARNINGS) { + CreateBasicParamsUIRequest(); + SmartObject& msg_params = (*msg_)[strings::msg_params]; + SmartObject& image = msg_params[cmd_icon]; + EXPECT_CALL(mock_message_helper_, VerifyImage(image, _, _)) + .WillOnce(Return(mobile_apis::Result::WARNINGS)); + + am::CommandsMap commands_map; + EXPECT_CALL(*mock_app_, commands_map()) + .WillRepeatedly(Return(DataAccessor( + commands_map, lock_ptr_))); + EXPECT_CALL( + mock_rpc_service_, + ManageHMICommand(HMIResultCodeIs(hmi_apis::FunctionID::UI_AddCommand))) + .WillOnce(Return(true)); + utils::SharedPtr request_ptr = + CreateCommand(msg_); + request_ptr->Run(); +} + TEST_F(AddCommandRequestTest, Run_MenuNameHasSyntaxError_EXPECT_INVALID_DATA) { CreateBasicParamsUIRequest(); SmartObject& msg_params = (*msg_)[strings::msg_params]; diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc index bf75e651c6..4a4fa88f33 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc @@ -306,27 +306,17 @@ TEST_F(SendLocationRequestTest, Run_LocationImageValid_Success) { SmartObject(smart_objects::SmartType_Map); (*message_)[strings::msg_params][strings::location_image][strings::value] = "1"; - EXPECT_CALL( - mock_message_helper_, - VerifyImage( - (*message_)[strings::msg_params][strings::location_image], _, _)) - .WillOnce(Return(mobile_apis::Result::SUCCESS)); FinishSetup(); command_->Run(); } -TEST_F(SendLocationRequestTest, Run_LocationImageInvalid_Cancelled) { +TEST_F(SendLocationRequestTest, Run_LocationImageInvalid_Warnings) { InitialSetup(message_); (*message_)[strings::msg_params][strings::location_image] = SmartObject(smart_objects::SmartType_Map); (*message_)[strings::msg_params][strings::location_image][strings::value] = "1"; - EXPECT_CALL( - mock_message_helper_, - VerifyImage( - (*message_)[strings::msg_params][strings::location_image], _, _)) - .WillOnce(Return(mobile_apis::Result::ABORTED)); - FinishSetupCancelled(mobile_apis::Result::ABORTED); + FinishSetup(); command_->Run(); } diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc index ff85180ad6..4ee5af8279 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc @@ -643,7 +643,33 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_NoVR_SUCCESS) { GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - std::shared_ptr command( + SharedPtr command( + CreateCommand(msg)); + + command->Run(); +} + +TEST_F(SetGlobalPropertiesRequestTest, Run_VRCouldNotGenerate_INVALID_DATA) { + MessageSharedPtr msg = CreateMsgParams(); + SmartObject keyboard_properties(smart_objects::SmartType_Map); + (*msg)[am::strings::msg_params][am::hmi_request::keyboard_properties] = + keyboard_properties; + SmartObject menu_title("Menu_Title"); + (*msg)[am::strings::msg_params][am::hmi_request::menu_title] = menu_title; + + EXPECT_CALL(app_mngr_, application(kConnectionKey)) + .WillOnce(Return(mock_app_)); + EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(kConnectionKey)); + SmartObject* vr_help_title = NULL; + CommandsMap commands_map; + SmartObject empty_msg(smart_objects::SmartType_Map); + commands_map.insert(std::pair(1u, &empty_msg)); + DataAccessor accessor(commands_map, lock_ptr_); + EXPECT_CALL(*mock_app_, commands_map()).WillOnce(Return(accessor)); + EXPECT_CALL(*mock_app_, vr_help_title()).WillOnce(Return(vr_help_title)); + EXPECT_CALL(*mock_app_, set_menu_title(_)).Times(0); + + SharedPtr command( CreateCommand(msg)); command->Run(); @@ -700,7 +726,50 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_NoVRNoDataDefaultCreated_SUCCESS) { GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - std::shared_ptr command( + SharedPtr command( + CreateCommand(msg)); + + command->Run(); +} + +TEST_F(SetGlobalPropertiesRequestTest, Run_NoVRNoDataFromSynonyms_SUCCESS) { + MessageSharedPtr msg = CreateMsgParams(); + SmartObject keyboard_properties(smart_objects::SmartType_Map); + (*msg)[am::strings::msg_params][am::hmi_request::keyboard_properties] = + keyboard_properties; + + EXPECT_CALL(app_mngr_, application(kConnectionKey)) + .WillOnce(Return(mock_app_)); + EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(kConnectionKey)); + SmartObject vr_help_title(smart_objects::SmartType_Null); + EXPECT_CALL(*mock_app_, vr_help_title()) + .Times(2) + .WillRepeatedly(Return(&vr_help_title)); + + CommandsMap commands_map; + DataAccessor accessor(commands_map, lock_ptr_); + EXPECT_CALL(*mock_app_, commands_map()).WillOnce(Return(accessor)); + SmartObject vr_help_array(smart_objects::SmartType_Array); + vr_help_array[0] = SmartObject(smart_objects::SmartType_Map); + vr_help_array[0][am::strings::text] = kText; + vr_help_array[0][am::strings::position] = kPosition; + SmartObject vr_synonyms(smart_objects::SmartType_Array); + vr_synonyms[0] = vr_help_array; + const CustomString name("name"); + EXPECT_CALL(*mock_app_, name()).WillOnce(ReturnRef(name)); + EXPECT_CALL(*mock_app_, set_vr_help_title(SmartObject(name))); + EXPECT_CALL(*mock_app_, set_menu_title(_)).Times(0); + EXPECT_CALL(*mock_app_, set_menu_icon(_)).Times(0); + EXPECT_CALL(*mock_app_, set_keyboard_props(keyboard_properties)); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + EXPECT_CALL( + mock_hmi_interfaces_, + GetInterfaceFromFunction(hmi_apis::FunctionID::UI_SetGlobalProperties)) + .WillOnce(Return(am::HmiInterfaces::HMI_INTERFACE_UI)); + ON_CALL(mock_hmi_interfaces_, + GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) + .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); + SharedPtr command( CreateCommand(msg)); command->Run(); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc index 1ea1300c35..f32aaa16c8 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc @@ -339,8 +339,6 @@ TEST_F(ShowRequestTest, Run_Graphic_SUCCESS) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); - EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) - .WillOnce(Return(mobile_apis::Result::SUCCESS)); EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); msg_params[am::strings::app_id] = kAppId; @@ -366,8 +364,6 @@ TEST_F(ShowRequestTest, Run_Graphic_Canceled) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); - EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) - .WillOnce(Return(mobile_apis::Result::ABORTED)); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); EXPECT_CALL(*mock_app_, app_id()).Times(0); EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)).Times(0); @@ -389,7 +385,6 @@ TEST_F(ShowRequestTest, Run_Graphic_WrongSyntax) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); - EXPECT_CALL(mock_message_helper_, VerifyImage(_, _, _)).Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); EXPECT_CALL(*mock_app_, app_id()).Times(0); @@ -412,8 +407,6 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_SUCCESS) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); - EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) - .WillOnce(Return(mobile_apis::Result::SUCCESS)); EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); msg_params[am::strings::app_id] = kAppId; @@ -438,8 +431,6 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_Canceled) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); - EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) - .WillOnce(Return(mobile_apis::Result::ABORTED)); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); EXPECT_CALL(*mock_app_, app_id()).Times(0); @@ -462,7 +453,6 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_WrongSyntax) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); - EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)).Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); EXPECT_CALL(*mock_app_, app_id()).Times(0); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc index acb421af6b..043344f7d7 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc @@ -202,13 +202,6 @@ TEST_F(UpdateTurnListRequestTest, Run_ValidTurnList_SUCCESS) { Ref(app_mngr_))) .WillOnce(Return(mobile_result::SUCCESS)); - EXPECT_CALL( - mock_message_helper_, - VerifyImage( - (*command_msg_)[am::strings::msg_params][am::strings::turn_list][0] - [am::strings::turn_icon], - Eq(mock_app), - Ref(app_mngr_))).WillOnce(Return(mobile_result::SUCCESS)); EXPECT_CALL(mock_message_helper_, SubscribeApplicationToSoftButton(_, _, kFunctionId)); -- cgit v1.2.1 From 32c4573733a9660e05d94ed6c1a192beeda177c1 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Thu, 28 Jun 2018 15:35:58 +0200 Subject: fix for invalid image rpc in create interaction choice set request --- .../resumption/resume_ctrl_impl.h | 9 ---- .../mobile/create_interaction_choice_set_request.h | 1 + .../create_interaction_choice_set_request.cc | 54 ++++++++++++++++++---- .../src/message_helper/message_helper.cc | 17 ------- .../src/resumption/resume_ctrl_impl.cc | 24 +--------- 5 files changed, 46 insertions(+), 59 deletions(-) diff --git a/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h b/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h index 17aabb6d60..b3aa5d099c 100644 --- a/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h +++ b/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h @@ -398,15 +398,6 @@ class ResumeCtrlImpl : public ResumeCtrl, bool CheckAppRestrictions(app_mngr::ApplicationConstSharedPtr application, const smart_objects::SmartObject& saved_app); - /** - * @brief CheckIcons allows to check application icons - * @param application application under resumtion application - * @param json_object - * @return true in case icons exists, false otherwise - */ - bool CheckIcons(app_mngr::ApplicationSharedPtr application, - smart_objects::SmartObject& obj); - /** * @brief CheckDelayAfterIgnOn should check if SDL was started less * then N seconds ago. N will be readed from profile. diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/create_interaction_choice_set_request.h b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/create_interaction_choice_set_request.h index 950c295072..12b075af7a 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/create_interaction_choice_set_request.h +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/create_interaction_choice_set_request.h @@ -128,6 +128,7 @@ class CreateInteractionChoiceSetRequest int32_t choice_set_id_; size_t expected_chs_count_; size_t received_chs_count_; + bool should_send_warnings; /** * @brief Flag for stop sending VR commands to HMI, in case one of responses diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc index 89ae0c1aac..767a4e2c27 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc @@ -42,6 +42,7 @@ #include "utils/gen_hash.h" #include "utils/helpers.h" +#define INVALID_IMG_WARNING_INFO "Requested image(s) not found." namespace sdl_rpc_plugin { using namespace application_manager; @@ -95,6 +96,37 @@ void CreateInteractionChoiceSetRequest::Run() { SendResponse(false, result); return; } + + + should_send_warnings = false; + for (uint32_t i = 0; + i < (*message_)[strings::msg_params][strings::choice_set].length(); + ++i) { + Result::eType verification_result_image = Result::SUCCESS; + Result::eType verification_result_secondary_image = Result::SUCCESS; + if ((*message_)[strings::msg_params][strings::choice_set][i].keyExists( + strings::image)) { + verification_result_image = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::choice_set][i] + [strings::image], + app, + application_manager_); + } + if ((*message_)[strings::msg_params][strings::choice_set][i].keyExists( + strings::secondary_image)) { + verification_result_secondary_image = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::choice_set][i] + [strings::secondary_image], + app, + application_manager_); + } + if (verification_result_image == Result::INVALID_DATA || + verification_result_secondary_image == Result::INVALID_DATA) { + should_send_warnings = true; + break; + } + } + uint32_t grammar_id = application_manager_.GenerateGrammarID(); (*message_)[strings::msg_params][strings::grammar_id] = grammar_id; app->AddChoiceSet(choice_set_id_, (*message_)[strings::msg_params]); @@ -405,16 +437,18 @@ void CreateInteractionChoiceSetRequest::DeleteChoices() { } void CreateInteractionChoiceSetRequest::OnAllHMIResponsesReceived() { - LOG4CXX_AUTO_TRACE(logger_); - - if (!error_from_hmi_) { - SendResponse(true, mobile_apis::Result::SUCCESS); - } else { - DeleteChoices(); - } - - application_manager_.TerminateRequest( - connection_key(), correlation_id(), function_id()); + LOG4CXX_AUTO_TRACE(logger_); + + if (!error_from_hmi_ && should_send_warnings) { + SendResponse(true, mobile_apis::Result::WARNINGS,INVALID_IMG_WARNING_INFO); + } else if (!error_from_hmi_) { + SendResponse(true, mobile_apis::Result::SUCCESS); + } else { + DeleteChoices(); + } + + application_manager_.TerminateRequest( + connection_key(), correlation_id(), function_id()); } } // namespace commands diff --git a/src/components/application_manager/src/message_helper/message_helper.cc b/src/components/application_manager/src/message_helper/message_helper.cc index 978724a671..962fb767dc 100644 --- a/src/components/application_manager/src/message_helper/message_helper.cc +++ b/src/components/application_manager/src/message_helper/message_helper.cc @@ -94,9 +94,6 @@ bool ValidateSoftButtons(smart_objects::SmartObject& soft_buttons) { // Check if image parameter is valid if (button.keyExists(strings::image)) { SmartObject& buttonImage = button[strings::image]; - - // Image name must not be empty and must not contain incorrect - // character if (false == MessageHelper::VerifySoftButtonString( buttonImage[strings::value].asString())) { return false; @@ -2778,13 +2775,6 @@ mobile_apis::Result::eType MessageHelper::ProcessSoftButtons( if (request_soft_buttons[i].keyExists(strings::text)) { request_soft_buttons[i].erase(strings::text); } - - if ((!request_soft_buttons[i].keyExists(strings::image) || - (Result::SUCCESS != - VerifyImage( - request_soft_buttons[i][strings::image], app, app_mngr)))) { - return Result::INVALID_DATA; - } break; } case SoftButtonType::SBT_TEXT: { @@ -2805,13 +2795,6 @@ mobile_apis::Result::eType MessageHelper::ProcessSoftButtons( request_soft_buttons[i][strings::text].asString())))) { return Result::INVALID_DATA; } - - if ((!request_soft_buttons[i].keyExists(strings::image) || - (Result::SUCCESS != - VerifyImage( - request_soft_buttons[i][strings::image], app, app_mngr)))) { - return Result::INVALID_DATA; - } break; } default: { diff --git a/src/components/application_manager/src/resumption/resume_ctrl_impl.cc b/src/components/application_manager/src/resumption/resume_ctrl_impl.cc index 42dc335878..caa04c1ada 100644 --- a/src/components/application_manager/src/resumption/resume_ctrl_impl.cc +++ b/src/components/application_manager/src/resumption/resume_ctrl_impl.cc @@ -478,20 +478,7 @@ bool ResumeCtrlImpl::CheckPersistenceFilesForResumption( const std::string& device_mac = application->mac_address(); bool result = resumption_storage_->GetSavedApplication( application->policy_app_id(), device_mac, saved_app); - if (result) { - if (saved_app.keyExists(strings::application_commands)) { - if (!CheckIcons(application, saved_app[strings::application_commands])) { - return false; - } - } - if (saved_app.keyExists(strings::application_choice_sets)) { - if (!CheckIcons(application, - saved_app[strings::application_choice_sets])) { - return false; - } - } - } - return true; + return result; } bool ResumeCtrlImpl::CheckApplicationHash(ApplicationSharedPtr application, @@ -779,15 +766,6 @@ bool ResumeCtrlImpl::CheckAppRestrictions( return result; } -bool ResumeCtrlImpl::CheckIcons(ApplicationSharedPtr application, - smart_objects::SmartObject& obj) { - using namespace smart_objects; - LOG4CXX_AUTO_TRACE(logger_); - const mobile_apis::Result::eType verify_images = - MessageHelper::VerifyImageFiles(obj, application, application_manager_); - return mobile_apis::Result::INVALID_DATA != verify_images; -} - bool ResumeCtrlImpl::CheckDelayAfterIgnOn() { using namespace date_time; LOG4CXX_AUTO_TRACE(logger_); -- cgit v1.2.1 From fa700ca1122a7c5261170c35310e4d61f9deda27 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Fri, 29 Jun 2018 15:50:57 +0200 Subject: fix build after resolving conflict --- .../sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc index fedb4966f5..94338aae67 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc @@ -272,10 +272,10 @@ TEST_F(AddCommandRequestTest, Run_ImageVerificationFailed_EXPECT_WARNINGS) { am::CommandsMap commands_map; EXPECT_CALL(*mock_app_, commands_map()) - .WillRepeatedly(Return(DataAccessor( - commands_map, lock_ptr_))); + .WillRepeatedly(Return( + DataAccessor(commands_map, lock_ptr_))); EXPECT_CALL( - mock_rpc_service_, + mock_rpc_service_, ManageHMICommand(HMIResultCodeIs(hmi_apis::FunctionID::UI_AddCommand))) .WillOnce(Return(true)); utils::SharedPtr request_ptr = -- cgit v1.2.1 From 58c57fed825fb73a01bceb17c2a2b48473fff6f2 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Fri, 29 Jun 2018 16:32:30 +0200 Subject: fix for show RPC UT --- .../test/commands/mobile/show_test.cc | 27 +++++++++++++++------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc index f32aaa16c8..f582072398 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc @@ -351,7 +351,7 @@ TEST_F(ShowRequestTest, Run_Graphic_SUCCESS) { command->Run(); } -TEST_F(ShowRequestTest, Run_Graphic_Canceled) { +TEST_F(ShowRequestTest, Run_Graphic_Not_Verified) { MessageSharedPtr msg = CreateMsgParams(); SmartObject msg_params(smart_objects::SmartType_Map); @@ -364,11 +364,17 @@ TEST_F(ShowRequestTest, Run_Graphic_Canceled) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) + .Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); - EXPECT_CALL(*mock_app_, app_id()).Times(0); - EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)).Times(0); - EXPECT_CALL(*mock_app_, set_show_command(msg_params)).Times(0); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + msg_params[am::strings::app_id] = kAppId; + msg_params[am::hmi_request::show_strings] = + smart_objects::SmartObject(smart_objects::SmartType_Array); + + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)); + EXPECT_CALL(*mock_app_, set_show_command(msg_params)); command->Run(); } @@ -418,7 +424,7 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_SUCCESS) { command->Run(); } -TEST_F(ShowRequestTest, Run_SecondaryGraphic_Canceled) { +TEST_F(ShowRequestTest, Run_SecondaryGraphic_Not_Verified) { MessageSharedPtr msg = CreateMsgParams(); SmartObject msg_params(smart_objects::SmartType_Map); @@ -431,11 +437,16 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_Canceled) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) + .Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); - EXPECT_CALL(*mock_app_, app_id()).Times(0); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); - EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)).Times(0); - EXPECT_CALL(*mock_app_, set_show_command(msg_params)).Times(0); + msg_params[am::strings::app_id] = kAppId; + msg_params[am::hmi_request::show_strings] = + smart_objects::SmartObject(smart_objects::SmartType_Array); + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)); + EXPECT_CALL(*mock_app_, set_show_command(msg_params)); command->Run(); } -- cgit v1.2.1 From 604057538b085ec79b766bdf6bdf8a25c9258013 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Mon, 2 Jul 2018 15:37:59 +0200 Subject: revert image check and return Warning or invalid data --- .../include/application_manager/message_helper.h | 4 +- .../resumption/resume_ctrl_impl.h | 9 +++ .../src/commands/mobile/add_command_request.cc | 13 ++++ .../create_interaction_choice_set_request.cc | 29 +++++---- .../commands/mobile/perform_interaction_request.cc | 10 +++ .../src/commands/mobile/send_location_request.cc | 13 ++++ .../mobile/set_global_properties_request.cc | 26 ++++++++ .../commands/mobile/show_constant_tbt_request.cc | 20 ++++++ .../src/commands/mobile/show_request.cc | 26 ++++++++ .../commands/mobile/update_turn_list_request.cc | 16 +++++ .../commands/mobile/send_location_request_test.cc | 16 ++++- .../commands/mobile/set_global_properties_test.cc | 2 +- .../test/commands/mobile/show_test.cc | 33 +++++----- .../mobile/update_turn_list_request_test.cc | 7 +++ .../src/message_helper/message_helper.cc | 72 +++++++++++++--------- .../src/resumption/resume_ctrl_impl.cc | 24 +++++++- .../test/message_helper/message_helper_test.cc | 12 ++-- 17 files changed, 262 insertions(+), 70 deletions(-) diff --git a/src/components/application_manager/include/application_manager/message_helper.h b/src/components/application_manager/include/application_manager/message_helper.h index 9559c2007e..b1bf3bd567 100644 --- a/src/components/application_manager/include/application_manager/message_helper.h +++ b/src/components/application_manager/include/application_manager/message_helper.h @@ -615,7 +615,7 @@ class MessageHelper { * @param app current application * @return verification result */ - static mobile_apis::Result::eType VerifyImageApplyPath( + static void ApplyImagePath( smart_objects::SmartObject& image, ApplicationConstSharedPtr app, ApplicationManager& app_mngr); @@ -663,7 +663,7 @@ class MessageHelper { * @return returns FALSE if string contains incorrect character or * string is empty otherwise returns TRUE */ - static bool VerifySoftButtonString(const std::string& str); + static bool VerifyString(const std::string& str); static mobile_apis::Result::eType ProcessSoftButtons( smart_objects::SmartObject& message_params, diff --git a/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h b/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h index b3aa5d099c..17aabb6d60 100644 --- a/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h +++ b/src/components/application_manager/include/application_manager/resumption/resume_ctrl_impl.h @@ -398,6 +398,15 @@ class ResumeCtrlImpl : public ResumeCtrl, bool CheckAppRestrictions(app_mngr::ApplicationConstSharedPtr application, const smart_objects::SmartObject& saved_app); + /** + * @brief CheckIcons allows to check application icons + * @param application application under resumtion application + * @param json_object + * @return true in case icons exists, false otherwise + */ + bool CheckIcons(app_mngr::ApplicationSharedPtr application, + smart_objects::SmartObject& obj); + /** * @brief CheckDelayAfterIgnOn should check if SDL was started less * then N seconds ago. N will be readed from profile. diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc index ab9baea7ef..155f819761 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc @@ -90,6 +90,19 @@ void AddCommandRequest::Run() { return; } + if ((*message_)[strings::msg_params].keyExists(strings::cmd_icon)) { + mobile_apis::Result::eType verification_result = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::cmd_icon], + app, + application_manager_); + + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR( + logger_, "MessageHelper::VerifyImage return " << verification_result); + SendResponse(false, verification_result); + return; + } + } if (!((*message_)[strings::msg_params].keyExists(strings::cmd_id))) { LOG4CXX_ERROR(logger_, "INVALID_DATA"); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc index 767a4e2c27..d36eda18e1 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc @@ -122,6 +122,11 @@ void CreateInteractionChoiceSetRequest::Run() { } if (verification_result_image == Result::INVALID_DATA || verification_result_secondary_image == Result::INVALID_DATA) { + LOG4CXX_ERROR(logger_, "Image verification failed."); + SendResponse(false, Result::INVALID_DATA); + return; + } else if (verification_result_image == Result::WARNINGS || + verification_result_secondary_image == Result::WARNINGS) { should_send_warnings = true; break; } @@ -437,18 +442,18 @@ void CreateInteractionChoiceSetRequest::DeleteChoices() { } void CreateInteractionChoiceSetRequest::OnAllHMIResponsesReceived() { - LOG4CXX_AUTO_TRACE(logger_); - - if (!error_from_hmi_ && should_send_warnings) { - SendResponse(true, mobile_apis::Result::WARNINGS,INVALID_IMG_WARNING_INFO); - } else if (!error_from_hmi_) { - SendResponse(true, mobile_apis::Result::SUCCESS); - } else { - DeleteChoices(); - } - - application_manager_.TerminateRequest( - connection_key(), correlation_id(), function_id()); + LOG4CXX_AUTO_TRACE(logger_); + + if (!error_from_hmi_ && should_send_warnings) { + SendResponse(true, mobile_apis::Result::WARNINGS,INVALID_IMG_WARNING_INFO); + } else if (!error_from_hmi_) { + SendResponse(true, mobile_apis::Result::SUCCESS); + } else { + DeleteChoices(); + } + + application_manager_.TerminateRequest( + connection_key(), correlation_id(), function_id()); } } // namespace commands diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc index fb3516c24c..76dd30525a 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc @@ -164,6 +164,16 @@ void PerformInteractionRequest::Run() { return; } + if (msg_params.keyExists(strings::vr_help)) { + if (mobile_apis::Result::INVALID_DATA == + MessageHelper::VerifyImageVrHelpItems( + msg_params[strings::vr_help], app, application_manager_)) { + LOG4CXX_ERROR(logger_, + "Verification of " << strings::vr_help << " failed."); + SendResponse(false, mobile_apis::Result::INVALID_DATA); + return; + } + } if (IsWhiteSpaceExist()) { LOG4CXX_ERROR(logger_, diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc index e9e11b3f24..2719d0674d 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/send_location_request.cc @@ -117,6 +117,19 @@ void SendLocationRequest::Run() { return; } + if (msg_params.keyExists(strings::location_image)) { + mobile_apis::Result::eType verification_result = + mobile_apis::Result::SUCCESS; + verification_result = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::location_image], + app, + application_manager_); + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR(logger_, "VerifyImage INVALID_DATA!"); + SendResponse(false, verification_result); + return; + } + } SmartObject request_msg_params = SmartObject(smart_objects::SmartType_Map); request_msg_params = msg_params; diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc index 0e8c4fcbfc..0c217b6b4f 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_global_properties_request.cc @@ -88,6 +88,32 @@ void SetGlobalPropertiesRequest::Run() { return; } + mobile_apis::Result::eType verification_result = mobile_apis::Result::SUCCESS; + + if ((*message_)[strings::msg_params].keyExists(strings::menu_icon)) { + verification_result = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::menu_icon], + app, + application_manager_); + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR( + logger_, "MessageHelper::VerifyImage return " << verification_result); + SendResponse(false, verification_result); + return; + } + } + // Check for image file(s) in vrHelpItem + if ((*message_)[strings::msg_params].keyExists(strings::vr_help)) { + if (mobile_apis::Result::INVALID_DATA == + MessageHelper::VerifyImageVrHelpItems( + (*message_)[strings::msg_params][strings::vr_help], + app, + application_manager_)) { + LOG4CXX_ERROR(logger_, "MessageHelper::VerifyImage return INVALID_DATA!"); + SendResponse(false, mobile_apis::Result::INVALID_DATA); + return; + } + } if (IsWhiteSpaceExist()) { LOG4CXX_ERROR(logger_, "White spaces found"); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc index 1e5767258d..3b2936e6cd 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_request.cc @@ -101,6 +101,26 @@ void ShowConstantTBTRequest::Run() { return; } + mobile_apis::Result::eType verification_result = mobile_apis::Result::SUCCESS; + if (msg_params.keyExists(strings::turn_icon)) { + verification_result = MessageHelper::VerifyImage( + msg_params[strings::turn_icon], app, application_manager_); + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR(logger_, "VerifyImage INVALID_DATA!"); + SendResponse(false, verification_result); + return; + } + } + + if (msg_params.keyExists(strings::next_turn_icon)) { + verification_result = MessageHelper::VerifyImage( + msg_params[strings::next_turn_icon], app, application_manager_); + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR(logger_, "VerifyImage INVALID_DATA!"); + SendResponse(false, verification_result); + return; + } + } msg_params[strings::app_id] = app->app_id(); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc index 25b96d6aab..e0c3e1da9a 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_request.cc @@ -135,6 +135,32 @@ void ShowRequest::Run() { return; } + mobile_apis::Result::eType verification_result = mobile_apis::Result::SUCCESS; + if (((*message_)[strings::msg_params].keyExists(strings::graphic)) && + ((*message_)[strings::msg_params][strings::graphic][strings::value] + .asString()).length()) { + verification_result = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::graphic], + app, + application_manager_); + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR(logger_, "Image verification failed."); + SendResponse(false, verification_result); + return; + } + } + + if ((*message_)[strings::msg_params].keyExists(strings::secondary_graphic)) { + verification_result = MessageHelper::VerifyImage( + (*message_)[strings::msg_params][strings::secondary_graphic], + app, + application_manager_); + if (mobile_apis::Result::INVALID_DATA == verification_result) { + LOG4CXX_ERROR(logger_, "Image verification failed."); + SendResponse(false, verification_result); + return; + } + } smart_objects::SmartObject msg_params = smart_objects::SmartObject(smart_objects::SmartType_Map); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc index f28656514f..c04ad34f5f 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_request.cc @@ -95,6 +95,22 @@ void UpdateTurnListRequest::Run() { return; } + if ((*message_)[strings::msg_params].keyExists(strings::turn_list)) { + smart_objects::SmartObject& turn_list_array = + ((*message_)[strings::msg_params][strings::turn_list]); + for (uint32_t i = 0; i < turn_list_array.length(); ++i) { + if ((turn_list_array[i].keyExists(strings::turn_icon)) && + (mobile_apis::Result::INVALID_DATA == + MessageHelper::VerifyImage(turn_list_array[i][strings::turn_icon], + app, + application_manager_))) { + LOG4CXX_ERROR(logger_, + "MessageHelper::VerifyImage return INVALID_DATA"); + SendResponse(false, mobile_apis::Result::INVALID_DATA); + return; + } + } + } smart_objects::SmartObject msg_params = smart_objects::SmartObject(smart_objects::SmartType_Map); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc index 4a4fa88f33..ebbd633912 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc @@ -306,17 +306,27 @@ TEST_F(SendLocationRequestTest, Run_LocationImageValid_Success) { SmartObject(smart_objects::SmartType_Map); (*message_)[strings::msg_params][strings::location_image][strings::value] = "1"; + EXPECT_CALL( + mock_message_helper_, + VerifyImage( + (*message_)[strings::msg_params][strings::location_image], _, _)) + .WillOnce(Return(mobile_apis::Result::SUCCESS)); FinishSetup(); command_->Run(); } -TEST_F(SendLocationRequestTest, Run_LocationImageInvalid_Warnings) { +TEST_F(SendLocationRequestTest, Run_LocationImageInvalid_Cancelled) { InitialSetup(message_); (*message_)[strings::msg_params][strings::location_image] = SmartObject(smart_objects::SmartType_Map); (*message_)[strings::msg_params][strings::location_image][strings::value] = - "1"; - FinishSetup(); + " 1"; + EXPECT_CALL( + mock_message_helper_, + VerifyImage( + (*message_)[strings::msg_params][strings::location_image], _, _)) + .WillOnce(Return(mobile_apis::Result::INVALID_DATA)); + FinishSetupCancelled(mobile_apis::Result::INVALID_DATA); command_->Run(); } diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc index 4ee5af8279..ae82f24b61 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc @@ -496,7 +496,7 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRBrokenMenuIcon_Canceled) { SmartObject vr_help_title("yes"); (*msg)[am::strings::msg_params][am::strings::vr_help_title] = vr_help_title; SmartObject menu_icon(smart_objects::SmartType_Map); - menu_icon[am::strings::value] = "1"; + menu_icon[am::strings::value] = " 1"; (*msg)[am::strings::msg_params][am::hmi_request::menu_icon] = menu_icon; EXPECT_CALL(app_mngr_, application(kConnectionKey)) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc index f582072398..ac2b59a613 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc @@ -339,6 +339,8 @@ TEST_F(ShowRequestTest, Run_Graphic_SUCCESS) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) + .WillOnce(Return(mobile_apis::Result::SUCCESS)); EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); msg_params[am::strings::app_id] = kAppId; @@ -351,7 +353,7 @@ TEST_F(ShowRequestTest, Run_Graphic_SUCCESS) { command->Run(); } -TEST_F(ShowRequestTest, Run_Graphic_Not_Verified) { +TEST_F(ShowRequestTest, Run_Graphic_Canceled) { MessageSharedPtr msg = CreateMsgParams(); SmartObject msg_params(smart_objects::SmartType_Map); @@ -365,16 +367,12 @@ TEST_F(ShowRequestTest, Run_Graphic_Not_Verified) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) - .Times(0); + .WillOnce(Return(mobile_apis::Result::INVALID_DATA)); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); - EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); - - msg_params[am::strings::app_id] = kAppId; - msg_params[am::hmi_request::show_strings] = - smart_objects::SmartObject(smart_objects::SmartType_Array); + EXPECT_CALL(*mock_app_, app_id()).Times(0); + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)).Times(0); + EXPECT_CALL(*mock_app_, set_show_command(msg_params)).Times(0); - EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)); - EXPECT_CALL(*mock_app_, set_show_command(msg_params)); command->Run(); } @@ -391,6 +389,7 @@ TEST_F(ShowRequestTest, Run_Graphic_WrongSyntax) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(_, _, _)).Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); EXPECT_CALL(*mock_app_, app_id()).Times(0); @@ -413,6 +412,8 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_SUCCESS) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) + .WillOnce(Return(mobile_apis::Result::SUCCESS)); EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); msg_params[am::strings::app_id] = kAppId; @@ -424,7 +425,7 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_SUCCESS) { command->Run(); } -TEST_F(ShowRequestTest, Run_SecondaryGraphic_Not_Verified) { +TEST_F(ShowRequestTest, Run_SecondaryGraphic_Canceled) { MessageSharedPtr msg = CreateMsgParams(); SmartObject msg_params(smart_objects::SmartType_Map); @@ -438,15 +439,12 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_Not_Verified) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) - .Times(0); + .WillOnce(Return(mobile_apis::Result::INVALID_DATA)); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); - EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + EXPECT_CALL(*mock_app_, app_id()).Times(0); - msg_params[am::strings::app_id] = kAppId; - msg_params[am::hmi_request::show_strings] = - smart_objects::SmartObject(smart_objects::SmartType_Array); - EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)); - EXPECT_CALL(*mock_app_, set_show_command(msg_params)); + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)).Times(0); + EXPECT_CALL(*mock_app_, set_show_command(msg_params)).Times(0); command->Run(); } @@ -464,6 +462,7 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_WrongSyntax) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)).Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); EXPECT_CALL(*mock_app_, app_id()).Times(0); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc index 043344f7d7..acb421af6b 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc @@ -202,6 +202,13 @@ TEST_F(UpdateTurnListRequestTest, Run_ValidTurnList_SUCCESS) { Ref(app_mngr_))) .WillOnce(Return(mobile_result::SUCCESS)); + EXPECT_CALL( + mock_message_helper_, + VerifyImage( + (*command_msg_)[am::strings::msg_params][am::strings::turn_list][0] + [am::strings::turn_icon], + Eq(mock_app), + Ref(app_mngr_))).WillOnce(Return(mobile_result::SUCCESS)); EXPECT_CALL(mock_message_helper_, SubscribeApplicationToSoftButton(_, _, kFunctionId)); diff --git a/src/components/application_manager/src/message_helper/message_helper.cc b/src/components/application_manager/src/message_helper/message_helper.cc index 962fb767dc..58b3552e9f 100644 --- a/src/components/application_manager/src/message_helper/message_helper.cc +++ b/src/components/application_manager/src/message_helper/message_helper.cc @@ -94,7 +94,7 @@ bool ValidateSoftButtons(smart_objects::SmartObject& soft_buttons) { // Check if image parameter is valid if (button.keyExists(strings::image)) { SmartObject& buttonImage = button[strings::image]; - if (false == MessageHelper::VerifySoftButtonString( + if (false == MessageHelper::VerifyString( buttonImage[strings::value].asString())) { return false; } @@ -2598,27 +2598,17 @@ mobile_apis::Result::eType MessageHelper::VerifyImageFiles( return mobile_apis::Result::SUCCESS; } -mobile_apis::Result::eType MessageHelper::VerifyImageApplyPath( +void MessageHelper::ApplyImagePath( smart_objects::SmartObject& image, ApplicationConstSharedPtr app, ApplicationManager& app_mngr) { - // Checking image type first: if STATIC - skip existence check, since it is - // HMI related file and it should know it location - const uint32_t image_type = image[strings::image_type].asUInt(); - mobile_apis::ImageType::eType type = - static_cast(image_type); - if (mobile_apis::ImageType::STATIC == type) { - return mobile_apis::Result::SUCCESS; - } const std::string& file_name = image[strings::value].asString(); const std::string& full_file_path = GetAppFilePath(file_name, app, app_mngr); image[strings::value] = full_file_path; - if (file_system::FileExists(full_file_path)) { - return mobile_apis::Result::SUCCESS; - } - return mobile_apis::Result::INVALID_DATA; + + return; } std::string MessageHelper::GetAppFilePath(std::string file_name, @@ -2679,19 +2669,29 @@ mobile_apis::Result::eType MessageHelper::VerifyImage( smart_objects::SmartObject& image, ApplicationConstSharedPtr app, ApplicationManager& app_mngr) { - smart_objects::SmartObject temp_image = image; + const uint32_t image_type = image[strings::image_type].asUInt(); - const mobile_apis::ImageType::eType type = + mobile_apis::ImageType::eType type = static_cast(image_type); + const std::string& file_name = image[strings::value].asString(); - const mobile_apis::Result::eType result = - VerifyImageApplyPath(temp_image, app, app_mngr); - if ((mobile_apis::Result::SUCCESS == result) && - (mobile_apis::ImageType::DYNAMIC == type)) { - image[strings::value] = temp_image[strings::value]; + + if(!VerifyString(file_name)){ + return mobile_apis::Result::INVALID_DATA; } - return result; + if (mobile_apis::ImageType::STATIC == type) { + return mobile_apis::Result::SUCCESS; + } + + ApplyImagePath(image, app, app_mngr); + + const std::string& imagePath = image[strings::value].asString(); + + if (file_system::FileExists(imagePath)) { + return mobile_apis::Result::SUCCESS; + } + return mobile_apis::Result::WARNINGS; } mobile_apis::Result::eType MessageHelper::VerifyImageVrHelpItems( @@ -2704,21 +2704,21 @@ mobile_apis::Result::eType MessageHelper::VerifyImageVrHelpItems( if (message[i].keyExists(strings::image)) { verification_result_image = VerifyImage(message[i][strings::image], app, app_mngr); - if (mobile_apis::Result::SUCCESS != verification_result_image) { - return verification_result_image; + if (mobile_apis::Result::INVALID_DATA == verification_result_image) { + break; } } } - return mobile_apis::Result::SUCCESS; + return verification_result_image; } -bool MessageHelper::VerifySoftButtonString(const std::string& str) { +bool MessageHelper::VerifyString(const std::string& str) { if ((std::string::npos != str.find_first_of("\t\n")) || (std::string::npos != str.find("\\n")) || (std::string::npos != str.find("\\t")) || (std::string::npos == str.find_first_not_of(' '))) { LOG4CXX_ERROR(logger_, - "MessageHelper::VerifySoftButtonString" + "MessageHelper::VerifyString" "string contains incorrect character"); return false; } @@ -2775,6 +2775,13 @@ mobile_apis::Result::eType MessageHelper::ProcessSoftButtons( if (request_soft_buttons[i].keyExists(strings::text)) { request_soft_buttons[i].erase(strings::text); } + + if ((!request_soft_buttons[i].keyExists(strings::image) || + (Result::INVALID_DATA == + VerifyImage( + request_soft_buttons[i][strings::image], app, app_mngr)))) { + return Result::INVALID_DATA; + } break; } case SoftButtonType::SBT_TEXT: { @@ -2782,7 +2789,7 @@ mobile_apis::Result::eType MessageHelper::ProcessSoftButtons( request_soft_buttons[i].erase(strings::image); } if ((!request_soft_buttons[i].keyExists(strings::text)) || - (!VerifySoftButtonString( + (!VerifyString( request_soft_buttons[i][strings::text].asString()))) { return Result::INVALID_DATA; } @@ -2791,10 +2798,17 @@ mobile_apis::Result::eType MessageHelper::ProcessSoftButtons( case SoftButtonType::SBT_BOTH: { if ((!request_soft_buttons[i].keyExists(strings::text)) || ((request_soft_buttons[i][strings::text].length()) && - (!VerifySoftButtonString( + (!VerifyString( request_soft_buttons[i][strings::text].asString())))) { return Result::INVALID_DATA; } + + if ((!request_soft_buttons[i].keyExists(strings::image) || + (Result::INVALID_DATA == + VerifyImage( + request_soft_buttons[i][strings::image], app, app_mngr)))) { + return Result::INVALID_DATA; + } break; } default: { diff --git a/src/components/application_manager/src/resumption/resume_ctrl_impl.cc b/src/components/application_manager/src/resumption/resume_ctrl_impl.cc index caa04c1ada..42dc335878 100644 --- a/src/components/application_manager/src/resumption/resume_ctrl_impl.cc +++ b/src/components/application_manager/src/resumption/resume_ctrl_impl.cc @@ -478,7 +478,20 @@ bool ResumeCtrlImpl::CheckPersistenceFilesForResumption( const std::string& device_mac = application->mac_address(); bool result = resumption_storage_->GetSavedApplication( application->policy_app_id(), device_mac, saved_app); - return result; + if (result) { + if (saved_app.keyExists(strings::application_commands)) { + if (!CheckIcons(application, saved_app[strings::application_commands])) { + return false; + } + } + if (saved_app.keyExists(strings::application_choice_sets)) { + if (!CheckIcons(application, + saved_app[strings::application_choice_sets])) { + return false; + } + } + } + return true; } bool ResumeCtrlImpl::CheckApplicationHash(ApplicationSharedPtr application, @@ -766,6 +779,15 @@ bool ResumeCtrlImpl::CheckAppRestrictions( return result; } +bool ResumeCtrlImpl::CheckIcons(ApplicationSharedPtr application, + smart_objects::SmartObject& obj) { + using namespace smart_objects; + LOG4CXX_AUTO_TRACE(logger_); + const mobile_apis::Result::eType verify_images = + MessageHelper::VerifyImageFiles(obj, application, application_manager_); + return mobile_apis::Result::INVALID_DATA != verify_images; +} + bool ResumeCtrlImpl::CheckDelayAfterIgnOn() { using namespace date_time; LOG4CXX_AUTO_TRACE(logger_); diff --git a/src/components/application_manager/test/message_helper/message_helper_test.cc b/src/components/application_manager/test/message_helper/message_helper_test.cc index 1e8a5f5a40..338c359bbe 100644 --- a/src/components/application_manager/test/message_helper/message_helper_test.cc +++ b/src/components/application_manager/test/message_helper/message_helper_test.cc @@ -715,7 +715,7 @@ TEST_F(MessageHelperTest, VerifySoftButtonString_WrongStrings_False) { "soft_button1\\n", "soft_button1\\t"}; for (size_t i = 0; i < wrong_strings.size(); ++i) { - EXPECT_FALSE(MessageHelper::VerifySoftButtonString(wrong_strings[i])); + EXPECT_FALSE(MessageHelper::VerifyString(wrong_strings[i])); } } @@ -726,7 +726,7 @@ TEST_F(MessageHelperTest, VerifySoftButtonString_CorrectStrings_True) { "soft_button1??....asd", "soft_button12313fcvzxc./.,"}; for (size_t i = 0; i < wrong_strings.size(); ++i) { - EXPECT_TRUE(MessageHelper::VerifySoftButtonString(wrong_strings[i])); + EXPECT_TRUE(MessageHelper::VerifyString(wrong_strings[i])); } } @@ -771,6 +771,7 @@ TEST_F(MessageHelperTest, VerifyImage_ImageTypeIsStatic_Success) { // Creating input data for method smart_objects::SmartObject image; image[strings::image_type] = mobile_apis::ImageType::STATIC; + image[strings::value] = "static_icon"; // Method call mobile_apis::Result::eType result = MessageHelper::VerifyImage( image, appSharedMock, mock_application_manager); @@ -801,10 +802,9 @@ TEST_F(MessageHelperTest, VerifyImageApplyPath_ImageTypeIsStatic_Success) { image[strings::image_type] = mobile_apis::ImageType::STATIC; image[strings::value] = "icon.png"; // Method call - mobile_apis::Result::eType result = MessageHelper::VerifyImageApplyPath( + MessageHelper::ApplyImagePath( image, appSharedMock, mock_application_manager); // EXPECT - EXPECT_EQ(mobile_apis::Result::SUCCESS, result); EXPECT_EQ("icon.png", image[strings::value].asString()); } @@ -817,7 +817,7 @@ TEST_F(MessageHelperTest, VerifyImageApplyPath_ImageValueNotValid_InvalidData) { // Invalid value image[strings::value] = " "; // Method call - mobile_apis::Result::eType result = MessageHelper::VerifyImageApplyPath( + mobile_apis::Result::eType result = MessageHelper::VerifyImage( image, appSharedMock, mock_application_manager); // EXPECT EXPECT_EQ(mobile_apis::Result::INVALID_DATA, result); @@ -830,6 +830,8 @@ TEST_F(MessageHelperTest, VerifyImageFiles_SmartObjectWithValidData_Success) { smart_objects::SmartObject images; images[0][strings::image_type] = mobile_apis::ImageType::STATIC; images[1][strings::image_type] = mobile_apis::ImageType::STATIC; + images[0][strings::value] = "static_icon"; + images[1][strings::value] = "static_icon"; // Method call mobile_apis::Result::eType result = MessageHelper::VerifyImageFiles( images, appSharedMock, mock_application_manager); -- cgit v1.2.1 From 2b1b413d63ee94503d26d0acc039e7083730b5dd Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Mon, 2 Jul 2018 15:56:21 +0200 Subject: create interaction choice set run method changes --- .../create_interaction_choice_set_request.cc | 35 ++++++++++------------ 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc index d36eda18e1..0612daa3e3 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc @@ -79,25 +79,6 @@ void CreateInteractionChoiceSetRequest::Run() { SendResponse(false, mobile_apis::Result::APPLICATION_NOT_REGISTERED); return; } - - choice_set_id_ = - (*message_)[strings::msg_params][strings::interaction_choice_set_id] - .asInt(); - - if (app->FindChoiceSet(choice_set_id_)) { - LOG4CXX_ERROR(logger_, - "Choice set with id " << choice_set_id_ << " is not found."); - SendResponse(false, Result::INVALID_ID); - return; - } - - Result::eType result = CheckChoiceSet(app); - if (Result::SUCCESS != result) { - SendResponse(false, result); - return; - } - - should_send_warnings = false; for (uint32_t i = 0; i < (*message_)[strings::msg_params][strings::choice_set].length(); @@ -132,6 +113,22 @@ void CreateInteractionChoiceSetRequest::Run() { } } + choice_set_id_ = + (*message_)[strings::msg_params][strings::interaction_choice_set_id] + .asInt(); + + if (app->FindChoiceSet(choice_set_id_)) { + LOG4CXX_ERROR(logger_, + "Choice set with id " << choice_set_id_ << " is not found."); + SendResponse(false, Result::INVALID_ID); + return; + } + + Result::eType result = CheckChoiceSet(app); + if (Result::SUCCESS != result) { + SendResponse(false, result); + return; + } uint32_t grammar_id = application_manager_.GenerateGrammarID(); (*message_)[strings::msg_params][strings::grammar_id] = grammar_id; app->AddChoiceSet(choice_set_id_, (*message_)[strings::msg_params]); -- cgit v1.2.1 From 193aaf8b290128669cc0e75b176931a16d6ca5a0 Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Mon, 2 Jul 2018 16:23:53 +0200 Subject: added UT for warnings --- .../commands/mobile/send_location_request_test.cc | 15 ++++ .../commands/mobile/set_global_properties_test.cc | 88 ++++++++++++++++++++++ .../test/commands/mobile/show_test.cc | 53 +++++++++++++ .../mobile/update_turn_list_request_test.cc | 60 +++++++++++++++ .../test/message_helper/message_helper_test.cc | 8 +- 5 files changed, 223 insertions(+), 1 deletion(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc index ebbd633912..e4144c0911 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc @@ -315,6 +315,21 @@ TEST_F(SendLocationRequestTest, Run_LocationImageValid_Success) { command_->Run(); } +TEST_F(SendLocationRequestTest, Run_LocationImageValid_Warnings) { + InitialSetup(message_); + (*message_)[strings::msg_params][strings::location_image] = + SmartObject(smart_objects::SmartType_Map); + (*message_)[strings::msg_params][strings::location_image][strings::value] = + "notavailable"; + EXPECT_CALL( + mock_message_helper_, + VerifyImage( + (*message_)[strings::msg_params][strings::location_image], _, _)) + .WillOnce(Return(mobile_apis::Result::WARNINGS)); + FinishSetup(); + command_->Run(); +} + TEST_F(SendLocationRequestTest, Run_LocationImageInvalid_Cancelled) { InitialSetup(message_); (*message_)[strings::msg_params][strings::location_image] = diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc index ae82f24b61..6b9d8cc468 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc @@ -491,6 +491,94 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_SUCCESS) { command->Run(); } +TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_Menu_WARNINGS) { + MessageSharedPtr msg = CreateMsgParams(); + SmartObject vr_help_title("yes"); + SmartObject vr_help_array(smart_objects::SmartType_Array); + VRArraySetupHelper(msg, vr_help_title, vr_help_array); + (*msg)[am::strings::msg_params][am::strings::vr_help] = vr_help_array; + SmartObject menu_title("Menu_Title"); + (*msg)[am::strings::msg_params][am::hmi_request::menu_title] = menu_title; + SmartObject menu_icon(smart_objects::SmartType_Map); + menu_icon[am::strings::value] = "1"; + (*msg)[am::strings::msg_params][am::hmi_request::menu_icon] = menu_icon; + SmartObject keyboard_properties(smart_objects::SmartType_Map); + (*msg)[am::strings::msg_params][am::hmi_request::keyboard_properties] = + keyboard_properties; + + EXPECT_CALL(mock_message_helper_, VerifyImage(menu_icon, _, _)) + .WillOnce((Return(mobile_apis::Result::WARNINGS))); + EXPECT_CALL(mock_message_helper_, VerifyImageVrHelpItems(vr_help_array, _, _)) + .WillOnce((Return(mobile_apis::Result::SUCCESS))); + EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(kConnectionKey)); + EXPECT_CALL(*mock_app_, set_vr_help_title(vr_help_title)); + EXPECT_CALL(*mock_app_, set_vr_help(vr_help_array)); + EXPECT_CALL(*mock_app_, vr_help_title()).WillOnce(Return(&vr_help_title)); + EXPECT_CALL(*mock_app_, vr_help()).WillOnce(Return(&vr_help_array)); + EXPECT_CALL(*mock_app_, set_menu_title(menu_title)); + EXPECT_CALL(*mock_app_, set_menu_icon(menu_icon)); + EXPECT_CALL(*mock_app_, set_keyboard_props(keyboard_properties)); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + + EXPECT_CALL( + mock_hmi_interfaces_, + GetInterfaceFromFunction(hmi_apis::FunctionID::UI_SetGlobalProperties)) + .WillOnce(Return(am::HmiInterfaces::HMI_INTERFACE_UI)); + + ON_CALL(mock_hmi_interfaces_, + GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) + .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); + + SharedPtr command( + CreateCommand(msg)); + + command->Run(); +} + +TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_VR_WARNINGS) { + MessageSharedPtr msg = CreateMsgParams(); + SmartObject vr_help_title("yes"); + SmartObject vr_help_array(smart_objects::SmartType_Array); + VRArraySetupHelper(msg, vr_help_title, vr_help_array); + (*msg)[am::strings::msg_params][am::strings::vr_help] = vr_help_array; + SmartObject menu_title("Menu_Title"); + (*msg)[am::strings::msg_params][am::hmi_request::menu_title] = menu_title; + SmartObject menu_icon(smart_objects::SmartType_Map); + menu_icon[am::strings::value] = "1"; + (*msg)[am::strings::msg_params][am::hmi_request::menu_icon] = menu_icon; + SmartObject keyboard_properties(smart_objects::SmartType_Map); + (*msg)[am::strings::msg_params][am::hmi_request::keyboard_properties] = + keyboard_properties; + + EXPECT_CALL(mock_message_helper_, VerifyImage(menu_icon, _, _)) + .WillOnce((Return(mobile_apis::Result::SUCCESS))); + EXPECT_CALL(mock_message_helper_, VerifyImageVrHelpItems(vr_help_array, _, _)) + .WillOnce((Return(mobile_apis::Result::WARNINGS))); + EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(kConnectionKey)); + EXPECT_CALL(*mock_app_, set_vr_help_title(vr_help_title)); + EXPECT_CALL(*mock_app_, set_vr_help(vr_help_array)); + EXPECT_CALL(*mock_app_, vr_help_title()).WillOnce(Return(&vr_help_title)); + EXPECT_CALL(*mock_app_, vr_help()).WillOnce(Return(&vr_help_array)); + EXPECT_CALL(*mock_app_, set_menu_title(menu_title)); + EXPECT_CALL(*mock_app_, set_menu_icon(menu_icon)); + EXPECT_CALL(*mock_app_, set_keyboard_props(keyboard_properties)); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + + EXPECT_CALL( + mock_hmi_interfaces_, + GetInterfaceFromFunction(hmi_apis::FunctionID::UI_SetGlobalProperties)) + .WillOnce(Return(am::HmiInterfaces::HMI_INTERFACE_UI)); + + ON_CALL(mock_hmi_interfaces_, + GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) + .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); + + SharedPtr command( + CreateCommand(msg)); + + command->Run(); +} + TEST_F(SetGlobalPropertiesRequestTest, Run_VRBrokenMenuIcon_Canceled) { MessageSharedPtr msg = CreateMsgParams(); SmartObject vr_help_title("yes"); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc index ac2b59a613..2d9be1b5cf 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc @@ -353,6 +353,33 @@ TEST_F(ShowRequestTest, Run_Graphic_SUCCESS) { command->Run(); } +TEST_F(ShowRequestTest, Run_Graphic_WARNINGS) { + MessageSharedPtr msg = CreateMsgParams(); + + SmartObject msg_params(smart_objects::SmartType_Map); + SmartObject graphic(smart_objects::SmartType_Map); + graphic[am::strings::value] = "1"; + msg_params[am::strings::graphic] = graphic; + (*msg)[am::strings::msg_params] = msg_params; + + SharedPtr command(CreateCommand(msg)); + + EXPECT_CALL(app_mngr_, application(kConnectionKey)) + .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) + .WillOnce(Return(mobile_apis::Result::WARNINGS)); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + + msg_params[am::strings::app_id] = kAppId; + msg_params[am::hmi_request::show_strings] = + smart_objects::SmartObject(smart_objects::SmartType_Array); + + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)); + EXPECT_CALL(*mock_app_, set_show_command(msg_params)); + + command->Run(); +} + TEST_F(ShowRequestTest, Run_Graphic_Canceled) { MessageSharedPtr msg = CreateMsgParams(); @@ -425,6 +452,32 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_SUCCESS) { command->Run(); } +TEST_F(ShowRequestTest, Run_SecondaryGraphic_WARNINGS) { + MessageSharedPtr msg = CreateMsgParams(); + + SmartObject msg_params(smart_objects::SmartType_Map); + SmartObject graphic(smart_objects::SmartType_Map); + graphic[am::strings::value] = "1"; + msg_params[am::strings::secondary_graphic] = graphic; + (*msg)[am::strings::msg_params] = msg_params; + + SharedPtr command(CreateCommand(msg)); + + EXPECT_CALL(app_mngr_, application(kConnectionKey)) + .WillOnce(Return(mock_app_)); + EXPECT_CALL(mock_message_helper_, VerifyImage(graphic, _, _)) + .WillOnce(Return(mobile_apis::Result::WARNINGS)); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + + msg_params[am::strings::app_id] = kAppId; + msg_params[am::hmi_request::show_strings] = + smart_objects::SmartObject(smart_objects::SmartType_Array); + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)); + EXPECT_CALL(*mock_app_, set_show_command(msg_params)); + + command->Run(); +} + TEST_F(ShowRequestTest, Run_SecondaryGraphic_Canceled) { MessageSharedPtr msg = CreateMsgParams(); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc index acb421af6b..d07e76350f 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc @@ -241,6 +241,66 @@ TEST_F(UpdateTurnListRequestTest, Run_ValidTurnList_SUCCESS) { .asString()); } +TEST_F(UpdateTurnListRequestTest, Run_ValidTurnList_WARNINGS) { + const std::string kNavigationText = "valid_navigation_text"; + + (*command_msg_)[am::strings::msg_params][am::strings::turn_list][0] + [am::strings::navigation_text] = kNavigationText; + (*command_msg_)[am::strings::msg_params][am::strings::turn_list][0] + [am::strings::turn_icon][am::strings::value] = + "valid_turn_icon"; + (*command_msg_)[am::strings::msg_params][am::strings::soft_buttons] = 0; + + MockAppPtr mock_app(CreateMockApp()); + EXPECT_CALL(app_mngr_, application(kConnectionKey)) + .WillOnce(Return(mock_app)); + + EXPECT_CALL(mock_message_helper_, + ProcessSoftButtons((*command_msg_)[am::strings::msg_params], + Eq(mock_app), + Ref(mock_policy_handler_), + Ref(app_mngr_))) + .WillOnce(Return(mobile_result::SUCCESS)); + + EXPECT_CALL( + mock_message_helper_, + VerifyImage( + (*command_msg_)[am::strings::msg_params][am::strings::turn_list][0] + [am::strings::turn_icon], + Eq(mock_app), + Ref(app_mngr_))).WillOnce(Return(mobile_result::WARNINGS)); + + EXPECT_CALL(mock_message_helper_, + SubscribeApplicationToSoftButton(_, _, kFunctionId)); + + MessageSharedPtr result_msg(CatchHMICommandResult(CallRun(*command_))); + ASSERT_TRUE(result_msg); + EXPECT_EQ( + hmi_apis::FunctionID::Navigation_UpdateTurnList, + (*result_msg)[am::strings::params][am::strings::function_id].asInt()); + + ASSERT_TRUE((*result_msg)[am::strings::msg_params][am::strings::turn_list][0] + .keyExists(am::hmi_request::navi_text)); + + EXPECT_TRUE((*result_msg)[am::strings::msg_params][am::strings::turn_list][0] + [am::hmi_request::navi_text].keyExists( + am::hmi_request::field_name)); + EXPECT_EQ( + hmi_apis::Common_TextFieldName::turnText, + (*result_msg)[am::strings::msg_params][am::strings::turn_list][0] + [am::hmi_request::navi_text][am::hmi_request::field_name] + .asInt()); + + EXPECT_TRUE((*result_msg)[am::strings::msg_params][am::strings::turn_list][0] + [am::hmi_request::navi_text].keyExists( + am::hmi_request::field_text)); + EXPECT_EQ( + kNavigationText, + (*result_msg)[am::strings::msg_params][am::strings::turn_list][0] + [am::hmi_request::navi_text][am::hmi_request::field_text] + .asString()); +} + TEST_F(UpdateTurnListRequestTest, OnEvent_UnknownEvent_UNSUCCESS) { Event event(hmi_apis::FunctionID::INVALID_ENUM); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)).Times(0); diff --git a/src/components/application_manager/test/message_helper/message_helper_test.cc b/src/components/application_manager/test/message_helper/message_helper_test.cc index 338c359bbe..d8cc4329d3 100644 --- a/src/components/application_manager/test/message_helper/message_helper_test.cc +++ b/src/components/application_manager/test/message_helper/message_helper_test.cc @@ -802,8 +802,9 @@ TEST_F(MessageHelperTest, VerifyImageApplyPath_ImageTypeIsStatic_Success) { image[strings::image_type] = mobile_apis::ImageType::STATIC; image[strings::value] = "icon.png"; // Method call - MessageHelper::ApplyImagePath( + mobile_apis::Result::eType result = MessageHelper::VerifyImage( image, appSharedMock, mock_application_manager); + EXPECT_EQ(mobile_apis::Result::SUCCESS, result); // EXPECT EXPECT_EQ("icon.png", image[strings::value].asString()); } @@ -867,6 +868,11 @@ TEST_F(MessageHelperTest, mobile_apis::ImageType::STATIC; message[1][strings::image][strings::image_type] = mobile_apis::ImageType::STATIC; + + message[0][strings::image][strings::value] = + "static_icon"; + message[1][strings::image][strings::value] = + "static_icon"; // Method call mobile_apis::Result::eType result = MessageHelper::VerifyImageVrHelpItems( message, appSharedMock, mock_application_manager); -- cgit v1.2.1 From ac1216b8b697b287217175b76da6ad6b0f5649ba Mon Sep 17 00:00:00 2001 From: Ashwin Karemore Date: Mon, 2 Jul 2018 17:35:54 +0200 Subject: fix style --- .../include/application_manager/message_helper.h | 7 +++---- .../test/commands/mobile/add_command_request_test.cc | 6 +++--- .../test/commands/mobile/set_global_properties_test.cc | 3 ++- .../src/message_helper/message_helper.cc | 18 +++++++----------- .../test/message_helper/message_helper_test.cc | 6 ++---- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/components/application_manager/include/application_manager/message_helper.h b/src/components/application_manager/include/application_manager/message_helper.h index b1bf3bd567..127ff967b4 100644 --- a/src/components/application_manager/include/application_manager/message_helper.h +++ b/src/components/application_manager/include/application_manager/message_helper.h @@ -615,10 +615,9 @@ class MessageHelper { * @param app current application * @return verification result */ - static void ApplyImagePath( - smart_objects::SmartObject& image, - ApplicationConstSharedPtr app, - ApplicationManager& app_mngr); + static void ApplyImagePath(smart_objects::SmartObject& image, + ApplicationConstSharedPtr app, + ApplicationManager& app_mngr); /* * @brief Verify image and add image file full path diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc index 94338aae67..fedb4966f5 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc @@ -272,10 +272,10 @@ TEST_F(AddCommandRequestTest, Run_ImageVerificationFailed_EXPECT_WARNINGS) { am::CommandsMap commands_map; EXPECT_CALL(*mock_app_, commands_map()) - .WillRepeatedly(Return( - DataAccessor(commands_map, lock_ptr_))); + .WillRepeatedly(Return(DataAccessor( + commands_map, lock_ptr_))); EXPECT_CALL( - mock_rpc_service_, + mock_rpc_service_, ManageHMICommand(HMIResultCodeIs(hmi_apis::FunctionID::UI_AddCommand))) .WillOnce(Return(true)); utils::SharedPtr request_ptr = diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc index 6b9d8cc468..6f1ccb9cc3 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc @@ -491,7 +491,8 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_SUCCESS) { command->Run(); } -TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_Menu_WARNINGS) { +TEST_F(SetGlobalPropertiesRequestTest, + Run_VRWithMenuAndKeyboard_Menu_WARNINGS) { MessageSharedPtr msg = CreateMsgParams(); SmartObject vr_help_title("yes"); SmartObject vr_help_array(smart_objects::SmartType_Array); diff --git a/src/components/application_manager/src/message_helper/message_helper.cc b/src/components/application_manager/src/message_helper/message_helper.cc index 58b3552e9f..052d619fcc 100644 --- a/src/components/application_manager/src/message_helper/message_helper.cc +++ b/src/components/application_manager/src/message_helper/message_helper.cc @@ -94,8 +94,8 @@ bool ValidateSoftButtons(smart_objects::SmartObject& soft_buttons) { // Check if image parameter is valid if (button.keyExists(strings::image)) { SmartObject& buttonImage = button[strings::image]; - if (false == MessageHelper::VerifyString( - buttonImage[strings::value].asString())) { + if (false == + MessageHelper::VerifyString(buttonImage[strings::value].asString())) { return false; } } @@ -2598,11 +2598,9 @@ mobile_apis::Result::eType MessageHelper::VerifyImageFiles( return mobile_apis::Result::SUCCESS; } -void MessageHelper::ApplyImagePath( - smart_objects::SmartObject& image, - ApplicationConstSharedPtr app, - ApplicationManager& app_mngr) { - +void MessageHelper::ApplyImagePath(smart_objects::SmartObject& image, + ApplicationConstSharedPtr app, + ApplicationManager& app_mngr) { const std::string& file_name = image[strings::value].asString(); const std::string& full_file_path = GetAppFilePath(file_name, app, app_mngr); @@ -2669,19 +2667,17 @@ mobile_apis::Result::eType MessageHelper::VerifyImage( smart_objects::SmartObject& image, ApplicationConstSharedPtr app, ApplicationManager& app_mngr) { - const uint32_t image_type = image[strings::image_type].asUInt(); mobile_apis::ImageType::eType type = static_cast(image_type); const std::string& file_name = image[strings::value].asString(); - - if(!VerifyString(file_name)){ + if (!VerifyString(file_name)) { return mobile_apis::Result::INVALID_DATA; } if (mobile_apis::ImageType::STATIC == type) { - return mobile_apis::Result::SUCCESS; + return mobile_apis::Result::SUCCESS; } ApplyImagePath(image, app, app_mngr); diff --git a/src/components/application_manager/test/message_helper/message_helper_test.cc b/src/components/application_manager/test/message_helper/message_helper_test.cc index d8cc4329d3..15a09c33cc 100644 --- a/src/components/application_manager/test/message_helper/message_helper_test.cc +++ b/src/components/application_manager/test/message_helper/message_helper_test.cc @@ -869,10 +869,8 @@ TEST_F(MessageHelperTest, message[1][strings::image][strings::image_type] = mobile_apis::ImageType::STATIC; - message[0][strings::image][strings::value] = - "static_icon"; - message[1][strings::image][strings::value] = - "static_icon"; + message[0][strings::image][strings::value] = "static_icon"; + message[1][strings::image][strings::value] = "static_icon"; // Method call mobile_apis::Result::eType result = MessageHelper::VerifyImageVrHelpItems( message, appSharedMock, mock_application_manager); -- cgit v1.2.1 From 5f0bb3cd3d606adf9659e38e3348c8ed8afd3735 Mon Sep 17 00:00:00 2001 From: AKalinich-Luxoft Date: Thu, 26 Jul 2018 19:57:23 +0300 Subject: Answer Livio comments Fixed affected unit tests Reverted changes related to namespace naming --- .../create_interaction_choice_set_request.cc | 17 ++++--- .../commands/mobile/add_command_request_test.cc | 4 +- .../commands/mobile/send_location_request_test.cc | 2 +- .../commands/mobile/set_global_properties_test.cc | 59 ++++++++++++---------- .../test/commands/mobile/show_test.cc | 5 +- .../mobile/update_turn_list_request_test.cc | 2 +- 6 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc index 0612daa3e3..9b7653ac52 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc @@ -42,12 +42,13 @@ #include "utils/gen_hash.h" #include "utils/helpers.h" -#define INVALID_IMG_WARNING_INFO "Requested image(s) not found." -namespace sdl_rpc_plugin { -using namespace application_manager; +const char* kInvalidImageWarningInfo = "Requested image(s) not found."; +namespace sdl_rpc_plugin { namespace commands { +using namespace application_manager; + CreateInteractionChoiceSetRequest::CreateInteractionChoiceSetRequest( const application_manager::commands::MessageSharedPtr& message, ApplicationManager& application_manager, @@ -106,10 +107,10 @@ void CreateInteractionChoiceSetRequest::Run() { LOG4CXX_ERROR(logger_, "Image verification failed."); SendResponse(false, Result::INVALID_DATA); return; - } else if (verification_result_image == Result::WARNINGS || - verification_result_secondary_image == Result::WARNINGS) { - should_send_warnings = true; - break; + } else if (verification_result_image == Result::WARNINGS || + verification_result_secondary_image == Result::WARNINGS) { + should_send_warnings = true; + break; } } @@ -442,7 +443,7 @@ void CreateInteractionChoiceSetRequest::OnAllHMIResponsesReceived() { LOG4CXX_AUTO_TRACE(logger_); if (!error_from_hmi_ && should_send_warnings) { - SendResponse(true, mobile_apis::Result::WARNINGS,INVALID_IMG_WARNING_INFO); + SendResponse(true, mobile_apis::Result::WARNINGS, kInvalidImageWarningInfo); } else if (!error_from_hmi_) { SendResponse(true, mobile_apis::Result::SUCCESS); } else { diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc index fedb4966f5..dfcceea889 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc @@ -31,6 +31,7 @@ */ #include +#include #include #include @@ -64,7 +65,6 @@ using am::ApplicationManager; using am::commands::MessageSharedPtr; using am::ApplicationSharedPtr; using ::testing::_; - using ::testing::Return; using ::testing::InSequence; using sdl_rpc_plugin::commands::AddCommandRequest; @@ -278,7 +278,7 @@ TEST_F(AddCommandRequestTest, Run_ImageVerificationFailed_EXPECT_WARNINGS) { mock_rpc_service_, ManageHMICommand(HMIResultCodeIs(hmi_apis::FunctionID::UI_AddCommand))) .WillOnce(Return(true)); - utils::SharedPtr request_ptr = + std::shared_ptr request_ptr = CreateCommand(msg_); request_ptr->Run(); } diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc index e4144c0911..4f44293fc9 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/send_location_request_test.cc @@ -335,7 +335,7 @@ TEST_F(SendLocationRequestTest, Run_LocationImageInvalid_Cancelled) { (*message_)[strings::msg_params][strings::location_image] = SmartObject(smart_objects::SmartType_Map); (*message_)[strings::msg_params][strings::location_image][strings::value] = - " 1"; + "1"; EXPECT_CALL( mock_message_helper_, VerifyImage( diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc index 6f1ccb9cc3..03aa5d85a1 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/set_global_properties_test.cc @@ -31,6 +31,7 @@ */ #include +#include #include #include @@ -520,6 +521,10 @@ TEST_F(SetGlobalPropertiesRequestTest, EXPECT_CALL(*mock_app_, set_menu_icon(menu_icon)); EXPECT_CALL(*mock_app_, set_keyboard_props(keyboard_properties)); EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + EXPECT_CALL(*mock_app_, help_prompt_manager()) + .WillOnce(ReturnRef(*mock_help_prompt_manager_.get())); + EXPECT_CALL(*mock_help_prompt_manager_, + OnSetGlobalPropertiesReceived(_, false)); EXPECT_CALL( mock_hmi_interfaces_, @@ -530,7 +535,7 @@ TEST_F(SetGlobalPropertiesRequestTest, GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - SharedPtr command( + std::shared_ptr command( CreateCommand(msg)); command->Run(); @@ -564,6 +569,10 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_VR_WARNINGS) { EXPECT_CALL(*mock_app_, set_menu_icon(menu_icon)); EXPECT_CALL(*mock_app_, set_keyboard_props(keyboard_properties)); EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + EXPECT_CALL(*mock_app_, help_prompt_manager()) + .WillOnce(ReturnRef(*mock_help_prompt_manager_.get())); + EXPECT_CALL(*mock_help_prompt_manager_, + OnSetGlobalPropertiesReceived(_, false)); EXPECT_CALL( mock_hmi_interfaces_, @@ -574,7 +583,7 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRWithMenuAndKeyboard_VR_WARNINGS) { GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - SharedPtr command( + std::shared_ptr command( CreateCommand(msg)); command->Run(); @@ -593,9 +602,7 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRBrokenMenuIcon_Canceled) { EXPECT_CALL(mock_message_helper_, VerifyImage(menu_icon, _, _)) .WillOnce((Return(mobile_apis::Result::ABORTED))); EXPECT_CALL(mock_message_helper_, VerifyImageVrHelpItems(_, _, _)).Times(0); - EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(_)).Times(0); EmptyExpectationsSetupHelper(); - std::shared_ptr command( CreateCommand(msg)); @@ -610,14 +617,27 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRBrokenVRHelp_Canceled) { (*msg)[am::strings::msg_params][am::hmi_request::menu_icon] = menu_icon; SmartObject vr_help_array(smart_objects::SmartType_Array); VRArraySetupHelper(msg, vr_help_title, vr_help_array); + SmartObject menu_title("Menu_Title"); + (*msg)[am::strings::msg_params][am::hmi_request::menu_title] = menu_title; EXPECT_CALL(mock_message_helper_, VerifyImage(menu_icon, _, _)) .WillOnce((Return(mobile_apis::Result::SUCCESS))); EXPECT_CALL(mock_message_helper_, VerifyImageVrHelpItems(vr_help_array, _, _)) .WillOnce((Return(mobile_apis::Result::ABORTED))); - EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(_)).Times(0); EmptyExpectationsSetupHelper(); + EXPECT_CALL(*mock_app_, vr_help_title()).WillOnce(Return(&vr_help_title)); + EXPECT_CALL(*mock_app_, vr_help()).WillOnce(Return(&vr_help_array)); + EXPECT_CALL(*mock_app_, set_vr_help_title(vr_help_title)); + EXPECT_CALL(*mock_app_, set_vr_help(vr_help_array)); + EXPECT_CALL(*mock_app_, set_menu_title(menu_title)); + EXPECT_CALL(*mock_app_, set_menu_icon(menu_icon)); + EXPECT_CALL(*mock_app_, app_id()).WillOnce(Return(kAppId)); + EXPECT_CALL(*mock_app_, help_prompt_manager()) + .WillOnce(ReturnRef(*mock_help_prompt_manager_.get())); + EXPECT_CALL(*mock_help_prompt_manager_, + OnSetGlobalPropertiesReceived(_, false)); + std::shared_ptr command( CreateCommand(msg)); @@ -732,7 +752,7 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_NoVR_SUCCESS) { GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - SharedPtr command( + std::shared_ptr command( CreateCommand(msg)); command->Run(); @@ -749,16 +769,9 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_VRCouldNotGenerate_INVALID_DATA) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(kConnectionKey)); - SmartObject* vr_help_title = NULL; - CommandsMap commands_map; - SmartObject empty_msg(smart_objects::SmartType_Map); - commands_map.insert(std::pair(1u, &empty_msg)); - DataAccessor accessor(commands_map, lock_ptr_); - EXPECT_CALL(*mock_app_, commands_map()).WillOnce(Return(accessor)); - EXPECT_CALL(*mock_app_, vr_help_title()).WillOnce(Return(vr_help_title)); - EXPECT_CALL(*mock_app_, set_menu_title(_)).Times(0); + EXPECT_CALL(*mock_app_, set_menu_title(menu_title)); - SharedPtr command( + std::shared_ptr command( CreateCommand(msg)); command->Run(); @@ -815,7 +828,7 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_NoVRNoDataDefaultCreated_SUCCESS) { GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - SharedPtr command( + std::shared_ptr command( CreateCommand(msg)); command->Run(); @@ -830,23 +843,13 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_NoVRNoDataFromSynonyms_SUCCESS) { EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); EXPECT_CALL(app_mngr_, RemoveAppFromTTSGlobalPropertiesList(kConnectionKey)); - SmartObject vr_help_title(smart_objects::SmartType_Null); - EXPECT_CALL(*mock_app_, vr_help_title()) - .Times(2) - .WillRepeatedly(Return(&vr_help_title)); - - CommandsMap commands_map; - DataAccessor accessor(commands_map, lock_ptr_); - EXPECT_CALL(*mock_app_, commands_map()).WillOnce(Return(accessor)); + SmartObject vr_help_array(smart_objects::SmartType_Array); vr_help_array[0] = SmartObject(smart_objects::SmartType_Map); vr_help_array[0][am::strings::text] = kText; vr_help_array[0][am::strings::position] = kPosition; SmartObject vr_synonyms(smart_objects::SmartType_Array); vr_synonyms[0] = vr_help_array; - const CustomString name("name"); - EXPECT_CALL(*mock_app_, name()).WillOnce(ReturnRef(name)); - EXPECT_CALL(*mock_app_, set_vr_help_title(SmartObject(name))); EXPECT_CALL(*mock_app_, set_menu_title(_)).Times(0); EXPECT_CALL(*mock_app_, set_menu_icon(_)).Times(0); EXPECT_CALL(*mock_app_, set_keyboard_props(keyboard_properties)); @@ -858,7 +861,7 @@ TEST_F(SetGlobalPropertiesRequestTest, Run_NoVRNoDataFromSynonyms_SUCCESS) { ON_CALL(mock_hmi_interfaces_, GetInterfaceState(am::HmiInterfaces::HMI_INTERFACE_UI)) .WillByDefault(Return(am::HmiInterfaces::STATE_NOT_AVAILABLE)); - SharedPtr command( + std::shared_ptr command( CreateCommand(msg)); command->Run(); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc index 2d9be1b5cf..4725af4671 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/show_test.cc @@ -31,6 +31,7 @@ */ #include +#include #include #include @@ -362,7 +363,7 @@ TEST_F(ShowRequestTest, Run_Graphic_WARNINGS) { msg_params[am::strings::graphic] = graphic; (*msg)[am::strings::msg_params] = msg_params; - SharedPtr command(CreateCommand(msg)); + std::shared_ptr command(CreateCommand(msg)); EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); @@ -461,7 +462,7 @@ TEST_F(ShowRequestTest, Run_SecondaryGraphic_WARNINGS) { msg_params[am::strings::secondary_graphic] = graphic; (*msg)[am::strings::msg_params] = msg_params; - SharedPtr command(CreateCommand(msg)); + std::shared_ptr command(CreateCommand(msg)); EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillOnce(Return(mock_app_)); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc index d07e76350f..78fca8681a 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/update_turn_list_request_test.cc @@ -274,7 +274,7 @@ TEST_F(UpdateTurnListRequestTest, Run_ValidTurnList_WARNINGS) { SubscribeApplicationToSoftButton(_, _, kFunctionId)); MessageSharedPtr result_msg(CatchHMICommandResult(CallRun(*command_))); - ASSERT_TRUE(result_msg); + ASSERT_TRUE(result_msg != nullptr); EXPECT_EQ( hmi_apis::FunctionID::Navigation_UpdateTurnList, (*result_msg)[am::strings::params][am::strings::function_id].asInt()); -- cgit v1.2.1 From 8e8305da3c0f0cc72855416f2f846a5f932c38ca Mon Sep 17 00:00:00 2001 From: AKalinich-Luxoft Date: Fri, 27 Jul 2018 19:14:46 +0300 Subject: Fix defects found after ATF testing Apply feature for AddSubMenu RPC Fixed PI request does not resend choiceId Fixed wrong RPC creation in mobile factory --- .../src/commands/mobile/add_sub_menu_request.cc | 7 +--- .../commands/mobile/perform_interaction_request.cc | 48 ++++++++++++---------- .../sdl_rpc_plugin/src/mobile_command_factory.cc | 6 +-- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_sub_menu_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_sub_menu_request.cc index 3dcc414f9e..183b445326 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_sub_menu_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_sub_menu_request.cc @@ -76,7 +76,7 @@ void AddSubMenuRequest::Run() { verification_result = MessageHelper::VerifyImage( received_msg_params[strings::menu_icon], app, application_manager_); - if (mobile_apis::Result::SUCCESS != verification_result) { + if (mobile_apis::Result::INVALID_DATA == verification_result) { LOG4CXX_ERROR( logger_, "MessageHelper::VerifyImage return " << verification_result); SendResponse(false, verification_result); @@ -117,10 +117,7 @@ void AddSubMenuRequest::Run() { msg_params[strings::menu_params][strings::menu_name] = received_msg_params[strings::menu_name]; msg_params[strings::app_id] = app->app_id(); - - if (mobile_apis::Result::SUCCESS == verification_result) { - msg_params[strings::menu_icon] = received_msg_params[strings::menu_icon]; - } + msg_params[strings::menu_icon] = received_msg_params[strings::menu_icon]; StartAwaitForInterface(HmiInterfaces::HMI_INTERFACE_UI); SendHMIRequest(hmi_apis::FunctionID::UI_AddSubMenu, &msg_params, true); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc index 76dd30525a..ec06f0bb31 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc @@ -343,27 +343,31 @@ bool PerformInteractionRequest::ProcessVRResponse( return false; } - if (Common_Result::SUCCESS == vr_result_code_ && - InteractionMode::MANUAL_ONLY == interaction_mode_) { - LOG4CXX_DEBUG(logger_, - "VR response SUCCESS in MANUAL_ONLY mode " - << "Wait for UI response"); - // in case MANUAL_ONLY mode VR.PI SUCCESS just return - return false; - } - const SmartObject& hmi_msg_params = message[strings::msg_params]; if (hmi_msg_params.keyExists(strings::choice_id)) { - const int choise_id = hmi_msg_params[strings::choice_id].asInt(); - if (!CheckChoiceIDFromResponse(app, choise_id)) { + const int choice_id = hmi_msg_params[strings::choice_id].asInt(); + if (!CheckChoiceIDFromResponse(app, choice_id)) { LOG4CXX_ERROR(logger_, "Wrong choiceID was received from HMI"); TerminatePerformInteraction(); SendResponse( false, Result::GENERIC_ERROR, "Wrong choiceID was received from HMI"); return true; } - msg_params[strings::choice_id] = choise_id; + msg_params[strings::choice_id] = choice_id; + } + + const bool is_vr_result_success = Compare( + vr_result_code_, Common_Result::SUCCESS, Common_Result::WARNINGS); + + if (is_vr_result_success && + InteractionMode::MANUAL_ONLY == interaction_mode_) { + LOG4CXX_DEBUG(logger_, + "VR response is successfull in MANUAL_ONLY mode " + << "Wait for UI response"); + // in case MANUAL_ONLY mode VR.PI SUCCESS just return + return false; } + return false; } @@ -401,19 +405,19 @@ void PerformInteractionRequest::ProcessUIResponse( ui_result_code_, hmi_apis::Common_Result::UNSUPPORTED_RESOURCE); if (result) { - if (is_pi_warning) { - ui_result_code_ = hmi_apis::Common_Result::WARNINGS; - ui_info_ = message[strings::msg_params][strings::info].asString(); - if (message.keyExists(strings::params) && - message[strings::params].keyExists(strings::data)) { - msg_params = message[strings::params][strings::data]; - } - } else if (is_pi_unsupported) { + if (is_pi_unsupported) { ui_result_code_ = hmi_apis::Common_Result::UNSUPPORTED_RESOURCE; ui_info_ = message[strings::msg_params][strings::info].asString(); - } else if (message.keyExists(strings::msg_params)) { - msg_params = message[strings::msg_params]; + } else { + if (message.keyExists(strings::msg_params)) { + msg_params = message[strings::msg_params]; + } + if (is_pi_warning) { + ui_result_code_ = hmi_apis::Common_Result::WARNINGS; + ui_info_ = message[strings::msg_params][strings::info].asString(); + } } + // result code must be GENERIC_ERROR in case wrong choice_id if (msg_params.keyExists(strings::choice_id)) { if (!CheckChoiceIDFromResponse(app, diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/mobile_command_factory.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/mobile_command_factory.cc index 3d7e2f6437..4251eeadc9 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/mobile_command_factory.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/mobile_command_factory.cc @@ -263,7 +263,7 @@ CommandCreator& MobileCommandFactory::get_creator_factory( } case mobile_apis::FunctionID::GetWayPointsID: { return mobile_api::messageType::request == message_type - ? factory.GetCreator() + ? factory.GetCreator() : factory.GetCreator(); } case mobile_apis::FunctionID::SubscribeWayPointsID: { @@ -273,8 +273,8 @@ CommandCreator& MobileCommandFactory::get_creator_factory( } case mobile_apis::FunctionID::UnsubscribeWayPointsID: { return mobile_api::messageType::request == message_type - ? factory.GetCreator() - : factory.GetCreator(); + ? factory.GetCreator() + : factory.GetCreator(); } case mobile_apis::FunctionID::GetSystemCapabilityID: { return mobile_api::messageType::request == message_type -- cgit v1.2.1 From c652b54ea3224f789e04453f778cdd41b96b435e Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Mon, 30 Jul 2018 15:18:14 -0400 Subject: Return correct pointer type in ParseArray() --- src/components/policy/policy_regular/src/policy_manager_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/policy/policy_regular/src/policy_manager_impl.cc b/src/components/policy/policy_regular/src/policy_manager_impl.cc index 297d538165..ebc695142e 100644 --- a/src/components/policy/policy_regular/src/policy_manager_impl.cc +++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc @@ -116,9 +116,9 @@ std::shared_ptr PolicyManagerImpl::ParseArray( // For PT Update received from SDL Server. if (value["data"].size() != 0) { Json::Value data = value["data"]; - return new policy_table::Table(&data[0]); + return std::make_shared(&data[0]); } else { - return new policy_table::Table(&value); + return std::make_shared(&value); } } else { return std::shared_ptr(); -- cgit v1.2.1 From 4d6a095eac495e7b9a4b2c7522f358eeae03271c Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Mon, 30 Jul 2018 17:15:36 -0400 Subject: remove Lock(bool) constructors --- .../policy/policy_external/include/policy/cache_manager.h | 2 +- .../policy/policy_external/include/policy/update_status_manager.h | 2 +- src/components/policy/policy_external/src/cache_manager.cc | 6 ++---- src/components/policy/policy_external/src/update_status_manager.cc | 1 - 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/components/policy/policy_external/include/policy/cache_manager.h b/src/components/policy/policy_external/include/policy/cache_manager.h index 6c7bbeb9c9..d30e7cea24 100644 --- a/src/components/policy/policy_external/include/policy/cache_manager.h +++ b/src/components/policy/policy_external/include/policy/cache_manager.h @@ -789,7 +789,7 @@ class CacheManager : public CacheManagerInterface { typedef std::set UnpairedDevices; UnpairedDevices is_unpaired_; - mutable sync_primitives::Lock cache_lock_; + mutable sync_primitives::RecursiveLock cache_lock_; sync_primitives::Lock unpaired_lock_; typedef std::map AppCalculatedPermissions; diff --git a/src/components/policy/policy_external/include/policy/update_status_manager.h b/src/components/policy/policy_external/include/policy/update_status_manager.h index cdd1225ef7..b4a373d1cf 100644 --- a/src/components/policy/policy_external/include/policy/update_status_manager.h +++ b/src/components/policy/policy_external/include/policy/update_status_manager.h @@ -227,7 +227,7 @@ class UpdateStatusManager { volatile uint32_t timeout_; volatile bool stop_flag_; - sync_primitives::Lock state_lock_; + sync_primitives::RecursiveLock state_lock_; sync_primitives::ConditionalVariable termination_condition_; UpdateStatusManager* update_status_manager_; }; diff --git a/src/components/policy/policy_external/src/cache_manager.cc b/src/components/policy/policy_external/src/cache_manager.cc index cc9921be14..717baa0443 100644 --- a/src/components/policy/policy_external/src/cache_manager.cc +++ b/src/components/policy/policy_external/src/cache_manager.cc @@ -253,8 +253,7 @@ CacheManager::CacheManager() : CacheManagerInterface() , pt_(new policy_table::Table) , backup_(new SQLPTExtRepresentation()) - , update_required(false) - , cache_lock_(true) { + , update_required(false) { InitBackupThread(); } @@ -262,8 +261,7 @@ CacheManager::CacheManager(bool in_memory) : CacheManagerInterface() , pt_(new policy_table::Table) , backup_(new SQLPTExtRepresentation(in_memory)) - , update_required(false) - , cache_lock_(true) { + , update_required(false) { InitBackupThread(); } diff --git a/src/components/policy/policy_external/src/update_status_manager.cc b/src/components/policy/policy_external/src/update_status_manager.cc index 1f00a440b3..e738554799 100644 --- a/src/components/policy/policy_external/src/update_status_manager.cc +++ b/src/components/policy/policy_external/src/update_status_manager.cc @@ -209,7 +209,6 @@ UpdateStatusManager::UpdateThreadDelegate::UpdateThreadDelegate( UpdateStatusManager* update_status_manager) : timeout_(0) , stop_flag_(false) - , state_lock_(true) , update_status_manager_(update_status_manager) { LOG4CXX_AUTO_TRACE(logger_); LOG4CXX_DEBUG(logger_, "Create UpdateThreadDelegate"); -- cgit v1.2.1 From 1f053a1ef1b6240e18f890959817bcac073a6c7b Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 25 Jul 2018 15:54:58 +0300 Subject: Set absolute path for linking boost libraries to utils Specify boost libraries search path for linkage --- src/appMain/CMakeLists.txt | 3 ++- src/components/hmi_message_handler/test/CMakeLists.txt | 2 +- src/components/utils/CMakeLists.txt | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/appMain/CMakeLists.txt b/src/appMain/CMakeLists.txt index a05f4f97a1..b821c94260 100644 --- a/src/appMain/CMakeLists.txt +++ b/src/appMain/CMakeLists.txt @@ -141,7 +141,8 @@ add_executable(${PROJECT} ${SOURCES}) if (HMIADAPTER STREQUAL "messagebroker") add_dependencies(${PROJECT} Boost) - list(APPEND LIBRARIES libboost_system.so) + GET_PROPERTY(BOOST_LIBS_DIRECTORY GLOBAL PROPERTY GLOBAL_BOOST_LIBS) + list(APPEND LIBRARIES boost_system -L${BOOST_LIBS_DIRECTORY}) endif() target_link_libraries(${PROJECT} ${LIBRARIES}) diff --git a/src/components/hmi_message_handler/test/CMakeLists.txt b/src/components/hmi_message_handler/test/CMakeLists.txt index 9e855c563e..6d30d6ee51 100644 --- a/src/components/hmi_message_handler/test/CMakeLists.txt +++ b/src/components/hmi_message_handler/test/CMakeLists.txt @@ -56,7 +56,7 @@ collect_sources(SOURCES "${CMAKE_CURRENT_SOURCE_DIR}" "${EXCLUDE_PATHS}") if (HMIADAPTER STREQUAL "messagebroker") GET_PROPERTY(BOOST_LIBS_DIRECTORY GLOBAL PROPERTY GLOBAL_BOOST_LIBS) - list(APPEND LIBRARIES ${BOOST_LIBS_DIRECTORY}/libboost_system.so) + list(APPEND LIBRARIES boost_system -L${BOOST_LIBS_DIRECTORY}) endif() create_test(hmi_message_handler_test "${SOURCES}" "${LIBRARIES}") diff --git a/src/components/utils/CMakeLists.txt b/src/components/utils/CMakeLists.txt index 0eb64ae8c2..ed90c6fb45 100644 --- a/src/components/utils/CMakeLists.txt +++ b/src/components/utils/CMakeLists.txt @@ -114,7 +114,9 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Linux") endif() add_library("Utils" ${SOURCES}) -list(APPEND LIBRARIES -lboost_system -lboost_thread) +GET_PROPERTY(BOOST_LIBS_DIRECTORY GLOBAL PROPERTY GLOBAL_BOOST_LIBS) +list(APPEND LIBRARIES boost_system -L${BOOST_LIBS_DIRECTORY}) +list(APPEND LIBRARIES boost_thread -L${BOOST_LIBS_DIRECTORY}) target_link_libraries("Utils" ${LIBRARIES}) add_dependencies("Utils" Boost) -- cgit v1.2.1 From f52143d6b097dcd219ca7c2178da1d5d4f83457e Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 1 Aug 2018 16:47:48 -0400 Subject: Make iconResumed optional --- src/components/interfaces/MOBILE_API.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/interfaces/MOBILE_API.xml b/src/components/interfaces/MOBILE_API.xml index 97bc33ceae..185976a48b 100644 --- a/src/components/interfaces/MOBILE_API.xml +++ b/src/components/interfaces/MOBILE_API.xml @@ -3410,7 +3410,7 @@ The software version of the system that implements the SmartDeviceLink core. - + Existence of apps icon at system. If true, apps icon was resumed at system. If false, apps icon is not resumed at system -- cgit v1.2.1 From e41e1dc062231f947d927b19c1cea7fae991fd49 Mon Sep 17 00:00:00 2001 From: Conlain Kelly Date: Wed, 1 Aug 2018 16:52:03 -0400 Subject: formatting cleanup --- src/components/interfaces/MOBILE_API.xml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/interfaces/MOBILE_API.xml b/src/components/interfaces/MOBILE_API.xml index 185976a48b..b69e88c936 100644 --- a/src/components/interfaces/MOBILE_API.xml +++ b/src/components/interfaces/MOBILE_API.xml @@ -3411,8 +3411,10 @@ The software version of the system that implements the SmartDeviceLink core. - Existence of apps icon at system. If true, apps icon - was resumed at system. If false, apps icon is not resumed at system + + Existence of apps icon at system. If true, apps icon + was resumed at system. If false, apps icon is not resumed at system + -- cgit v1.2.1