From dae80d699d379a9a05d24b9625f988fa6a8d9259 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 13 Oct 2015 18:32:03 +0300 Subject: [PATCH 1/7] Using english in TTS if sound.txt doesn't contain the current language. Some refactoring. --- map/framework.hpp | 5 ++++ platform/get_text_by_id.cpp | 56 +++++++++++++++++++++++++++++-------- platform/get_text_by_id.hpp | 26 ++++++++++++----- routing/turns_tts_text.cpp | 22 ++++++++++----- routing/turns_tts_text.hpp | 6 ++-- 5 files changed, 86 insertions(+), 29 deletions(-) diff --git a/map/framework.hpp b/map/framework.hpp index 2d30557cd8..c6799e26a7 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -603,7 +603,12 @@ public: // Sound notifications for turn instructions. inline void EnableTurnNotifications(bool enable) { m_routingSession.EnableTurnNotifications(enable); } inline bool AreTurnNotificationsEnabled() const { return m_routingSession.AreTurnNotificationsEnabled(); } + /// \brief Sets a locale for TTS. + /// \param locale is a string with locale code. For example "en", "ru", "zh-Hant" and so on. + /// See sound/tts/languages.txt for the full list. inline void SetTurnNotificationsLocale(string const & locale) { m_routingSession.SetTurnNotificationsLocale(locale); } + /// @return current TTS locale. For example "en", "ru", "zh-Hant" and so on. + /// In case of error returns an empty string. inline string GetTurnNotificationsLocale() const { return m_routingSession.GetTurnNotificationsLocale(); } /// \brief When an end user is going to a turn he gets sound turn instructions. /// If C++ part wants the client to pronounce an instruction GenerateTurnSound (in turnNotifications) returns diff --git a/platform/get_text_by_id.cpp b/platform/get_text_by_id.cpp index 9abda6c21d..a14da57200 100644 --- a/platform/get_text_by_id.cpp +++ b/platform/get_text_by_id.cpp @@ -11,6 +11,8 @@ namespace { +string const kDefaultLanguage = "en"; + string GetTextSourceString(platform::TextSource textSouce) { #if defined(OMIM_OS_ANDROID) || defined(OMIM_OS_IPHONE) @@ -28,30 +30,62 @@ string GetTextSourceString(platform::TextSource textSouce) #endif ASSERT(false, ()); } -} // namespace -namespace platform -{ -GetTextById::GetTextById(TextSource textSouce, string const & localeName) +bool GetJsonBuffer(platform::TextSource textSouce, string const & localeName, string & jsonBuffer) { string const pathToJson = my::JoinFoldersToPath( {GetTextSourceString(textSouce), localeName + ".json"}, "localize.json"); - // @TODO(vbykoianko) Add assert if locale path pathToJson is not valid. - string jsonBuffer; try { + jsonBuffer.clear(); GetPlatform().GetReader(pathToJson)->ReadAsString(jsonBuffer); } catch (RootException const & ex) { - LOG(LWARNING, ("Can't open sound instructions file", pathToJson, ex.what())); - return; + LOG(LWARNING, ("Can't open", localeName,"sound instructions file. pathToJson is", + pathToJson, ex.what())); + return false; // No json file for localeName } - InitFromJson(jsonBuffer); + return true; +} +} // namespace + +namespace platform +{ +unique_ptr MakeGetTextById(string const & jsonBuffer, string const & localeName) +{ + unique_ptr result(new GetTextById(jsonBuffer, localeName)); + if (!result || !result->IsValid()) + { + ASSERT(false, ()); + return nullptr; + } + return result; } -GetTextById::GetTextById(string const & jsonBuffer) { InitFromJson(jsonBuffer); } +unique_ptr GetTextByIdFactory(TextSource textSouce, string const & localeName) +{ + string jsonBuffer; + if (GetJsonBuffer(textSouce, localeName, jsonBuffer)) + return MakeGetTextById(jsonBuffer, localeName); + + if (GetJsonBuffer(textSouce, kDefaultLanguage, jsonBuffer)) + return MakeGetTextById(jsonBuffer, kDefaultLanguage); + + ASSERT(false, ("sound.txt does not contain default language.")); + return nullptr; +} + +unique_ptr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName) +{ + return MakeGetTextById(jsonBuffer, localeName); +} + +GetTextById::GetTextById(string const & jsonBuffer, string const & localeName) : m_locale(localeName) +{ + InitFromJson(jsonBuffer); +} void GetTextById::InitFromJson(string const & jsonBuffer) { @@ -84,7 +118,7 @@ void GetTextById::InitFromJson(string const & jsonBuffer) string GetTextById::operator()(string const & textId) const { if (!IsValid()) - return ""; + return string(); auto const textIt = m_localeTexts.find(textId); if (textIt == m_localeTexts.end()) diff --git a/platform/get_text_by_id.hpp b/platform/get_text_by_id.hpp index 7d1c5cac50..50c5184a46 100644 --- a/platform/get_text_by_id.hpp +++ b/platform/get_text_by_id.hpp @@ -1,6 +1,7 @@ #pragma once #include "std/string.hpp" +#include "std/unique_ptr.hpp" #include "std/unordered_map.hpp" #include "std/utility.hpp" @@ -18,19 +19,30 @@ enum class TextSource class GetTextById { public: - GetTextById(TextSource textSouce, string const & localeName); - /// The constructor is used for writing unit tests only. - explicit GetTextById(string const & jsonBuffer); - - bool IsValid() const { return !m_localeTexts.empty(); } /// @return a pair of a text string in a specified locale for textId and a boolean flag. /// If textId is found in m_localeTexts then the boolean flag is set to true. /// The boolean flag is set to false otherwise. string operator()(string const & textId) const; - + string GetLocale() const { return m_locale; } private: - void InitFromJson(string const & jsonBuffer); + friend unique_ptr GetTextByIdFactory(TextSource textSouce, string const & localeName); + friend unique_ptr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); + friend unique_ptr MakeGetTextById(string const & jsonBuffer, string const & localeName); + GetTextById(string const & jsonBuffer, string const & localeName); + void InitFromJson(string const & jsonBuffer); + /// \note IsValid is used only in factories and shall be private. + bool IsValid() const { return !m_localeTexts.empty(); } + + string m_locale; unordered_map m_localeTexts; }; + +/// Factoris to create intances of GetTextById. +/// If unique_ptr is created by GetTextByIdFactory or ForTestingGetTextByIdFactory +/// threre are only two possibities. +/// * a factory returns a valid instance +/// * a factory returns nullptr +unique_ptr GetTextByIdFactory(TextSource textSouce, string const & localeName); +unique_ptr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); } // namespace platform diff --git a/routing/turns_tts_text.cpp b/routing/turns_tts_text.cpp index 25caeabd75..cb2e302d32 100644 --- a/routing/turns_tts_text.cpp +++ b/routing/turns_tts_text.cpp @@ -36,16 +36,14 @@ namespace sound { void GetTtsText::SetLocale(string const & locale) { - m_locale = locale; - m_getCurLang.reset(new platform::GetTextById(platform::TextSource::TtsSound, locale)); - /// @todo Factor out file check from constructor and do not create m_getCurLang object in case of error. + m_getCurLang = platform::GetTextByIdFactory(platform::TextSource::TtsSound, locale); ASSERT(m_getCurLang, ()); } -void GetTtsText::ForTestingSetLocaleWithJson(string const & jsonBuffer) +void GetTtsText::ForTestingSetLocaleWithJson(string const & jsonBuffer, string const & locale) { - m_getCurLang.reset(new platform::GetTextById(jsonBuffer)); - ASSERT(m_getCurLang && m_getCurLang->IsValid(), ()); + m_getCurLang = platform::ForTestingGetTextByIdFactory(jsonBuffer, locale); + ASSERT(m_getCurLang, ()); } string GetTtsText::operator()(Notification const & notification) const @@ -63,11 +61,21 @@ string GetTtsText::operator()(Notification const & notification) const return distStr + " " + dirStr; } +string GetTtsText::GetLocale() const +{ + if (m_getCurLang) + { + ASSERT(false, ()); + return string(); + } + return m_getCurLang->GetLocale(); +} + string GetTtsText::GetTextById(string const & textId) const { ASSERT(!textId.empty(), ()); - if (!m_getCurLang || !m_getCurLang->IsValid()) + if (!m_getCurLang) { ASSERT(false, ()); return ""; diff --git a/routing/turns_tts_text.hpp b/routing/turns_tts_text.hpp index 43390e7463..880da596c7 100644 --- a/routing/turns_tts_text.hpp +++ b/routing/turns_tts_text.hpp @@ -22,16 +22,14 @@ class GetTtsText { public: string operator()(Notification const & notification) const; - /// TODO(vbykoianko) Check if locale is available. If not use default (en) locale. void SetLocale(string const & locale); - inline string GetLocale() const { return m_locale; } - void ForTestingSetLocaleWithJson(string const & jsonBuffer); + string GetLocale() const; + void ForTestingSetLocaleWithJson(string const & jsonBuffer, string const & locale); private: string GetTextById(string const & textId) const; unique_ptr m_getCurLang; - string m_locale; }; /// Generates text message id about the distance of the notification. For example: In 300 meters. From 91b9d32760e8bf4c799dc79412137c8d42198ea9 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 13 Oct 2015 18:33:41 +0300 Subject: [PATCH 2/7] Fixing unit tests. --- .../platform_tests/get_text_by_id_tests.cpp | 90 +++++++++---------- routing/routing_tests/turns_sound_test.cpp | 10 +-- .../routing_tests/turns_tts_text_tests.cpp | 4 +- 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/platform/platform_tests/get_text_by_id_tests.cpp b/platform/platform_tests/get_text_by_id_tests.cpp index 32f5fd1b45..b1eff8f3a9 100644 --- a/platform/platform_tests/get_text_by_id_tests.cpp +++ b/platform/platform_tests/get_text_by_id_tests.cpp @@ -15,16 +15,16 @@ UNIT_TEST(GetTextByIdEnglishTest) \"in_1_mile\":\"In one mile.\"\ }"; - platform::GetTextById getEnglish(shortJson); - TEST_EQUAL(getEnglish("make_a_slight_right_turn"), "Make a slight right turn.", ()); - TEST_EQUAL(getEnglish("in_900_meters"), "In nine hundred meters.", ()); - TEST_EQUAL(getEnglish("then"), "Then.", ()); - TEST_EQUAL(getEnglish("in_1_mile"), "In one mile.", ()); + unique_ptr getEnglish = platform::ForTestingGetTextByIdFactory(shortJson, "en"); + TEST_EQUAL((*getEnglish)("make_a_slight_right_turn"), "Make a slight right turn.", ()); + TEST_EQUAL((*getEnglish)("in_900_meters"), "In nine hundred meters.", ()); + TEST_EQUAL((*getEnglish)("then"), "Then.", ()); + TEST_EQUAL((*getEnglish)("in_1_mile"), "In one mile.", ()); - TEST_EQUAL(getEnglish("some_nonexistent_key"), "", ()); - TEST_EQUAL(getEnglish("some_nonexistent_key"), "", ()); - TEST_EQUAL(getEnglish(""), "", ()); - TEST_EQUAL(getEnglish(" "), "", ()); + TEST_EQUAL((*getEnglish)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getEnglish)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getEnglish)(""), "", ()); + TEST_EQUAL((*getEnglish)(" "), "", ()); } UNIT_TEST(GetTextByIdRussianTest) @@ -38,16 +38,16 @@ UNIT_TEST(GetTextByIdRussianTest) \"in_1_mile\":\"Через одну милю.\"\ }"; - platform::GetTextById getRussian(shortJson); - TEST_EQUAL(getRussian("in_800_meters"), "Через восемьсот метров.", ()); - TEST_EQUAL(getRussian("make_a_slight_right_turn"), "Плавный поворот направо.", ()); - TEST_EQUAL(getRussian("take_the_6th_exit"), "Шестой поворот с кольца.", ()); - TEST_EQUAL(getRussian("in_1_mile"), "Через одну милю.", ()); + unique_ptr getRussian = platform::ForTestingGetTextByIdFactory(shortJson, "ru"); + TEST_EQUAL((*getRussian)("in_800_meters"), "Через восемьсот метров.", ()); + TEST_EQUAL((*getRussian)("make_a_slight_right_turn"), "Плавный поворот направо.", ()); + TEST_EQUAL((*getRussian)("take_the_6th_exit"), "Шестой поворот с кольца.", ()); + TEST_EQUAL((*getRussian)("in_1_mile"), "Через одну милю.", ()); - TEST_EQUAL(getRussian("some_nonexistent_key"), "", ()); - TEST_EQUAL(getRussian("some_nonexistent_key"), "", ()); - TEST_EQUAL(getRussian(""), "", ()); - TEST_EQUAL(getRussian(" "), "", ()); + TEST_EQUAL((*getRussian)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getRussian)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getRussian)(""), "", ()); + TEST_EQUAL((*getRussian)(" "), "", ()); } UNIT_TEST(GetTextByIdKoreanTest) @@ -61,16 +61,16 @@ UNIT_TEST(GetTextByIdKoreanTest) \"in_5000_feet\":\"5000피트 앞\"\ }"; - platform::GetTextById getKorean(shortJson); - TEST_EQUAL(getKorean("in_700_meters"), "700 미터 앞", ()); - TEST_EQUAL(getKorean("make_a_right_turn"), "우회전입니다.", ()); - TEST_EQUAL(getKorean("take_the_5th_exit"), "다섯 번째 출구입니다.", ()); - TEST_EQUAL(getKorean("in_5000_feet"), "5000피트 앞", ()); + unique_ptr getKorean = platform::ForTestingGetTextByIdFactory(shortJson, "ko"); + TEST_EQUAL((*getKorean)("in_700_meters"), "700 미터 앞", ()); + TEST_EQUAL((*getKorean)("make_a_right_turn"), "우회전입니다.", ()); + TEST_EQUAL((*getKorean)("take_the_5th_exit"), "다섯 번째 출구입니다.", ()); + TEST_EQUAL((*getKorean)("in_5000_feet"), "5000피트 앞", ()); - TEST_EQUAL(getKorean("some_nonexistent_key"), "", ()); - TEST_EQUAL(getKorean("some_nonexistent_key"), "", ()); - TEST_EQUAL(getKorean(""), "", ()); - TEST_EQUAL(getKorean(" "), "", ()); + TEST_EQUAL((*getKorean)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getKorean)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getKorean)(""), "", ()); + TEST_EQUAL((*getKorean)(" "), "", ()); } UNIT_TEST(GetTextByIdArabicTest) @@ -84,16 +84,16 @@ UNIT_TEST(GetTextByIdArabicTest) \"in_4000_feet\":\"بعد 4000 قدم\"\ }"; - platform::GetTextById getArabic(shortJson); - TEST_EQUAL(getArabic("in_1_kilometer"), "بعد كيلو متر واحدٍ", ()); - TEST_EQUAL(getArabic("leave_the_roundabout"), "اخرج من الطريق الدوار", ()); - TEST_EQUAL(getArabic("take_the_3rd_exit"), "اسلك المخرج الثالث", ()); - TEST_EQUAL(getArabic("in_4000_feet"), "بعد 4000 قدم", ()); + unique_ptr getArabic = platform::ForTestingGetTextByIdFactory(shortJson, "ar"); + TEST_EQUAL((*getArabic)("in_1_kilometer"), "بعد كيلو متر واحدٍ", ()); + TEST_EQUAL((*getArabic)("leave_the_roundabout"), "اخرج من الطريق الدوار", ()); + TEST_EQUAL((*getArabic)("take_the_3rd_exit"), "اسلك المخرج الثالث", ()); + TEST_EQUAL((*getArabic)("in_4000_feet"), "بعد 4000 قدم", ()); - TEST_EQUAL(getArabic("some_nonexistent_key"), "", ()); - TEST_EQUAL(getArabic("some_nonexistent_key"), "", ()); - TEST_EQUAL(getArabic(""), "", ()); - TEST_EQUAL(getArabic(" "), "", ()); + TEST_EQUAL((*getArabic)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getArabic)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getArabic)(""), "", ()); + TEST_EQUAL((*getArabic)(" "), "", ()); } UNIT_TEST(GetTextByIdFrenchTest) @@ -107,14 +107,14 @@ UNIT_TEST(GetTextByIdFrenchTest) \"in_3500_feet\":\"Dans trois mille cinq cents pieds.\"\ }"; - platform::GetTextById getFrench(shortJson); - TEST_EQUAL(getFrench("in_1_5_kilometers"), "Dans un virgule cinq kilomètre.", ()); - TEST_EQUAL(getFrench("enter_the_roundabout"), "Prenez le rond-point.", ()); - TEST_EQUAL(getFrench("take_the_2nd_exit"), "Prenez la deuxième sortie.", ()); - TEST_EQUAL(getFrench("in_3500_feet"), "Dans trois mille cinq cents pieds.", ()); + unique_ptr getFrench = platform::ForTestingGetTextByIdFactory(shortJson, "fr"); + TEST_EQUAL((*getFrench)("in_1_5_kilometers"), "Dans un virgule cinq kilomètre.", ()); + TEST_EQUAL((*getFrench)("enter_the_roundabout"), "Prenez le rond-point.", ()); + TEST_EQUAL((*getFrench)("take_the_2nd_exit"), "Prenez la deuxième sortie.", ()); + TEST_EQUAL((*getFrench)("in_3500_feet"), "Dans trois mille cinq cents pieds.", ()); - TEST_EQUAL(getFrench("some_nonexistent_key"), "", ()); - TEST_EQUAL(getFrench("some_nonexistent_key"), "", ()); - TEST_EQUAL(getFrench(""), "", ()); - TEST_EQUAL(getFrench(" "), "", ()); + TEST_EQUAL((*getFrench)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getFrench)("some_nonexistent_key"), "", ()); + TEST_EQUAL((*getFrench)(""), "", ()); + TEST_EQUAL((*getFrench)(" "), "", ()); } diff --git a/routing/routing_tests/turns_sound_test.cpp b/routing/routing_tests/turns_sound_test.cpp index e8b6d9843d..9d3eee53eb 100644 --- a/routing/routing_tests/turns_sound_test.cpp +++ b/routing/routing_tests/turns_sound_test.cpp @@ -98,7 +98,7 @@ UNIT_TEST(TurnsSoundMetersTest) \"in_600_meters\":\"In 600 meters.\",\ \"make_a_right_turn\":\"Make a right turn.\"\ }"; - turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson); + turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson, "en"); turnSound.m_settings.ForTestingSetNotificationTimeSecond(20); turnSound.Reset(); @@ -185,7 +185,7 @@ UNIT_TEST(TurnsSoundMetersTwoTurnsTest) \"make_a_sharp_right_turn\":\"Make a sharp right turn.\",\ \"enter_the_roundabout\":\"Enter the roundabout.\"\ }"; - turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson); + turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson, "en"); turnSound.m_settings.ForTestingSetNotificationTimeSecond(20); turnSound.Reset(); @@ -257,7 +257,7 @@ UNIT_TEST(TurnsSoundFeetTest) \"in_2000_feet\":\"In 2000 feet.\",\ \"enter_the_roundabout\":\"Enter the roundabout.\"\ }"; - turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson); + turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson, "en"); turnSound.m_settings.ForTestingSetNotificationTimeSecond(20); turnSound.Reset(); @@ -341,7 +341,7 @@ UNIT_TEST(TurnsSoundComposedTurnTest) \"then\":\"Then.\",\ \"you_have_reached_the_destination\":\"You have reached the destination.\"\ }"; - turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson); + turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson, "en"); turnSound.m_settings.ForTestingSetNotificationTimeSecond(30); turnSound.Reset(); @@ -412,7 +412,7 @@ UNIT_TEST(TurnsSoundRoundaboutTurnTest) \"in_600_meters\":\"In 600 meters.\",\ \"then\":\"Then.\"\ }"; - turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson); + turnSound.m_getTtsText.ForTestingSetLocaleWithJson(engShortJson, "en"); turnSound.m_settings.ForTestingSetNotificationTimeSecond(30); turnSound.Reset(); diff --git a/routing/routing_tests/turns_tts_text_tests.cpp b/routing/routing_tests/turns_tts_text_tests.cpp index b566df4932..e1661a43dc 100644 --- a/routing/routing_tests/turns_tts_text_tests.cpp +++ b/routing/routing_tests/turns_tts_text_tests.cpp @@ -85,13 +85,13 @@ UNIT_TEST(GetTtsTextTest) ::Settings::Metric); Notification const notifiation4(0, 0, true, TurnDirection::TurnLeft, ::Settings::Metric); - getTtsText.ForTestingSetLocaleWithJson(engShortJson); + getTtsText.ForTestingSetLocaleWithJson(engShortJson, "en"); TEST_EQUAL(getTtsText(notifiation1), "In 500 meters. Make a right turn.", ()); TEST_EQUAL(getTtsText(notifiation2), "In 300 meters. Make a left turn.", ()); TEST_EQUAL(getTtsText(notifiation3), "You have reached the destination.", ()); TEST_EQUAL(getTtsText(notifiation4), "Then. Make a left turn.", ()); - getTtsText.ForTestingSetLocaleWithJson(rusShortJson); + getTtsText.ForTestingSetLocaleWithJson(rusShortJson, "ru"); TEST_EQUAL(getTtsText(notifiation1), "Через 500 метров. Поворот направо.", ()); TEST_EQUAL(getTtsText(notifiation2), "Через 300 метров. Поворот налево.", ()); TEST_EQUAL(getTtsText(notifiation3), "Вы достигли конца маршрута.", ()); From fc2898bf5129154bbfd323e38a0d5b049dab494e Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Oct 2015 10:31:32 +0300 Subject: [PATCH 3/7] Corrections after colleagues comments. --- map/framework.hpp | 4 ++- platform/get_text_by_id.cpp | 29 +++++++++---------- platform/get_text_by_id.hpp | 19 +++++++----- .../platform_tests/get_text_by_id_tests.cpp | 10 +++---- routing/turns_tts_text.cpp | 2 -- routing/turns_tts_text.hpp | 7 +++++ 6 files changed, 39 insertions(+), 32 deletions(-) diff --git a/map/framework.hpp b/map/framework.hpp index c6799e26a7..fa96242462 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -605,10 +605,12 @@ public: inline bool AreTurnNotificationsEnabled() const { return m_routingSession.AreTurnNotificationsEnabled(); } /// \brief Sets a locale for TTS. /// \param locale is a string with locale code. For example "en", "ru", "zh-Hant" and so on. - /// See sound/tts/languages.txt for the full list. + /// \note ee sound/tts/languages.txt for the full list of available locales. inline void SetTurnNotificationsLocale(string const & locale) { m_routingSession.SetTurnNotificationsLocale(locale); } /// @return current TTS locale. For example "en", "ru", "zh-Hant" and so on. /// In case of error returns an empty string. + /// \note The method returns correct locale after SetTurnNotificationsLocale has been called. + /// If not, it returns an empty string. inline string GetTurnNotificationsLocale() const { return m_routingSession.GetTurnNotificationsLocale(); } /// \brief When an end user is going to a turn he gets sound turn instructions. /// If C++ part wants the client to pronounce an instruction GenerateTurnSound (in turnNotifications) returns diff --git a/platform/get_text_by_id.cpp b/platform/get_text_by_id.cpp index a14da57200..c9b715c375 100644 --- a/platform/get_text_by_id.cpp +++ b/platform/get_text_by_id.cpp @@ -13,16 +13,16 @@ namespace { string const kDefaultLanguage = "en"; -string GetTextSourceString(platform::TextSource textSouce) +string GetTextSourceString(platform::TextSource textSource) { #if defined(OMIM_OS_ANDROID) || defined(OMIM_OS_IPHONE) - switch (textSouce) + switch (textSource) { case platform::TextSource::TtsSound: return "sound-strings"; } #else - switch (textSouce) + switch (textSource) { case platform::TextSource::TtsSound: return "../data/sound-strings"; @@ -31,10 +31,10 @@ string GetTextSourceString(platform::TextSource textSouce) ASSERT(false, ()); } -bool GetJsonBuffer(platform::TextSource textSouce, string const & localeName, string & jsonBuffer) +bool GetJsonBuffer(platform::TextSource textSource, string const & localeName, string & jsonBuffer) { string const pathToJson = my::JoinFoldersToPath( - {GetTextSourceString(textSouce), localeName + ".json"}, "localize.json"); + {GetTextSourceString(textSource), localeName + ".json"}, "localize.json"); try { @@ -53,10 +53,10 @@ bool GetJsonBuffer(platform::TextSource textSouce, string const & localeName, st namespace platform { -unique_ptr MakeGetTextById(string const & jsonBuffer, string const & localeName) +TGetTextByIdPtr MakeGetTextById(string const & jsonBuffer, string const & localeName) { - unique_ptr result(new GetTextById(jsonBuffer, localeName)); - if (!result || !result->IsValid()) + TGetTextByIdPtr result(new GetTextById(jsonBuffer, localeName)); + if (!result->IsValid()) { ASSERT(false, ()); return nullptr; @@ -64,20 +64,20 @@ unique_ptr MakeGetTextById(string const & jsonBuffer, string const return result; } -unique_ptr GetTextByIdFactory(TextSource textSouce, string const & localeName) +TGetTextByIdPtr GetTextByIdFactory(TextSource textSource, string const & localeName) { string jsonBuffer; - if (GetJsonBuffer(textSouce, localeName, jsonBuffer)) + if (GetJsonBuffer(textSource, localeName, jsonBuffer)) return MakeGetTextById(jsonBuffer, localeName); - if (GetJsonBuffer(textSouce, kDefaultLanguage, jsonBuffer)) + if (GetJsonBuffer(textSource, kDefaultLanguage, jsonBuffer)) return MakeGetTextById(jsonBuffer, kDefaultLanguage); ASSERT(false, ("sound.txt does not contain default language.")); return nullptr; } -unique_ptr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName) +TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName) { return MakeGetTextById(jsonBuffer, localeName); } @@ -117,12 +117,9 @@ void GetTextById::InitFromJson(string const & jsonBuffer) string GetTextById::operator()(string const & textId) const { - if (!IsValid()) - return string(); - auto const textIt = m_localeTexts.find(textId); if (textIt == m_localeTexts.end()) - return ""; + return string(); return textIt->second; } } // namespace platform diff --git a/platform/get_text_by_id.hpp b/platform/get_text_by_id.hpp index 50c5184a46..ff644e37d3 100644 --- a/platform/get_text_by_id.hpp +++ b/platform/get_text_by_id.hpp @@ -14,6 +14,9 @@ enum class TextSource TtsSound = 0 }; +class GetTextById; +typedef unique_ptr TGetTextByIdPtr; + /// GetTextById represents text messages which are saved in textsDir /// in a specified locale. class GetTextById @@ -25,9 +28,9 @@ public: string operator()(string const & textId) const; string GetLocale() const { return m_locale; } private: - friend unique_ptr GetTextByIdFactory(TextSource textSouce, string const & localeName); - friend unique_ptr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); - friend unique_ptr MakeGetTextById(string const & jsonBuffer, string const & localeName); + friend TGetTextByIdPtr GetTextByIdFactory(TextSource textSource, string const & localeName); + friend TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); + friend TGetTextByIdPtr MakeGetTextById(string const & jsonBuffer, string const & localeName); GetTextById(string const & jsonBuffer, string const & localeName); void InitFromJson(string const & jsonBuffer); @@ -38,11 +41,11 @@ private: unordered_map m_localeTexts; }; -/// Factoris to create intances of GetTextById. -/// If unique_ptr is created by GetTextByIdFactory or ForTestingGetTextByIdFactory -/// threre are only two possibities. +/// Factories to create GetTextById instances. +/// If TGetTextByIdPtr is created by GetTextByIdFactory or ForTestingGetTextByIdFactory +/// threre are only two possibities: /// * a factory returns a valid instance /// * a factory returns nullptr -unique_ptr GetTextByIdFactory(TextSource textSouce, string const & localeName); -unique_ptr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); +TGetTextByIdPtr GetTextByIdFactory(TextSource textSource, string const & localeName); +TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); } // namespace platform diff --git a/platform/platform_tests/get_text_by_id_tests.cpp b/platform/platform_tests/get_text_by_id_tests.cpp index b1eff8f3a9..395199908c 100644 --- a/platform/platform_tests/get_text_by_id_tests.cpp +++ b/platform/platform_tests/get_text_by_id_tests.cpp @@ -15,7 +15,7 @@ UNIT_TEST(GetTextByIdEnglishTest) \"in_1_mile\":\"In one mile.\"\ }"; - unique_ptr getEnglish = platform::ForTestingGetTextByIdFactory(shortJson, "en"); + auto getEnglish = platform::ForTestingGetTextByIdFactory(shortJson, "en"); TEST_EQUAL((*getEnglish)("make_a_slight_right_turn"), "Make a slight right turn.", ()); TEST_EQUAL((*getEnglish)("in_900_meters"), "In nine hundred meters.", ()); TEST_EQUAL((*getEnglish)("then"), "Then.", ()); @@ -38,7 +38,7 @@ UNIT_TEST(GetTextByIdRussianTest) \"in_1_mile\":\"Через одну милю.\"\ }"; - unique_ptr getRussian = platform::ForTestingGetTextByIdFactory(shortJson, "ru"); + auto getRussian = platform::ForTestingGetTextByIdFactory(shortJson, "ru"); TEST_EQUAL((*getRussian)("in_800_meters"), "Через восемьсот метров.", ()); TEST_EQUAL((*getRussian)("make_a_slight_right_turn"), "Плавный поворот направо.", ()); TEST_EQUAL((*getRussian)("take_the_6th_exit"), "Шестой поворот с кольца.", ()); @@ -61,7 +61,7 @@ UNIT_TEST(GetTextByIdKoreanTest) \"in_5000_feet\":\"5000피트 앞\"\ }"; - unique_ptr getKorean = platform::ForTestingGetTextByIdFactory(shortJson, "ko"); + auto getKorean = platform::ForTestingGetTextByIdFactory(shortJson, "ko"); TEST_EQUAL((*getKorean)("in_700_meters"), "700 미터 앞", ()); TEST_EQUAL((*getKorean)("make_a_right_turn"), "우회전입니다.", ()); TEST_EQUAL((*getKorean)("take_the_5th_exit"), "다섯 번째 출구입니다.", ()); @@ -84,7 +84,7 @@ UNIT_TEST(GetTextByIdArabicTest) \"in_4000_feet\":\"بعد 4000 قدم\"\ }"; - unique_ptr getArabic = platform::ForTestingGetTextByIdFactory(shortJson, "ar"); + auto getArabic = platform::ForTestingGetTextByIdFactory(shortJson, "ar"); TEST_EQUAL((*getArabic)("in_1_kilometer"), "بعد كيلو متر واحدٍ", ()); TEST_EQUAL((*getArabic)("leave_the_roundabout"), "اخرج من الطريق الدوار", ()); TEST_EQUAL((*getArabic)("take_the_3rd_exit"), "اسلك المخرج الثالث", ()); @@ -107,7 +107,7 @@ UNIT_TEST(GetTextByIdFrenchTest) \"in_3500_feet\":\"Dans trois mille cinq cents pieds.\"\ }"; - unique_ptr getFrench = platform::ForTestingGetTextByIdFactory(shortJson, "fr"); + auto getFrench = platform::ForTestingGetTextByIdFactory(shortJson, "fr"); TEST_EQUAL((*getFrench)("in_1_5_kilometers"), "Dans un virgule cinq kilomètre.", ()); TEST_EQUAL((*getFrench)("enter_the_roundabout"), "Prenez le rond-point.", ()); TEST_EQUAL((*getFrench)("take_the_2nd_exit"), "Prenez la deuxième sortie.", ()); diff --git a/routing/turns_tts_text.cpp b/routing/turns_tts_text.cpp index cb2e302d32..6932c710d0 100644 --- a/routing/turns_tts_text.cpp +++ b/routing/turns_tts_text.cpp @@ -37,13 +37,11 @@ namespace sound void GetTtsText::SetLocale(string const & locale) { m_getCurLang = platform::GetTextByIdFactory(platform::TextSource::TtsSound, locale); - ASSERT(m_getCurLang, ()); } void GetTtsText::ForTestingSetLocaleWithJson(string const & jsonBuffer, string const & locale) { m_getCurLang = platform::ForTestingGetTextByIdFactory(jsonBuffer, locale); - ASSERT(m_getCurLang, ()); } string GetTtsText::operator()(Notification const & notification) const diff --git a/routing/turns_tts_text.hpp b/routing/turns_tts_text.hpp index 880da596c7..89154d0335 100644 --- a/routing/turns_tts_text.hpp +++ b/routing/turns_tts_text.hpp @@ -22,8 +22,15 @@ class GetTtsText { public: string operator()(Notification const & notification) const; + /// \brief Sets a locale. + /// @param locale is a string representation of locale. For example "en", "ru", "zh-Hant" and so on. + /// \note ee sound/tts/languages.txt for the full list of available locales. void SetLocale(string const & locale); + /// @return current TTS locale. For example "en", "ru", "zh-Hant" and so on. + /// \note The method returns correct locale after SetLocale has been called. + /// If not, it returns an empty string. string GetLocale() const; + void ForTestingSetLocaleWithJson(string const & jsonBuffer, string const & locale); private: From 8bdc8b39e52c1716c5451d888ae0f95fcdd36f71 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Oct 2015 10:34:00 +0300 Subject: [PATCH 4/7] git-clang-format --- platform/get_text_by_id.cpp | 9 +++++---- platform/get_text_by_id.hpp | 4 +++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/platform/get_text_by_id.cpp b/platform/get_text_by_id.cpp index c9b715c375..8683710e80 100644 --- a/platform/get_text_by_id.cpp +++ b/platform/get_text_by_id.cpp @@ -43,9 +43,9 @@ bool GetJsonBuffer(platform::TextSource textSource, string const & localeName, s } catch (RootException const & ex) { - LOG(LWARNING, ("Can't open", localeName,"sound instructions file. pathToJson is", - pathToJson, ex.what())); - return false; // No json file for localeName + LOG(LWARNING, ("Can't open", localeName, "sound instructions file. pathToJson is", pathToJson, + ex.what())); + return false; // No json file for localeName } return true; } @@ -82,7 +82,8 @@ TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, string c return MakeGetTextById(jsonBuffer, localeName); } -GetTextById::GetTextById(string const & jsonBuffer, string const & localeName) : m_locale(localeName) +GetTextById::GetTextById(string const & jsonBuffer, string const & localeName) + : m_locale(localeName) { InitFromJson(jsonBuffer); } diff --git a/platform/get_text_by_id.hpp b/platform/get_text_by_id.hpp index ff644e37d3..d002149d1a 100644 --- a/platform/get_text_by_id.hpp +++ b/platform/get_text_by_id.hpp @@ -27,9 +27,11 @@ public: /// The boolean flag is set to false otherwise. string operator()(string const & textId) const; string GetLocale() const { return m_locale; } + private: friend TGetTextByIdPtr GetTextByIdFactory(TextSource textSource, string const & localeName); - friend TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); + friend TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, + string const & localeName); friend TGetTextByIdPtr MakeGetTextById(string const & jsonBuffer, string const & localeName); GetTextById(string const & jsonBuffer, string const & localeName); From 12fd9dc7a1fd9d3fddf0cf88db0ffe0bf28e89d5 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Oct 2015 10:37:33 +0300 Subject: [PATCH 5/7] Corrections after colleages comments. Second part. --- map/framework.hpp | 2 +- platform/get_text_by_id.cpp | 3 ++- platform/get_text_by_id.hpp | 4 ++-- routing/turns_tts_text.cpp | 4 ++-- routing/turns_tts_text.hpp | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/map/framework.hpp b/map/framework.hpp index fa96242462..ecfd2ee871 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -605,7 +605,7 @@ public: inline bool AreTurnNotificationsEnabled() const { return m_routingSession.AreTurnNotificationsEnabled(); } /// \brief Sets a locale for TTS. /// \param locale is a string with locale code. For example "en", "ru", "zh-Hant" and so on. - /// \note ee sound/tts/languages.txt for the full list of available locales. + /// \note See sound/tts/languages.txt for the full list of available locales. inline void SetTurnNotificationsLocale(string const & locale) { m_routingSession.SetTurnNotificationsLocale(locale); } /// @return current TTS locale. For example "en", "ru", "zh-Hant" and so on. /// In case of error returns an empty string. diff --git a/platform/get_text_by_id.cpp b/platform/get_text_by_id.cpp index 8683710e80..4a2ebafb95 100644 --- a/platform/get_text_by_id.cpp +++ b/platform/get_text_by_id.cpp @@ -29,6 +29,7 @@ string GetTextSourceString(platform::TextSource textSource) } #endif ASSERT(false, ()); + return string(); } bool GetJsonBuffer(platform::TextSource textSource, string const & localeName, string & jsonBuffer) @@ -58,7 +59,7 @@ TGetTextByIdPtr MakeGetTextById(string const & jsonBuffer, string const & locale TGetTextByIdPtr result(new GetTextById(jsonBuffer, localeName)); if (!result->IsValid()) { - ASSERT(false, ()); + ASSERT(false, ("Can't create a GetTextById instance from a json file. localeName=", localeName)); return nullptr; } return result; diff --git a/platform/get_text_by_id.hpp b/platform/get_text_by_id.hpp index d002149d1a..9a6fd38467 100644 --- a/platform/get_text_by_id.hpp +++ b/platform/get_text_by_id.hpp @@ -15,7 +15,7 @@ enum class TextSource }; class GetTextById; -typedef unique_ptr TGetTextByIdPtr; +using TGetTextByIdPtr = unique_ptr; /// GetTextById represents text messages which are saved in textsDir /// in a specified locale. @@ -45,7 +45,7 @@ private: /// Factories to create GetTextById instances. /// If TGetTextByIdPtr is created by GetTextByIdFactory or ForTestingGetTextByIdFactory -/// threre are only two possibities: +/// there are only two possibities: /// * a factory returns a valid instance /// * a factory returns nullptr TGetTextByIdPtr GetTextByIdFactory(TextSource textSource, string const & localeName); diff --git a/routing/turns_tts_text.cpp b/routing/turns_tts_text.cpp index 6932c710d0..2cd97f735f 100644 --- a/routing/turns_tts_text.cpp +++ b/routing/turns_tts_text.cpp @@ -61,7 +61,7 @@ string GetTtsText::operator()(Notification const & notification) const string GetTtsText::GetLocale() const { - if (m_getCurLang) + if (m_getCurLang == nullptr) { ASSERT(false, ()); return string(); @@ -73,7 +73,7 @@ string GetTtsText::GetTextById(string const & textId) const { ASSERT(!textId.empty(), ()); - if (!m_getCurLang) + if (m_getCurLang == nullptr) { ASSERT(false, ()); return ""; diff --git a/routing/turns_tts_text.hpp b/routing/turns_tts_text.hpp index 89154d0335..0229c35645 100644 --- a/routing/turns_tts_text.hpp +++ b/routing/turns_tts_text.hpp @@ -24,7 +24,7 @@ public: string operator()(Notification const & notification) const; /// \brief Sets a locale. /// @param locale is a string representation of locale. For example "en", "ru", "zh-Hant" and so on. - /// \note ee sound/tts/languages.txt for the full list of available locales. + /// \note See sound/tts/languages.txt for the full list of available locales. void SetLocale(string const & locale); /// @return current TTS locale. For example "en", "ru", "zh-Hant" and so on. /// \note The method returns correct locale after SetLocale has been called. From 63ad289b25b44ce08b409be3ca1b7f36125d9d37 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Oct 2015 14:56:45 +0300 Subject: [PATCH 6/7] Get reading of pointing detailed path to sound-strings for Platform::GetReader. --- platform/get_text_by_id.cpp | 16 ++++------------ platform/get_text_by_id.hpp | 4 ++++ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/platform/get_text_by_id.cpp b/platform/get_text_by_id.cpp index 4a2ebafb95..f60be22468 100644 --- a/platform/get_text_by_id.cpp +++ b/platform/get_text_by_id.cpp @@ -15,23 +15,18 @@ string const kDefaultLanguage = "en"; string GetTextSourceString(platform::TextSource textSource) { -#if defined(OMIM_OS_ANDROID) || defined(OMIM_OS_IPHONE) switch (textSource) { case platform::TextSource::TtsSound: - return "sound-strings"; + return string("sound-strings"); } -#else - switch (textSource) - { - case platform::TextSource::TtsSound: - return "../data/sound-strings"; - } -#endif ASSERT(false, ()); return string(); } +} // namespace +namespace platform +{ bool GetJsonBuffer(platform::TextSource textSource, string const & localeName, string & jsonBuffer) { string const pathToJson = my::JoinFoldersToPath( @@ -50,10 +45,7 @@ bool GetJsonBuffer(platform::TextSource textSource, string const & localeName, s } return true; } -} // namespace -namespace platform -{ TGetTextByIdPtr MakeGetTextById(string const & jsonBuffer, string const & localeName) { TGetTextByIdPtr result(new GetTextById(jsonBuffer, localeName)); diff --git a/platform/get_text_by_id.hpp b/platform/get_text_by_id.hpp index 9a6fd38467..b153617820 100644 --- a/platform/get_text_by_id.hpp +++ b/platform/get_text_by_id.hpp @@ -50,4 +50,8 @@ private: /// * a factory returns nullptr TGetTextByIdPtr GetTextByIdFactory(TextSource textSource, string const & localeName); TGetTextByIdPtr ForTestingGetTextByIdFactory(string const & jsonBuffer, string const & localeName); + +/// \bried fills jsonBuffer with json file in twine format with strings in a language of localeName. +/// @return true if no error was happened and false otherwise. +bool GetJsonBuffer(platform::TextSource textSource, string const & localeName, string & jsonBuffer); } // namespace platform From d54a413afe6486715c3522305c0c2e6c1cabbba6 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Oct 2015 14:57:32 +0300 Subject: [PATCH 7/7] Adding unit test on GetJsonBuffer(). --- routing/routing_tests/turns_sound_test.cpp | 13 +++++++++++++ routing/turns_tts_text.cpp | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/routing/routing_tests/turns_sound_test.cpp b/routing/routing_tests/turns_sound_test.cpp index 9d3eee53eb..698e8e3c29 100644 --- a/routing/routing_tests/turns_sound_test.cpp +++ b/routing/routing_tests/turns_sound_test.cpp @@ -503,6 +503,19 @@ UNIT_TEST(TurnsSoundRoundaboutTurnTest) turnSound.GenerateTurnSound(turns9, turnNotifications); TEST_EQUAL(turnNotifications, expectedNotification9, ()); } + +UNIT_TEST(GetJsonBufferTest) +{ + string const localeNameEn = "en"; + string jsonBuffer; + TEST(GetJsonBuffer(platform::TextSource::TtsSound, localeNameEn, jsonBuffer), ()); + TEST(!jsonBuffer.empty(), ()); + + string const localeNameRu = "ru"; + jsonBuffer.clear(); + TEST(GetJsonBuffer(platform::TextSource::TtsSound, localeNameRu, jsonBuffer), ()); + TEST(!jsonBuffer.empty(), ()); +} } // namespace sound } // namespace turns } // namespace routing diff --git a/routing/turns_tts_text.cpp b/routing/turns_tts_text.cpp index 2cd97f735f..cdd02af9c5 100644 --- a/routing/turns_tts_text.cpp +++ b/routing/turns_tts_text.cpp @@ -76,7 +76,7 @@ string GetTtsText::GetTextById(string const & textId) const if (m_getCurLang == nullptr) { ASSERT(false, ()); - return ""; + return string(); } return (*m_getCurLang)(textId); }