~aleteoryx/muditaos: [EGD-5831] Fix handling URC

5 files changed, 174 insertions(+), 8 deletions(-)

M enabled_unittests
M module-cellular/Modem/ATURCStream.cpp
M module-cellular/Modem/ATURCStream.hpp
M module-cellular/test/unittest_ATURCStream.cpp
M module-services/service-cellular/ServiceCellular.cpp

M enabled_unittests => enabled_unittests +3 -0

@@ 400,3 400,6 @@ TESTS_LIST["catch2-unittest_ATURCStream"]="
     URC AT Stream Parser;
 "
 #---------
+
+
+

M module-cellular/Modem/ATURCStream.cpp => module-cellular/Modem/ATURCStream.cpp +4 -2

@@ 42,7 42,7 @@ namespace at
             if (posDelimiter != std::string::npos) {
                 auto posMark2 = atBuffer.find("\"", posMark + 1);
                 if ((posMark2 != std::string::npos) && (posDelimiter > posMark2)) {
-                    posMark           = posMark2 + 1;
+                    posMark           = posMark2;
                     openQuotationMark = !openQuotationMark;
                     return false;
                 }


@@ 59,13 59,15 @@ namespace at
     /**
      * Check for next quotation and urc, in general could look like
      * \r\n+URC: "literal \r\n...."\r\n\r\n+URC: "param",4,4\r\n ...
+     * or
+     * \r\nURC... \r\n
      * @return true if found and we could look for next URCin buffer
      */
     bool ATURCStream::lookForNext()
     {
         lookForNextURC    = false;
         openQuotationMark = false;
-        posMark           = 0; /// start from \r\n+ from search
+        posMark           = 0; /// start from \r\n(+) from search
         do {
             if (checkQuotation())
                 break;

M module-cellular/Modem/ATURCStream.hpp => module-cellular/Modem/ATURCStream.hpp +1 -1

@@ 8,7 8,7 @@
 
 namespace at
 {
-    constexpr auto urcStart = "\r\n+";
+    constexpr auto urcStart = "\r\n";
 
     class ATURCStream
     {

M module-cellular/test/unittest_ATURCStream.cpp => module-cellular/test/unittest_ATURCStream.cpp +164 -5

@@ 18,6 18,23 @@ TEST_CASE("URC AT Stream Parser")
         REQUIRE(*v.begin() == "\r\n+URC: one\r\n");
     }
 
+    SECTION("One URC multiple \"\"")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+URC: one \"\"\"\"\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+URC: one \"\"\"\"\r\n");
+    }
+
+    SECTION("One URC no closed \"")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+URC: one \"\"\"\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 0);
+    }
+
     SECTION("One URC with junk at the begin")
     {
         at::ATURCStream stream;


@@ 27,7 44,7 @@ TEST_CASE("URC AT Stream Parser")
         REQUIRE(*v.begin() == "\r\n+URC: one\r\n");
     }
 
-    SECTION("test 1")
+    SECTION("Two URC with junk at the begin")
     {
         at::ATURCStream stream;
         stream.write("ssss\r\n+URC: one\r\n");


@@ 39,7 56,7 @@ TEST_CASE("URC AT Stream Parser")
         REQUIRE(*v.begin() == "\r\n+URC: two\r\n");
     }
 
-    SECTION("test 2")
+    SECTION("Three URC with junk at the begin")
     {
         at::ATURCStream stream;
         stream.write("ssss\r\n+URC: one\r\n");


@@ 55,7 72,17 @@ TEST_CASE("URC AT Stream Parser")
         REQUIRE(*v.begin() == "\r\n+URC: 3,\"\r\n........\"\r\n");
     }
 
-    SECTION("test 4")
+    SECTION("URC split in empty literal")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+URC: \"");
+        stream.write("\"\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+URC: \"\"\r\n");
+    }
+
+    SECTION("Long URC in two parts")
     {
         at::ATURCStream stream;
         stream.write("\r\n+CUSD: 0,\"Pozostalo Ci 24,84zl do wykorzystania do 2021-02-18 23:59:59. Dodatkowo w ramach "


@@ 65,7 92,7 @@ TEST_CASE("URC AT Stream Parser")
         REQUIRE(v.size() == 1);
     }
 
-    SECTION("test 5")
+    SECTION("Long URC with new line in string section")
     {
         at::ATURCStream stream;
         stream.write("\r\n+CUSD: 1,\"Play 24 Menu\r\n"


@@ 77,7 104,7 @@ TEST_CASE("URC AT Stream Parser")
         REQUIRE(v.size() == 1);
     }
 
-    SECTION("test 6")
+    SECTION("Long URC with new line in string section in two parts")
     {
         at::ATURCStream stream;
         stream.write("\r\n+CUSD: 1,\"Play 24 Menu\r\n"


@@ 88,4 115,136 @@ TEST_CASE("URC AT Stream Parser")
         auto v = stream.getURCList();
         REQUIRE(v.size() == 1);
     }
+
+    SECTION("One urc (URC without +)")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\nNO CARRIER\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\nNO CARRIER\r\n");
+    }
+
+    SECTION("Two mixed URC")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\nNO CARRIER\r\n");
+        stream.write("\r\n+URC: two\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 2);
+        REQUIRE(*v.begin() == "\r\nNO CARRIER\r\n");
+        v.pop_front();
+        REQUIRE(*v.begin() == "\r\n+URC: two\r\n");
+    }
+
+    SECTION("Two mixed URC with junk at the beginning")
+    {
+        at::ATURCStream stream;
+        stream.write("junk\r\nNO CARRIER\r\n");
+        stream.write("\r\n+URC: two\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 2);
+        REQUIRE(*v.begin() == "\r\nNO CARRIER\r\n");
+        v.pop_front();
+        REQUIRE(*v.begin() == "\r\n+URC: two\r\n");
+    }
+
+    SECTION("Three mixed URC with junk at the beginning, read partially")
+    {
+        at::ATURCStream stream;
+        stream.write("ssss\r\n+URC: one\r\n");
+        stream.write("\r\n+URC: two\r\n");
+        stream.write("\r\n+URC: 3,\"\r\n....");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 2);
+        REQUIRE(*v.begin() == "\r\n+URC: one\r\n");
+        v.pop_front();
+        REQUIRE(*v.begin() == "\r\n+URC: two\r\n");
+
+        stream.write("....\"\r\n");
+        v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+URC: 3,\"\r\n........\"\r\n");
+    }
+
+    SECTION("Standard URC  1 - simple one")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\nRING\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\nRING\r\n");
+    }
+
+    SECTION("Standard URC  2 - with literals")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+CMTI: \"ME\",0\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+CMTI: \"ME\",0\r\n");
+    }
+
+    SECTION("Standard URC  3 - with symbol /")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+QCFG: \"urc/cache\",0\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+QCFG: \"urc/cache\",0\r\n");
+    }
+
+    SECTION("Standard URC  4 - with symbol &")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+QIND: \"act\",\"HSDPA&HSUPA\"\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+QIND: \"act\",\"HSDPA&HSUPA\"\r\n");
+    }
+
+    SECTION("Standard URC  5 - two string section")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+QIND: \"act\",\"UNKNOWN\"\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+QIND: \"act\",\"UNKNOWN\"\r\n");
+    }
+
+    SECTION("Standard URC  6 - mixed sections")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+CREG: 1,\"D509\",\"80D413D\",7\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+CREG: 1,\"D509\",\"80D413D\",7\r\n");
+    }
+
+    SECTION("Standard URC  7 - POWERED DOWN")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\nPOWERED DOWN\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\nPOWERED DOWN\r\n");
+    }
+
+    SECTION("Standard URC  8 - empty \"\"")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+CLIP: \"+48600222222\",145,\"\",0,,0\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+CLIP: \"+48600222222\",145,\"\",0,,0\r\n");
+    }
+
+    SECTION("Standard URC  9 - CLIP without empty \"\" multi ,")
+    {
+        at::ATURCStream stream;
+        stream.write("\r\n+CLIP: \"+48600222222\",145,,,,0\r\n");
+        auto v = stream.getURCList();
+        REQUIRE(v.size() == 1);
+        REQUIRE(*v.begin() == "\r\n+CLIP: \"+48600222222\",145,,,,0\r\n");
+    }
 }

M module-services/service-cellular/ServiceCellular.cpp => module-services/service-cellular/ServiceCellular.cpp +2 -0

@@ 199,6 199,8 @@ ServiceCellular::ServiceCellular() : sys::Service(serviceName, "", cellularStack
     notificationCallback = [this](std::string &data) {
         LOG_DEBUG("Notifications callback called with %u data bytes", static_cast<unsigned int>(data.size()));
 
+        std::string logStr = utils::removeNewLines(data);
+        LOG_DEBUG("Data: %s", logStr.c_str());
         atURCStream.write(data);
         auto vUrc = atURCStream.getURCList();