Merge pull request #297 from slsdetectorgroup/parse

Relaxed parsing supporting RR

slsDetectorSoftware/src/CmdParser.cpp

@@ -1,28 +1,13 @@
 
 #include "CmdParser.h"
-#include "sls/sls_detector_defs.h"
+#include "sls/string_utils.h"
-#include <cstdio>
+
-#include <cstring>
+#include <algorithm>
-#include <iostream>
 #include <iterator>
 #include <sstream>
 
 namespace sls {
 
-void CmdParser::Print() {
-    std::cout << "\nCmdParser::Print()\n";
-    std::cout << "\tmulti_id: " << multi_id_
-              << ", detector_id: " << detector_id_ << std::endl;
-    std::cout << "\texecutable: " << executable_ << '\n';
-    std::cout << "\tcommand: " << command_ << '\n';
-    std::cout << "\tn_arguments: " << n_arguments() << '\n';
-    std::cout << "\targuments: ";
-    for (const auto &argument : arguments_) {
-        std::cout << argument << " ";
-    }
-    std::cout << "\n\n";
-};
-
 void CmdParser::Parse(int argc, const char *const argv[]) {
     Reset();
     executable_ = argv[0]; // first arg is calling binary
@@ -36,88 +21,82 @@ void CmdParser::Parse(int argc, const char *const argv[]) {
     }
 }
 
-void CmdParser::Parse(const std::string &s) {
+void CmdParser::Parse(std::string s) {
+    // taking s by value we can modify it.
     Reset();
+
+    // Are we looking at -h --help? avoid removing h from command starting
+    // with h when combined with detector id (ex, 1-hostname)
+    bool h = replace_first(&s, "--help", " ");
+    h = h || replace_first(&s, " -h", " ");
+    h = h || replace_first(&s, "-h ", " ");
+    help_ = h;
+
+    // Extract the position indicies
+    auto pos = s.find_first_not_of("0123456789:- ");
+    if (pos != 0) {
+        auto pre = s.substr(0, pos);
+        pre.erase(std::remove(pre.begin(), pre.end(), ' '), pre.end());
+        s.erase(0, pos);
+        DecodeIdAndPosition(pre.c_str());
+    }
+
+    // Command and args should now be all that's left in the string
     std::istringstream iss(s);
     auto it = std::istream_iterator<std::string>(iss);
+    command_ = *it++; // First arg is the comand to run
+
     arguments_ =
         std::vector<std::string>(it, std::istream_iterator<std::string>());
-    auto old_size = arguments_.size();
+
-    arguments_.erase(std::remove_if(begin(arguments_), end(arguments_),
+    // allow comma sep
-                                    [](const std::string &item) {
+    for (auto &arg : arguments_) {
-                                        return (item == "-h" ||
-                                                item == "--help");
-                                    }),
-                     end(arguments_));
-    if (old_size - arguments_.size() > 0)
-        help_ = true;
-    if (!arguments_.empty()) {
-        command_ = arguments_[0];
-        arguments_.erase(begin(arguments_));
-    }
-    //allow comma sep
-    for (auto& arg : arguments_){
         if (arg.back() == ',')
             arg.pop_back();
     }
-
-    DecodeIdAndPosition(command_.c_str());
 }
 
-void CmdParser::DecodeIdAndPosition(const char *c) {
+void CmdParser::DecodeIdAndPosition(std::string pre) {
-    bool contains_id = std::strchr(c, '-') != nullptr;
+    if (pre.empty())
-    bool contains_pos = std::strchr(c, ':') != nullptr;
+        return;
-    char tmp[100];
 
-    if (contains_id && contains_pos) {
+    // Get the detector id
-        int r = sscanf(c, "%d-%d:%s", &multi_id_, &detector_id_, tmp);
+    auto pos = pre.find_first_of("-");
-        if (r != 3) {
+    if (pos != std::string::npos) {
-            throw(sls::RuntimeError(
+        multi_id_ = std::stoi(pre);
-                "Cannot decode client or detector id from: \"" +
+        pre.erase(0, pos + 1);
-                std::string(c) + "\"\n"));
     }
-        command_ = tmp;
+
-    } else if (contains_id && !contains_pos) {
+    // if there is nothing left to parse we need to return
-        int r = sscanf(c, "%d-%s", &multi_id_, tmp);
+    if (pre.empty()) {
-        if (r != 2) {
+        return;
-            throw(sls::RuntimeError("Cannot decode client id from: \"" +
-                                    std::string(c) + "\"\n"));
     }
-        command_ = tmp;
+
-    } else if (!contains_id && contains_pos) {
+    // now lets see if there is a :
-        int r = sscanf(c, "%d:%s", &detector_id_, tmp);
+    pos = pre.find_first_of(":");
-        if (r != 2) {
+    if (pos == std::string::npos) {
-            throw(sls::RuntimeError("Cannot decode detector id from: \"" +
+        // no : we only have the multi id
-                                    std::string(c) + "\"\n"));
+        detector_id_ = std::stoi(pre);
-        }
+
-        command_ = tmp;
+    } else if (pos == 0) {
+        // do nothing, there is no multi id specified
+        pre.erase(0, 1);
     } else {
-        command_ = c;
+        // the : is somewhere in the middle
+        detector_id_ = std::stoi(pre);
+        pre.erase(0, pos + 1);
     }
-}
 
-std::vector<const char *> CmdParser::argv() const {
+    // now if the string is not empty we also have a receiver id
-    std::vector<const char *> vec;
+    if (!pre.empty()) {
-    if (!command_.empty()) {
+        receiver_id_ = std::stoi(pre);
-        vec.push_back(&command_.front());
     }
-    for (auto &arg : arguments_) {
-        vec.push_back(&arg.front());
-    }
-    return vec;
-}
-
-std::string CmdParser::cli_line() const {
-    std::ostringstream os;
-    os << command_;
-    for (const auto &arg : arguments_)
-        os << " " << arg;
-    return os.str();
 }
 
 void CmdParser::Reset() {
     multi_id_ = 0;
     detector_id_ = -1;
+    receiver_id_ = -1;
     help_ = false;
     command_.clear();
     executable_.clear();

slsDetectorSoftware/src/CmdParser.h

@@ -22,11 +22,11 @@ namespace sls {
 class CmdParser {
   public:
     void Parse(int argc, const char *const argv[]);
-    void Parse(const std::string &s);
+    void Parse(std::string s);
-    void Print();
 
     int multi_id() const noexcept { return multi_id_; };
     int detector_id() const noexcept { return detector_id_; };
+    int receiver_id() const noexcept { return receiver_id_; };
     int n_arguments() const noexcept { return arguments_.size(); }
     const std::string &command() const noexcept { return command_; }
     void setCommand(std::string cmd) { command_ = cmd; }
@@ -36,14 +36,13 @@ class CmdParser {
     const std::vector<std::string> &arguments() const noexcept {
         return arguments_;
     };
-    std::vector<const char *> argv() const;
-    std::string cli_line() const;
 
   private:
-    void DecodeIdAndPosition(const char *c);
+    void DecodeIdAndPosition(std::string pre);
     void Reset(); // reset all private variables
     int multi_id_ = 0;
     int detector_id_ = -1;
+    int receiver_id_ = -1;
     bool help_ = false;
     std::string command_;
     std::string executable_;

slsDetectorSoftware/tests/test-CmdParser.cpp

@@ -18,8 +18,8 @@ SCENARIO("Construction", "[support]") {
             REQUIRE(p.multi_id() == 0);
             REQUIRE(p.command().empty());
             REQUIRE(p.arguments().empty());
-            REQUIRE(p.argv().empty());
+            // REQUIRE(p.argv().empty());
-            REQUIRE(p.argv().data() == nullptr);
+            // REQUIRE(p.argv().data() == nullptr);
         }
     }
 }
@@ -35,7 +35,7 @@ SCENARIO("Parsing a string with the command line parser", "[support]") {
                 REQUIRE(p.multi_id() == 0);
                 REQUIRE(p.command().empty());
                 REQUIRE(p.arguments().empty());
-                REQUIRE(p.argv().empty());
+                // REQUIRE(p.argv().empty());
             }
         }
         WHEN("Parsing a string with a single command") {
@@ -46,7 +46,7 @@ SCENARIO("Parsing a string with the command line parser", "[support]") {
                 REQUIRE(p.detector_id() == -1);
                 REQUIRE(p.multi_id() == 0);
                 REQUIRE(p.arguments().empty());
-                REQUIRE(p.argv().size() == 1);
+                // REQUIRE(p.argv().size() == 1);
             }
         }
         WHEN("Parsing a string with command and value") {
@@ -72,7 +72,7 @@ SCENARIO("Parsing a string with the command line parser", "[support]") {
                     REQUIRE(p.multi_id() == 0);
                     REQUIRE(p.command() == res[i]);
                     REQUIRE(p.arguments().empty());
-                    REQUIRE(p.argv().size() == 1);
+                    // REQUIRE(p.argv().size() == 1);
                 }
             }
         }
@@ -89,7 +89,7 @@ SCENARIO("Parsing a string with the command line parser", "[support]") {
                     REQUIRE(p.multi_id() == multi_id[i]);
                     REQUIRE(p.command() == res[i]);
                     REQUIRE(p.arguments().empty());
-                    REQUIRE(p.argv().size() == 1);
+                    // REQUIRE(p.argv().size() == 1);
                 }
             }
         }
@@ -105,16 +105,6 @@ SCENARIO("Parsing a string with the command line parser", "[support]") {
                 REQUIRE(p.arguments()[2] == "7000");
             }
         }
-
-        WHEN("Cliend id and or detector id cannot be decoded") {
-            vs arg{"o:cmd",     "-5:cmd",    "aedpva:cmd",
-                   "5-svc:vrf", "asv-5:cmd", "savc-asa:cmd"};
-            THEN("Parsing Throws") {
-                for (size_t i = 0; i != arg.size(); ++i) {
-                    REQUIRE_THROWS(p.Parse(arg[i]));
-                }
-            }
-        }
     }
 }
 
@@ -131,7 +121,7 @@ SCENARIO("Parsing strings with -h or --help", "[support]") {
                 REQUIRE(p.command() == "list");
                 REQUIRE(p.isHelp());
                 REQUIRE(p.arguments().empty());
-                REQUIRE(p.argv().size() == 1);
+                // REQUIRE(p.argv().size() == 1);
             }
         }
         WHEN("Parsing a string with -h at a different position") {
@@ -157,6 +147,20 @@ SCENARIO("Parsing strings with -h or --help", "[support]") {
     }
 }
 
+TEST_CASE("Parse string with --help"){
+    CmdParser p;
+    p.Parse("list --help");
+    REQUIRE(p.isHelp()==true);
+    REQUIRE(p.command()=="list");
+}
+
+TEST_CASE("Parse string with -h"){
+    CmdParser p;
+    p.Parse("list -h");
+    REQUIRE(p.isHelp()==true);
+    REQUIRE(p.command()=="list");
+}
+
 TEST_CASE("Parsing consecutive strings resets not found det id") {
     CmdParser p;
     p.Parse("1:exptime 0.5");
@@ -266,27 +270,143 @@ TEST_CASE("Double digit id", "[support]") {
     REQUIRE(p.arguments().empty());
 }
 
-TEST_CASE("Calling with wrong id throws invalid_argument", "[support]") {
+
-    int argc = 2;
+
-    const char *const argv[]{"caller", "asvldkn:vrf"};
+TEST_CASE("Allows space between mod id and command"){
     CmdParser p;
-    CHECK_THROWS(p.Parse(argc, argv));
+    p.Parse("7: exptime 0.5");
+    REQUIRE(p.detector_id() == 7);
+    REQUIRE(p.command() == "exptime");
+    REQUIRE(p.arguments().size() == 1);
+    REQUIRE(p.arguments()[0] == "0.5");
 }
 
-TEST_CASE("Calling with wrong client throws invalid_argument", "[support]") {
+TEST_CASE("Allows space between mod id and command also without :"){
-    int argc = 2;
-    const char *const argv[]{"caller", "lki-3:vrf"};
     CmdParser p;
-    CHECK_THROWS(p.Parse(argc, argv));
+    p.Parse("1 exptime 0.5");
+    REQUIRE(p.detector_id() == 1);
+    REQUIRE(p.command() == "exptime");
+    REQUIRE(p.arguments().size() == 1);
+    REQUIRE(p.arguments()[0] == "0.5");
 }
 
-TEST_CASE("Build up argv", "[support]") {
+TEST_CASE("Allows space between mod id and command when detector id is used"){
     CmdParser p;
-    REQUIRE(p.argv().empty());
+    p.Parse("1-5 exptime 0.5");
-    REQUIRE(p.argv().data() == nullptr);
+    REQUIRE(p.detector_id() == 5);
-
+    REQUIRE(p.multi_id() == 1);
-    std::string s = "trimen 3000 4000\n";
+    REQUIRE(p.command() == "exptime");
-    p.Parse(s);
+    REQUIRE(p.arguments().size() == 1);
-    REQUIRE(p.argv().data() != nullptr);
+    REQUIRE(p.arguments()[0] == "0.5");
-    REQUIRE(p.argv().size() == 3);
+}
+
+TEST_CASE("Allows space between mod id and command with detector id and :"){
+    CmdParser p;
+    p.Parse("1-5: exptime 0.5");
+    REQUIRE(p.detector_id() == 5);
+    REQUIRE(p.multi_id() == 1);
+    REQUIRE(p.command() == "exptime");
+    REQUIRE(p.arguments().size() == 1);
+    REQUIRE(p.arguments()[0] == "0.5");
+}
+
+TEST_CASE("Parse receiver ID"){
+    CmdParser p;
+    p.Parse("2-5:3 flowcontrol10g 1");
+    REQUIRE(p.detector_id() == 5);
+    REQUIRE(p.multi_id() == 2);
+    REQUIRE(p.command() == "flowcontrol10g");
+    REQUIRE(p.arguments().size() == 1);
+    REQUIRE(p.arguments()[0] == "1");
+    REQUIRE(p.receiver_id()==3);
+}
+
+TEST_CASE("Parse receiver ID no det id"){
+    CmdParser p;
+    p.Parse("5:95 flowcontrol10g");
+    REQUIRE(p.detector_id() == 5);
+    REQUIRE(p.multi_id() == 0);
+    REQUIRE(p.command() == "flowcontrol10g");
+    REQUIRE(p.arguments().size() == 0);
+    REQUIRE(p.receiver_id()==95);
+}
+
+
+
+TEST_CASE("Det id but no mod id"){
+    CmdParser p;
+    p.Parse("1-exptime");
+    REQUIRE(p.detector_id() == -1); //not there
+    REQUIRE(p.multi_id() == 1);
+    REQUIRE(p.command() == "exptime");
+}
+
+TEST_CASE("Det id but no mod id but with space after -"){
+    CmdParser p;
+    p.Parse("1- exptime");
+    REQUIRE(p.detector_id() == -1); //not there
+    REQUIRE(p.multi_id() == 1);
+    REQUIRE(p.command() == "exptime");
+}
+
+TEST_CASE("Parse receiver ID no det id no mod"){
+    CmdParser p;
+    p.Parse(":95 flowcontrol10g");
+    REQUIRE(p.detector_id() == -1); //not there
+    REQUIRE(p.multi_id() == 0);
+    REQUIRE(p.command() == "flowcontrol10g");
+    REQUIRE(p.arguments().size() == 0);
+    REQUIRE(p.receiver_id()==95);
+}
+
+TEST_CASE("Parse mod and receiver id"){
+    CmdParser p;
+    p.Parse("1:3 exptime");
+    REQUIRE(p.detector_id() == 1); 
+    REQUIRE(p.receiver_id()==3);
+    REQUIRE(p.command() == "exptime");
+}
+
+TEST_CASE("Det id but no no mod"){
+    CmdParser p;
+    p.Parse("2-:35 exptime");
+    REQUIRE(p.detector_id() == -1); 
+    REQUIRE(p.receiver_id()==35);
+    REQUIRE(p.multi_id() == 2);
+    REQUIRE(p.command() == "exptime");
+}
+
+TEST_CASE("All stuff"){
+    CmdParser p;
+    p.Parse("3-4:2 exptime");
+    REQUIRE(p.detector_id() == 4); 
+    REQUIRE(p.receiver_id()==2);
+    REQUIRE(p.multi_id() == 3);
+    REQUIRE(p.command() == "exptime");
+}
+
+TEST_CASE("Parse a command that has -h in it"){
+    CmdParser p;
+    p.Parse("1-hostname somepc");
+    REQUIRE(p.multi_id() == 1);
+    REQUIRE(p.command() == "hostname");
+    REQUIRE(p.arguments().size() == 1);
+    REQUIRE(p.arguments()[0]== "somepc");
+
+}
+
+TEST_CASE("Parse a command in the form 0-1 command"){
+    CmdParser p;
+    p.Parse("3-5 exptime");
+    REQUIRE(p.multi_id() == 3);
+    REQUIRE(p.detector_id() == 5);
+    REQUIRE(p.command() == "exptime");
+}
+
+TEST_CASE("Parse a command in the form 0-1:command"){
+    CmdParser p;
+    p.Parse("3-5:exptime");
+    REQUIRE(p.multi_id() == 3);
+    REQUIRE(p.detector_id() == 5);
+    REQUIRE(p.command() == "exptime");
 }

slsDetectorSoftware/tests/test-CmdProxy.cpp

@@ -104,28 +104,23 @@ TEST_CASE("detectorserverversion", "[.cmd]") {
 TEST_CASE("serialnumber", "[.cmd]") {
     Detector det;
     CmdProxy proxy(&det);
+    auto det_type = det.getDetectorType().squash();
+    if (det_type == defs::EIGER) {
+        REQUIRE_THROWS(proxy.Call("serialnumber", {}, -1, GET));
+    } else {
         REQUIRE_NOTHROW(proxy.Call("serialnumber", {}, -1, GET));
+    }
 }
 
 TEST_CASE("moduleid", "[.cmd]") {
     Detector det;
     CmdProxy proxy(&det);
-    if (det.getDetectorType().squash() == defs::GOTTHARD2) {
+    auto det_type = det.getDetectorType().squash();
-        auto prev_val = det.getModuleId();
+    if (det_type == defs::GOTTHARD2 || det_type == defs::MYTHEN3 ||
+        det_type == defs::EIGER) {
         REQUIRE_NOTHROW(proxy.Call("moduleid", {}, -1, GET));
-        std::ostringstream oss1, oss2, oss3;
-        proxy.Call("moduleid", {"0x5d"}, -1, PUT, oss1);
-        REQUIRE(oss1.str() == "moduleid 0x5d\n");
-        proxy.Call("moduleid", {}, -1, GET, oss2);
-        REQUIRE(oss2.str() == "moduleid 0x5d\n");
-        proxy.Call("moduleid", {"0xffff"}, -1, PUT, oss3);
-        REQUIRE(oss3.str() == "moduleid 0xffff\n");
-        REQUIRE_THROWS(proxy.Call("moduleid", {"65536"}, -1, PUT));
-        for (int i = 0; i != det.size(); ++i) {
-            det.setModuleId(prev_val[i], {i});
-        }
     } else {
-        REQUIRE_THROWS(proxy.Call("moduleid", {"0"}, -1, GET));
+        REQUIRE_THROWS(proxy.Call("moduleid", {}, -1, GET));
     }
 }
 

slsSupportLib/include/sls/string_utils.h

@@ -53,4 +53,7 @@ std::vector<std::string> split(const std::string &strToSplit, char delimeter);
 std::string RemoveUnit(std::string &str);
 
 bool is_int(const std::string &s);
+
+bool replace_first(std::string *s, const std::string& substr, const std::string& repl);
+
 } // namespace sls

slsSupportLib/src/string_utils.cpp

@@ -36,4 +36,14 @@ bool is_int(const std::string &s) {
                          }) == s.end();
 }
 
+bool replace_first(std::string *s, const std::string& substr, const std::string& repl){
+    auto pos = s->find(substr);
+    if (pos != std::string::npos){
+        s->replace(pos, substr.size(), repl);
+        return true;
+    }
+    return false;
+}
+
+
 }; // namespace sls

slsSupportLib/tests/test-string_utils.cpp

@@ -76,4 +76,33 @@ TEST_CASE("Check is string is integer") {
     REQUIRE_FALSE(sls::is_int(""));
 }
 
+
+TEST_CASE("Replace substring in string"){
+    std::string s = "this string should be replaced";
+    auto r = sls::replace_first(&s, "string ", "");
+    REQUIRE(r == true);
+    REQUIRE(s == "this should be replaced");
+}
+
+TEST_CASE("Replace --help in command"){
+    std::string s = "sls_detector_get --help exptime";
+    auto r = sls::replace_first(&s, " --help", "");
+    REQUIRE(r == true);
+    REQUIRE(s == "sls_detector_get exptime");
+}
+
+TEST_CASE("Replace -h in command"){
+    std::string s = "sls_detector_get -h exptime";
+    auto r = sls::replace_first(&s, " -h", "");
+    REQUIRE(r == true);
+    REQUIRE(s == "sls_detector_get exptime");
+}
+
+TEST_CASE("replace --help"){
+    std::string s = "list --help";
+    auto r = sls::replace_first(&s, " --help", "");
+    REQUIRE(r == true);
+    REQUIRE(s == "list");
+}
+
 // TEST_CASE("concat things not being strings")

tests/CMakeLists.txt

@@ -21,7 +21,7 @@ add_executable(tests ${SLS_TEST_SOURCES})
 target_link_libraries(tests
     PUBLIC 
       slsProjectOptions
-      slsSupportShared
+      slsSupportStatic
       pthread
       rt
     PRIVATE
@@ -30,7 +30,7 @@ target_link_libraries(tests
 
 if (SLS_USE_DETECTOR)
     target_link_libraries(tests PUBLIC
-        slsDetectorShared
+        slsDetectorStatic
     )
 endif (SLS_USE_DETECTOR)