From d205264c4de98709a8824d8eb3ae1f33d0dc103c Mon Sep 17 00:00:00 2001 From: Hubert Chrzaniuk Date: Thu, 26 Nov 2020 09:18:45 +0100 Subject: [PATCH] [EGD-4491] MMI call forwarding full support (#1069) * support for Basic Service Group and time parameter of call forwarding * fix RequestFactory, previously regex checking loop may have been left too early --- changelog.md | 1 + module-cellular/at/src/UrcClip.cpp | 5 +-- module-cellular/at/src/UrcCtze.cpp | 9 +--- .../service-audio/ServiceAudio.cpp | 17 ++++--- .../service-cellular/RequestFactory.cpp | 1 - .../requests/CallForwardingRequest.cpp | 38 ++++++++++++---- .../service-cellular/requests/Request.cpp | 11 +++-- .../requests/SupplementaryServicesRequest.cpp | 3 +- .../requests/CallForwardingRequest.hpp | 8 +++- .../service-cellular/requests/Request.hpp | 6 ++- .../service-cellular/tests/unittest_mmi.cpp | 45 +++++++++++-------- module-utils/Utils.hpp | 28 ++++++------ module-utils/test/unittest_utils.cpp | 4 +- 13 files changed, 105 insertions(+), 71 deletions(-) diff --git a/changelog.md b/changelog.md index 6405e984a7d1c9f61acfa212c41f46cef373518e..70b14f5efb6c04d86701586359c52163fb80a664 100644 --- a/changelog.md +++ b/changelog.md @@ -22,6 +22,7 @@ * `[gui][desktop]` ScreenLock logic unified with simLock's * `[messages]` Changed fonts of message snippet and its prefix. +* `[cellular]` Improve MMI call forwarding support ## [0.47.1 2020-11-20] diff --git a/module-cellular/at/src/UrcClip.cpp b/module-cellular/at/src/UrcClip.cpp index a8d12f7dc23c6258d6eae12f2c7df016d8134501..0197e897d347832fa628af76396b466a9f64da31 100644 --- a/module-cellular/at/src/UrcClip.cpp +++ b/module-cellular/at/src/UrcClip.cpp @@ -24,10 +24,7 @@ std::optional Clip::getType() const return std::nullopt; } - int addressType; - if (!utils::toNumeric(tokens[magic_enum::enum_integer(Tokens::Type)], addressType)) { - return std::nullopt; - } + auto addressType = utils::getNumericValue(tokens[magic_enum::enum_integer(Tokens::Type)]); constexpr auto addressTypes = magic_enum::enum_values(); for (const auto &type : addressTypes) { diff --git a/module-cellular/at/src/UrcCtze.cpp b/module-cellular/at/src/UrcCtze.cpp index 18f15d29f67762ca9d2c791c6b39ece5e391ab41..facb292dac9b808cdfb6d1c7f947669ba5bbfb9b 100644 --- a/module-cellular/at/src/UrcCtze.cpp +++ b/module-cellular/at/src/UrcCtze.cpp @@ -18,15 +18,10 @@ int Ctze::getTimeZoneOffset() const { const std::string &tzOffsetToken = tokens[static_cast(Tokens::GMTDifference)]; - int offsetInQuartersOfHour = 0; - bool failed = !utils::toNumeric(tzOffsetToken, offsetInQuartersOfHour); + auto offsetInQuartersOfHour = utils::getNumericValue(tzOffsetToken); if (offsetInQuartersOfHour != std::clamp(offsetInQuartersOfHour, minTimezoneOffset, maxTimezoneOffset)) { offsetInQuartersOfHour = 0; - failed = true; - } - - if (failed) { LOG_ERROR("Failed to parse Ctze time zone offset: %s", tzOffsetToken.c_str()); } @@ -69,7 +64,7 @@ auto Ctze::getTimeInfo() const noexcept -> tm auto gmtDifferenceStr = tokens[Tokens::GMTDifference]; - int gmtDifference = utils::getValue(gmtDifferenceStr); + int gmtDifference = utils::getNumericValue(gmtDifferenceStr); auto time = system_clock::to_time_t(tp) + gmtDifference * utils::time::minutesInQuarterOfHour * utils::time::secondsInMinute; timeinfo = *gmtime(&time); diff --git a/module-services/service-audio/ServiceAudio.cpp b/module-services/service-audio/ServiceAudio.cpp index 297943ea1160919ec7b86aeb5abd552b618b3480..6f336917f2d89f101ccc267ffc25b19a04c3543f 100644 --- a/module-services/service-audio/ServiceAudio.cpp +++ b/module-services/service-audio/ServiceAudio.cpp @@ -158,7 +158,8 @@ static void ExtVibrationStop() bool ServiceAudio::IsVibrationEnabled(const audio::PlaybackType &type) { - auto isEnabled = utils::getValue(getSetting(Setting::EnableVibration, Profile::Type::Idle, type)); + auto isEnabled = + utils::getNumericValue(getSetting(Setting::EnableVibration, Profile::Type::Idle, type)); return isEnabled; } bool ServiceAudio::IsOperationEnabled(const audio::PlaybackType &plType, const Operation::Type &opType) @@ -166,7 +167,8 @@ bool ServiceAudio::IsOperationEnabled(const audio::PlaybackType &plType, const O if (opType == Operation::Type::Router || opType == Operation::Type::Recorder) { return true; } - auto isEnabled = utils::getValue(getSetting(Setting::EnableSound, Profile::Type::Idle, plType)); + auto isEnabled = + utils::getNumericValue(getSetting(Setting::EnableSound, Profile::Type::Idle, plType)); return isEnabled; } @@ -442,7 +444,8 @@ auto ServiceAudio::HandleKeyPressed(const int step) -> std::unique_ptr(audio::RetCode::Success, 0, muted, context); } } - const auto volume = utils::getValue(getSetting(Setting::Volume, Profile::Type::Idle, PlaybackType::None)); + const auto volume = + utils::getNumericValue(getSetting(Setting::Volume, Profile::Type::Idle, PlaybackType::None)); const auto newVolume = std::clamp(volume + step, static_cast(minVolume), static_cast(maxVolume)); setSetting(Setting::Volume, std::to_string(newVolume), Profile::Type::Idle, PlaybackType::None); return std::make_unique(audio::RetCode::Success, newVolume, muted, context); @@ -473,7 +476,7 @@ sys::MessagePointer ServiceAudio::DataReceivedHandler(sys::DataMessage *msgl, sy else if (msgType == typeid(AudioGetSetting)) { auto *msg = static_cast(msgl); auto value = getSetting(msg->setting, msg->profileType, msg->playbackType); - responseMsg = std::make_shared(RetCode::Success, utils::getValue(value)); + responseMsg = std::make_shared(RetCode::Success, utils::getNumericValue(value)); } else if (msgType == typeid(AudioSetSetting)) { auto *msg = static_cast(msgl); @@ -612,7 +615,7 @@ void ServiceAudio::setSetting(const Setting &setting, else if (auto input = audioMux.GetIdleInput(); input && (setting == audio::Setting::Volume)) { updatedProfile = (*input)->audio->GetPriorityPlaybackProfile(); updatedPlayback = PlaybackType::CallRingtone; - valueToSet = std::clamp(utils::getValue(value), minVolume, maxVolume); + valueToSet = std::clamp(utils::getNumericValue(value), minVolume, maxVolume); } else { return; @@ -621,14 +624,14 @@ void ServiceAudio::setSetting(const Setting &setting, switch (setting) { case Setting::Volume: { - const auto clampedValue = std::clamp(utils::getValue(value), minVolume, maxVolume); + const auto clampedValue = std::clamp(utils::getNumericValue(value), minVolume, maxVolume); valueToSet = std::to_string(clampedValue); if (activeInput) { retCode = activeInput.value()->audio->SetOutputVolume(clampedValue); } } break; case Setting::Gain: { - const auto clampedValue = std::clamp(utils::getValue(value), minGain, maxGain); + const auto clampedValue = std::clamp(utils::getNumericValue(value), minGain, maxGain); valueToSet = std::to_string(clampedValue); if (activeInput) { retCode = activeInput.value()->audio->SetInputGain(clampedValue); diff --git a/module-services/service-cellular/RequestFactory.cpp b/module-services/service-cellular/RequestFactory.cpp index 16cf1cd071faa6670e1e418a424b9d42b249d175..4e46834c9e347a4583ed440ceb04daeb462629b8 100644 --- a/module-services/service-cellular/RequestFactory.cpp +++ b/module-services/service-cellular/RequestFactory.cpp @@ -74,7 +74,6 @@ namespace cellular catch (const std::runtime_error &except) { LOG_ERROR("Failed to create MMI request. Error message:\n%s", except.what()); } - break; } } return std::make_unique(request); diff --git a/module-services/service-cellular/requests/CallForwardingRequest.cpp b/module-services/service-cellular/requests/CallForwardingRequest.cpp index 563cd97343544bed4a8663432c4b8e3400b6f47a..d69667d3387cddf455ddaaa59d980aae541ff8f2 100644 --- a/module-services/service-cellular/requests/CallForwardingRequest.cpp +++ b/module-services/service-cellular/requests/CallForwardingRequest.cpp @@ -37,13 +37,26 @@ namespace cellular auto CallForwardingRequest::command() -> std::string { - std::vector commandParts = { - [this]() { return getCommandReason(); }, - [this]() { return getCommandMode(); }, - [this]() { return getCommandNumber(); }, - }; + std::vector commandParts = {[this]() { return getCommandReason(); }, + [this]() { return getCommandMode(); }, + [this]() { return getCommandNumber(); }}; - return buildCommand(at::AT::CCFC, commandParts); + bool trimEmpty = true; + + if (!getCommandClass().empty()) { + commandParts.emplace_back([this]() { return getCommandType(); }); + commandParts.emplace_back([this]() { return getCommandClass(); }); + trimEmpty = false; + } + + if (!getCommandTime().empty()) { + commandParts.emplace_back([this]() { return getCommandSubAddr(); }); + commandParts.emplace_back([this]() { return getCommandSatype(); }); + commandParts.emplace_back([this]() { return getCommandTime(); }); + trimEmpty = false; + } + + return buildCommand(at::AT::CCFC, commandParts, trimEmpty); } auto CallForwardingRequest::getCommandReason() const -> std::string @@ -65,10 +78,10 @@ namespace cellular { // according to EC25&EC21_AT_Commands_Manual_V1.3 if (auto pos = phoneNumber.find("+"); pos != std::string::npos) { - return addressFormatTypeDefault; + return addressFormatTypeInternational; } else { - return addressFormatTypeInternational; + return addressFormatTypeDefault; } } @@ -96,6 +109,15 @@ namespace cellular return noReplyConditionTimer; } + auto CallForwardingRequest::isValid() const noexcept -> bool + { + if (noReplyConditionTimer.empty()) { + return true; + } + auto time = utils::getNumericValue(noReplyConditionTimer); + return std::clamp(time, minNoReplyTime, maxNoReplyTime) == time; + } + void CallForwardingRequest::handle(RequestHandler &h, at::Result &result) { h.handle(*this, result); diff --git a/module-services/service-cellular/requests/Request.cpp b/module-services/service-cellular/requests/Request.cpp index 03b66b2a1c018450c3dc481fc6fa229c8369d9ba..c47bad88186980ed60f21c4ce0038ba7cbb1fc74 100644 --- a/module-services/service-cellular/requests/Request.cpp +++ b/module-services/service-cellular/requests/Request.cpp @@ -31,18 +31,21 @@ namespace cellular return true; } - std::string Request::buildCommand(at::AT atCommand, const std::vector &builderFunctions) const + std::string Request::buildCommand(at::AT atCommand, + const std::vector &builderFunctions, + bool trim) const { if (!isValid()) { return std::string(); } std::string cmd(at::factory(atCommand)); - bool formatFirst = true; + + auto formatFirst = true; for (auto &cmdPart : builderFunctions) { auto partStr = cmdPart(); - if (partStr.empty()) { - continue; + if (partStr.empty() && trim) { + break; } cmd.append(formatFirst ? partStr : "," + partStr); formatFirst = false; diff --git a/module-services/service-cellular/requests/SupplementaryServicesRequest.cpp b/module-services/service-cellular/requests/SupplementaryServicesRequest.cpp index 3c092ceafd4cb8c3016aea9b9dec10adb401589c..b7e4fa869dd4d3f4e561083513e5e620fc1c78a6 100644 --- a/module-services/service-cellular/requests/SupplementaryServicesRequest.cpp +++ b/module-services/service-cellular/requests/SupplementaryServicesRequest.cpp @@ -74,7 +74,6 @@ namespace cellular auto SupplementaryServicesRequest::getCommandInformationClass(const std::string &basicServiceGroup) const -> std::optional { - // according to EC25&EC21_AT_Commands_Manual_V1.3 int basicGroup = 0; int informationClass = 0; @@ -83,7 +82,7 @@ namespace cellular informationClass = atInformationClassAllTele + atInformationClassAllBearer; } else { - utils::toNumeric(basicServiceGroup, basicGroup); + basicGroup = utils::getNumericValue(basicServiceGroup); auto service = magic_enum::enum_cast(basicGroup); if (!service.has_value()) { diff --git a/module-services/service-cellular/service-cellular/requests/CallForwardingRequest.hpp b/module-services/service-cellular/service-cellular/requests/CallForwardingRequest.hpp index b0a42e04f2d877377daa31fba398a404dff92cdd..0947a8c4af6ccd045f754acb6660144df211f7c1 100644 --- a/module-services/service-cellular/service-cellular/requests/CallForwardingRequest.hpp +++ b/module-services/service-cellular/service-cellular/requests/CallForwardingRequest.hpp @@ -20,16 +20,20 @@ namespace cellular auto command() -> std::string final; void handle(RequestHandler &h, at::Result &result) final; + auto isValid() const noexcept -> bool final; private: + // source: EC25&EC21_AT_Commands_Manual static constexpr auto addressFormatTypeInternational = "145"; static constexpr auto addressFormatTypeDefault = "129"; - static constexpr auto subaddrDefault = "128"; + static constexpr auto subaddrDefault = ""; + static constexpr auto maxNoReplyTime = 30; + static constexpr auto minNoReplyTime = 0; std::string forwardReason; std::string &phoneNumber = supplementaryInfoA; std::string &basicServiceGroup = supplementaryInfoB; - std::string &noReplyConditionTimer = supplementaryInfoB; + std::string &noReplyConditionTimer = supplementaryInfoC; // command decomposition according to EC25&EC21_AT_Commands_Manual_V1.3 auto getCommandReason() const -> std::string; diff --git a/module-services/service-cellular/service-cellular/requests/Request.hpp b/module-services/service-cellular/service-cellular/requests/Request.hpp index 542e636690406c842ddf589e4e61ef92a73bb987..f8a373ce9a391e769de4b031d07ff5c22f8ce506 100644 --- a/module-services/service-cellular/service-cellular/requests/Request.hpp +++ b/module-services/service-cellular/service-cellular/requests/Request.hpp @@ -40,10 +40,12 @@ namespace cellular * Helper command for building output command * @param atCommand * @param builderFunctions functions that build parts of the output command in order of execution + * @param trim true to avoid appending commands that evaluate to empty string * @return formatted command or empty string if input is invalid */ - auto buildCommand(at::AT atCommand, const std::vector &builderFunctions) const - -> std::string; + auto buildCommand(at::AT atCommand, + const std::vector &builderFunctions, + bool trim = true) const -> std::string; bool isRequestHandled = false; std::string request; }; diff --git a/module-services/service-cellular/tests/unittest_mmi.cpp b/module-services/service-cellular/tests/unittest_mmi.cpp index be8491e4bfe437e38bbbd21972b1551050dd9a3d..b704bfde7b6e725c22f6842df3ca4297f4455063 100644 --- a/module-services/service-cellular/tests/unittest_mmi.cpp +++ b/module-services/service-cellular/tests/unittest_mmi.cpp @@ -29,6 +29,9 @@ TEST_CASE("MMI requests") }; std::vector testCases = { + /// USSD + {R"(*100*#)", R"(AT+CUSD=1,*100*#,15)", typeid(UssdRequest)}, + /// ImeiRequest {R"(*#06#)", R"(AT+GSN)", typeid(ImeiRequest)}, @@ -85,18 +88,9 @@ TEST_CASE("MMI requests") {R"(*#33*1111*12#)", R"(AT+CLCK="AO",2,"1111",12)", typeid(CallBarringRequest)}, // query with pass and BS {R"(**33#)", std::string(), typeid(CallBarringRequest), false}, // bad procedure - register {R"(##33#)", std::string(), typeid(CallBarringRequest), false}, // bad procedure - erasure - {R"(*#33*1111*17#)", - std::string(), - typeid(CallBarringRequest), - false}, // unsupported BS - Voice Group Call Service - {R"(*#33*1111*18#)", - std::string(), - typeid(CallBarringRequest), - false}, // unsupported BS - Voice Broadcast Service - {R"(*#33*1111*99#)", - std::string(), - typeid(CallBarringRequest), - false}, // unsupported BS - All GPRS bearer services + {R"(*#33*1111*17#)", std::string(), typeid(CallBarringRequest), false}, // unsupported BS - Voice Group Call + {R"(*#33*1111*18#)", std::string(), typeid(CallBarringRequest), false}, // unsupported BS - Voice Broadcast + {R"(*#33*1111*99#)", std::string(), typeid(CallBarringRequest), false}, // unsupported BS - All GPRS bearer {R"(*#33*1111*45#)", std::string(), typeid(CallBarringRequest), false}, // unsupported BS - random /// BOIC (Bar Outgoing International Calls) {R"(*331#)", R"(AT+CLCK="OI",1)", typeid(CallBarringRequest)}, // lock @@ -251,6 +245,14 @@ TEST_CASE("MMI requests") {R"(*#67#)", R"(AT+CCFC=1,2)", typeid(CallForwardingRequest)}, // alternative register and on {R"(*21*666555444#)", R"(AT+CCFC=0,1,"666555444")", typeid(CallForwardingRequest)}, + // optional parameters - basic service group + {R"(*21*666555444*16#)", R"(AT+CCFC=0,1,"666555444",129,8)", typeid(CallForwardingRequest)}, + {R"(*21*+48666555444*19#)", R"(AT+CCFC=0,1,"+48666555444",145,5)", typeid(CallForwardingRequest)}, + // optional parameters - time + {R"(*21*666555444*16*30#)", R"(AT+CCFC=0,1,"666555444",129,8,,,30)", typeid(CallForwardingRequest)}, + {R"(*21*+48666555444*19*20#)", R"(AT+CCFC=0,1,"+48666555444",145,5,,,20)", typeid(CallForwardingRequest)}, + // not valid - timeout exceeds maximum + {R"(*21*+48666555444*19*40#)", std::string(), typeid(CallForwardingRequest), false}, /// PasswordRegistrationRequest // total incoming and outgoing service barring (empty string) @@ -300,13 +302,13 @@ TEST_CASE("MMI requests") // Change PIN by PUK more than 4 {R"(**042*00002*11113*11113#)", R"(AT+CPWD="P2","00002","11113")", typeid(PinChangeRequest)}, // bad procedure type * - {R"(*042*00002*11112*11112#)", std::string(), typeid(CallRequest)}, + {R"(*042*00002*11112*11112#)", std::string(), typeid(UssdRequest)}, // bad procedure type ## {R"(##042*00002*11112*11112#)", std::string(), typeid(CallRequest)}, // bad procedure type # {R"(#042*00002*11112*11112#)", std::string(), typeid(CallRequest)}, // bad procedure type *# - {R"(*#042*00002*11112*11112#)", std::string(), typeid(CallRequest)}, + {R"(*#042*00002*11112*11112#)", std::string(), typeid(UssdRequest)}, // no password {R"(**042*00002**#)", std::string(), typeid(PinChangeRequest), false}, // no password @@ -315,8 +317,12 @@ TEST_CASE("MMI requests") {R"(**042**11112*#)", std::string(), typeid(PinChangeRequest), false}, // no password {R"(**042**11112*#)", std::string(), typeid(PinChangeRequest), false}, - // password does not match + // password does not match {R"(**042*0000*1111*2222#)", std::string(), typeid(PinChangeRequest), false}, + + /// call + {R"(666555777)", std::string(), typeid(CallRequest)}, + {R"(+48666555777)", std::string(), typeid(CallRequest)}, }; for (auto &testCase : testCases) { @@ -326,11 +332,14 @@ TEST_CASE("MMI requests") INFO("Failed on testcase: " << testCase.requestString); REQUIRE(typeid(*request.get()).name() == testCase.expectedType.name()); REQUIRE(request->isValid() == testCase.expectedValid); - if (typeid(*request.get()).name() != typeid(CallRequest).name()) { - REQUIRE(requestCommand == testCase.expectedCommandString); + if (typeid(*request.get()).name() == typeid(CallRequest).name()) { + REQUIRE(requestCommand == "ATD" + testCase.requestString + ";"); + } + else if (typeid(*request.get()).name() == typeid(UssdRequest).name()) { + REQUIRE(requestCommand == "AT+CUSD=1," + testCase.requestString + ",15"); } else { - REQUIRE(requestCommand == "ATD" + testCase.requestString + ";"); + REQUIRE(requestCommand == testCase.expectedCommandString); } } } diff --git a/module-utils/Utils.hpp b/module-utils/Utils.hpp index 9550a8aad6ea7bc766bcaf635b8108d58b4586a6..35ffc7e98cf01e121a263e98ab542d4b4fc330f3 100644 --- a/module-utils/Utils.hpp +++ b/module-utils/Utils.hpp @@ -152,11 +152,11 @@ namespace utils return std::string(magic_enum::enum_name(t)); } - /// Gets value of type T from string + /// Gets arithmetic value of type T from string /// /// @param value to be converted /// @return Value casted to type T - template [[nodiscard]] T getValue(const std::string &value) + template [[nodiscard]] T getNumericValue(const std::string &value) { static_assert(std::is_arithmetic_v); if (value.empty()) { @@ -167,6 +167,18 @@ namespace utils return ret; } + static inline bool toNumeric(const std::string &text, int &value) + { + try { + value = std::stoi(text); + } + catch (const std::exception &e) { + LOG_ERROR("toNumeric exception %s", e.what()); + return false; + } + return true; + } + static inline void findAndReplaceAll(std::string &data, const std::vector>> &values, std::function getReplaceString = nullptr) @@ -218,18 +230,6 @@ namespace utils findAndReplaceAll(data, {{toSearch, replaceStr}}); } - static inline bool toNumeric(const std::string &text, int &value) - { - try { - value = std::stoi(text); - } - catch (const std::exception &e) { - LOG_ERROR("toNumeric exception %s", e.what()); - return false; - } - return true; - } - static inline uint32_t swapBytes(uint32_t toSwap) { #ifdef __GNUC__ diff --git a/module-utils/test/unittest_utils.cpp b/module-utils/test/unittest_utils.cpp index a4a25f97337bdb53bab0e5b19bd7f00969c63659..5ac0fed3a22f7828ba1854285da6e8087a280432 100644 --- a/module-utils/test/unittest_utils.cpp +++ b/module-utils/test/unittest_utils.cpp @@ -168,14 +168,14 @@ TEST_CASE("Get value from string") SECTION("UInt32_t") { std::string testString = "10"; - const auto testValue = utils::getValue(testString); + const auto testValue = utils::getNumericValue(testString); REQUIRE(testValue == 10); } SECTION("float") { std::string testString = "1.f"; - const auto testValue = utils::getValue(testString); + const auto testValue = utils::getNumericValue(testString); Approx target = Approx(1.f).margin(.01f); REQUIRE(testValue == target); }