From 649a36ea09c38d1b0a98ce7dc76c030b844c9175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Mon, 12 Oct 2026 16:27:14 +0200 Subject: [PATCH] accept comma separated rx_dbitlist (#1463) * accept comma separated rx_dbitlist * added counters * bumping workflow versions * bumping actions version and fixing conda-build * check brackets * updated help * updated trimen * handling thresholds --------- Co-authored-by: AliceMazzoleni99 --- slsDetectorSoftware/src/CallerSpecial.cpp | 81 ++++++++++++----------- slsSupportLib/include/sls/ToString.h | 55 ++++++++++++++- slsSupportLib/include/sls/string_utils.h | 3 + slsSupportLib/src/string_utils.cpp | 10 +++ slsSupportLib/tests/test-ToString.cpp | 20 ++++++ slsSupportLib/tests/test-string_utils.cpp | 7 ++ 6 files changed, 134 insertions(+), 42 deletions(-) diff --git a/slsDetectorSoftware/src/CallerSpecial.cpp b/slsDetectorSoftware/src/CallerSpecial.cpp index 2ece1cd04..005f29196 100644 --- a/slsDetectorSoftware/src/CallerSpecial.cpp +++ b/slsDetectorSoftware/src/CallerSpecial.cpp @@ -424,41 +424,43 @@ std::string Caller::threshold(int action) { throw RuntimeError("Not implemented for this detector\n"); } } else if (action == defs::PUT_ACTION) { + if (args.empty()) { + WrongNumberOfParameters(1); + } defs::detectorType type = det->getDetectorType().squash(); - if (type == defs::EIGER && args.size() != 1 && args.size() != 2) { - WrongNumberOfParameters(1); - } - if (type == defs::MYTHEN3 && (args.size() < 1 || args.size() > 4)) { - WrongNumberOfParameters(1); - } - bool trimbits = (cmd == "thresholdnotb") ? false : true; - std::array energy = {StringTo(args[0]), 0, 0}; - energy[1] = energy[0]; - energy[2] = energy[0]; - defs::detectorSettings sett = defs::STANDARD; - // check if argument has settings or get it - if (args.size() == 2 || args.size() == 4) { - sett = StringTo(args[args.size() - 1]); + // settings is optional, last and the only argument starting with a + // letter. The energies before it accept "a b c" as well as "a,b,c" + auto energyArgs = args; + defs::detectorSettings sett = defs::STANDARD; + if (std::isalpha(static_cast(args.back()[0]))) { + sett = StringTo(args.back()); + energyArgs.pop_back(); } else { sett = det->getSettings(std::vector{det_id}) .tsquash("Inconsistent settings between detectors"); } + auto energy = StringTo>(join(energyArgs, ',')); - // get other threshold values - if (args.size() > 2) { - energy[1] = StringTo(args[1]); - energy[2] = StringTo(args[2]); - } switch (type) { case defs::EIGER: + if (energy.size() != 1) { + throw RuntimeError("Expected one threshold energy"); + } det->setThresholdEnergy(energy[0], sett, trimbits, std::vector{det_id}); break; case defs::MYTHEN3: - det->setThresholdEnergy(energy, sett, trimbits, - std::vector{det_id}); + // one energy applies to all three counters + if (energy.size() == 1) { + energy.resize(3, energy[0]); + } + if (energy.size() != 3) { + throw RuntimeError("Expected one or three threshold energies"); + } + det->setThresholdEnergy({energy[0], energy[1], energy[2]}, sett, + trimbits, std::vector{det_id}); break; default: throw RuntimeError("Not implemented for this detector\n"); @@ -485,19 +487,16 @@ std::string Caller::trimen(int action) { auto t = det->getTrimEnergies(std::vector{det_id}); os << OutString(t) << '\n'; } else if (action == defs::PUT_ACTION) { - std::vector t(args.size()); - if (!args.empty()) { - for (size_t i = 0; i < t.size(); ++i) { - t[i] = StringTo(args[i]); - } - } + // accepts both "4500 5400" and "4500,5400", empty list clears + auto t = StringTo>(join(args, ',')); det->setTrimEnergies(t, std::vector{det_id}); - os << ToString(args) << '\n'; + os << ToString(t) << '\n'; } else { throw RuntimeError("Unknown action"); } return os.str(); } + std::string Caller::badchannels(int action) { std::ostringstream os; if (action == defs::HELP_ACTION) { @@ -1013,6 +1012,9 @@ std::string Caller::counters(int action) { "enabled. Each element in list can be 0 - 2 and must be non " "repetitive. Enabling counters sets vth dacs to remembered " "values and disabling sets them to disabled values." + "Accepts both space separated and comma separated list of indices" + "optionally enclosed in brackets. Example: 0 1 2 or 0,1,2 or " + "[0,1,2] or [0 1 2]" << '\n'; } else if (action == defs::GET_ACTION) { if (!args.empty()) { @@ -1024,15 +1026,15 @@ std::string Caller::counters(int action) { if (args.empty()) { WrongNumberOfParameters(1); } - if (std::any_of(args.cbegin(), args.cend(), [](std::string s) { - return (StringTo(s) < 0 || StringTo(s) > 2); - })) { + // accepts both "0 1 2" and "0,1,2" + auto counters = StringTo>(join(args, ',')); + if (std::any_of(counters.cbegin(), counters.cend(), + [](int val) { return (val < 0 || val > 2); })) { throw RuntimeError("Invalid counter indices list. Example: 0 1 2"); } // convert vector to counter enable mask uint32_t mask = 0; - for (size_t i = 0; i < args.size(); ++i) { - int val = StringTo(args[i]); + for (auto val : counters) { // already enabled earlier if (mask & (1 << val)) { std::ostringstream oss; @@ -1042,7 +1044,7 @@ std::string Caller::counters(int action) { mask |= (1 << val); } det->setCounterMask(mask, std::vector{det_id}); - os << ToString(args) << '\n'; + os << ToString(counters) << '\n'; } else { throw RuntimeError("Unknown action"); } @@ -1216,6 +1218,9 @@ std::string Caller::rx_dbitlist(int action) { "receiver list, the data size will be bigger if the number of " "samples is not divisible by 8 as every signal bit is padded to " "the next byte when combining all the samples in the receiver." + "Accepts both space separated and comma separated list of indices" + "optionally enclosed in brackets. Example: 0 11 37 or 3,10,15 or " + "[62,21] or [0 1 2]" << '\n'; } else if (action == defs::GET_ACTION) { if (!args.empty()) { @@ -1236,14 +1241,10 @@ std::string Caller::rx_dbitlist(int action) { } // 'none' option already covered as t is empty by default else if (args[0] != "none") { - unsigned int ntrim = args.size(); - t.resize(ntrim); - for (unsigned int i = 0; i < ntrim; ++i) { - t[i] = StringTo(args[i]); - } + t = StringTo>(join(args, ',')); } det->setRxDbitList(t, std::vector{det_id}); - os << ToString(args) << '\n'; + os << ToString(t) << '\n'; } else { throw RuntimeError("Unknown action"); } diff --git a/slsSupportLib/include/sls/ToString.h b/slsSupportLib/include/sls/ToString.h index 7035f9c38..7c692badd 100644 --- a/slsSupportLib/include/sls/ToString.h +++ b/slsSupportLib/include/sls/ToString.h @@ -1,7 +1,6 @@ // SPDX-License-Identifier: LGPL-3.0-or-other // Copyright (C) 2021 Contributors to the SLS Detector Package #pragma once - /** * \file ToString.h * @@ -353,7 +352,9 @@ T StringTo(const std::string &f, const std::string &unit) { } } -template T StringTo(const std::string &t) { +template ::value, int> = 0> +T StringTo(const std::string &t) { std::string tmp{t}; auto unit = RemoveUnit(tmp); return StringTo(tmp, unit); @@ -398,6 +399,7 @@ ToString(const T &obj) { return obj.str(); } +/** Convert vector of strings to vector of type T */ template std::vector StringTo(const std::vector &strings) { std::vector result; @@ -407,4 +409,53 @@ std::vector StringTo(const std::vector &strings) { return result; } +/** Parse comma-separated string into vector type T (e.g., + * StringTo>()) */ +template ::value && + !std::is_same_v, + int> = 0> +T StringTo(const std::string &s) { + using ElementType = typename T::value_type; + // Enclosing brackets are optional but must match. Nested pairs are + // accepted since the caller may already have wrapped the list once. + std::string list = s; + auto strip = [](std::string &str) { + auto first = str.find_first_not_of(' '); + if (first == std::string::npos) { + str.clear(); + return; + } + str = str.substr(first, str.find_last_not_of(' ') - first + 1); + }; + strip(list); + while (list.size() >= 2 && list.front() == '[' && list.back() == ']') { + list = list.substr(1, list.size() - 2); + strip(list); + } + if (list.find_first_of("[]") != std::string::npos) { + throw RuntimeError("Mismatched brackets in list '" + s + "'"); + } + + T res; + std::istringstream ss(list); + std::string item; + while (std::getline(ss, item, ',')) { + item.erase(std::remove_if(item.begin(), item.end(), + [](char c) { return c == '"' || c == ' '; }), + item.end()); + + try { + if (item.empty()) + continue; + auto val = StringTo(item); + res.push_back(val); + } catch (const std::exception &e) { + throw RuntimeError("Could not convert '" + item + + "' to the container element type. " + + std::string(e.what())); + } + } + return res; +} + } // namespace sls diff --git a/slsSupportLib/include/sls/string_utils.h b/slsSupportLib/include/sls/string_utils.h index abeed79a7..d6afefebb 100644 --- a/slsSupportLib/include/sls/string_utils.h +++ b/slsSupportLib/include/sls/string_utils.h @@ -85,6 +85,9 @@ implementation should not be used in a performance critical place. */ std::vector split(const std::string &strToSplit, char delimeter); +/** Join strings with the specified delimiter, e.g. {"a", "b"} -> "a,b" */ +std::string join(const std::vector &strings, char delimeter); + std::string RemoveUnit(std::string &str); bool is_int(const std::string &s); diff --git a/slsSupportLib/src/string_utils.cpp b/slsSupportLib/src/string_utils.cpp index 36d161e4e..1f4c7635d 100644 --- a/slsSupportLib/src/string_utils.cpp +++ b/slsSupportLib/src/string_utils.cpp @@ -24,6 +24,16 @@ std::vector split(const std::string &strToSplit, char delimeter) { return splittedStrings; } +std::string join(const std::vector &strings, char delimeter) { + std::string joined; + for (const auto &s : strings) { + if (!joined.empty()) + joined += delimeter; + joined += s; + } + return joined; +} + std::string RemoveUnit(std::string &str) { auto it = str.begin(); while (it != str.end()) { diff --git a/slsSupportLib/tests/test-ToString.cpp b/slsSupportLib/tests/test-ToString.cpp index 0c35bdfc0..194f04442 100644 --- a/slsSupportLib/tests/test-ToString.cpp +++ b/slsSupportLib/tests/test-ToString.cpp @@ -415,4 +415,24 @@ TEST_CASE("string to timingInfoDecoder") { defs::timingInfoDecoder::SHINE); } + +TEST_CASE("Convert string to vector of ints") { + REQUIRE(StringTo>("1, 2, 3") == std::vector{1, 2, 3}); + REQUIRE(StringTo>(" 4 ,5,6 ") == std::vector{4, 5, 6}); + REQUIRE(StringTo>("[8]") == std::vector{8}); + REQUIRE(StringTo>("9, ") == std::vector{9}); + REQUIRE(StringTo>("") == std::vector{}); + REQUIRE(StringTo>("[]") == std::vector{}); + REQUIRE(StringTo>(" [ [1, 2] ] ") == + std::vector{1, 2}); +} + +TEST_CASE("Mismatched brackets in string to vector throws") { + REQUIRE_THROWS(StringTo>("[1, 2")); + REQUIRE_THROWS(StringTo>("1, 2]")); + REQUIRE_THROWS(StringTo>("[[1, 2]")); + REQUIRE_THROWS(StringTo>("1]2")); + REQUIRE_THROWS(StringTo>("[1], [2]")); +} + } // namespace sls diff --git a/slsSupportLib/tests/test-string_utils.cpp b/slsSupportLib/tests/test-string_utils.cpp index 58f431f54..eb27f6de9 100644 --- a/slsSupportLib/tests/test-string_utils.cpp +++ b/slsSupportLib/tests/test-string_utils.cpp @@ -52,6 +52,13 @@ TEST_CASE("split a string without end delimiter") { REQUIRE(r[2] == "filibom"); } +TEST_CASE("join strings") { + REQUIRE(join({"abra", "kadabra", "filibom"}, '+') == + "abra+kadabra+filibom"); + REQUIRE(join({"abra"}, '+') == "abra"); + REQUIRE(join({}, '+') == ""); +} + TEST_CASE("Remove char from string") { char str[] = "sometest"; removeChar(str, 'e');