From 5f287ea092d87999388fbb41de9554ca9eadb7dc Mon Sep 17 00:00:00 2001 From: Dhanya Thattil Date: Mon, 15 Dec 2025 13:41:56 +0100 Subject: [PATCH] changed bitPosition from int to uint32_t --- python/src/bit.cpp | 12 +++++-- slsDetectorSoftware/src/CallerSpecial.cpp | 7 ++-- slsSupportLib/include/sls/bit_utils.h | 12 ++++--- slsSupportLib/src/bit_utils.cpp | 23 ++++++++++-- slsSupportLib/tests/test-bit_utils.cpp | 43 +++++++++++++++++------ 5 files changed, 73 insertions(+), 24 deletions(-) diff --git a/python/src/bit.cpp b/python/src/bit.cpp index 2fb3093b9..27afce469 100644 --- a/python/src/bit.cpp +++ b/python/src/bit.cpp @@ -28,13 +28,19 @@ void init_bit(py::module &m) { py::class_(m, "BitAddress") .def(py::init()) - .def(py::init()) + .def(py::init()) .def("__repr__", &BitAddress::str) .def("str", &BitAddress::str) .def("address", &BitAddress::address) .def("bitPosition", &BitAddress::bitPosition) - .def("setAddress", &BitAddress::setAddress) - .def("setBitPosition", &BitAddress::setBitPosition) + .def("setBitPosition", + static_cast(&BitAddress::setBitPosition)) + .def("setBitPositionStr", + static_cast(&BitAddress::setBitPosition)) + .def("setAddress", + static_cast(&BitAddress::setAddress)) + .def("setAddressStr", + static_cast(&BitAddress::setAddress)) .def(py::self == py::self) .def(py::self != py::self); diff --git a/slsDetectorSoftware/src/CallerSpecial.cpp b/slsDetectorSoftware/src/CallerSpecial.cpp index 68a3e00e3..a7d6adf9c 100644 --- a/slsDetectorSoftware/src/CallerSpecial.cpp +++ b/slsDetectorSoftware/src/CallerSpecial.cpp @@ -1556,7 +1556,7 @@ std::string Caller::define_bit(int action) { // get name from position and address else if (args.size() == 2) { auto pos = - BitAddress(parseAddress(args[0]), StringTo(args[1])); + BitAddress(parseAddress(args[0]), StringTo(args[1])); try { auto t = det->getBitDefinitionName(pos); os << t << '\n'; @@ -1581,7 +1581,8 @@ std::string Caller::define_bit(int action) { throw RuntimeError("Bit position must be an integer value."); } auto name = args[0]; - auto bit = BitAddress(parseAddress(args[1]), StringTo(args[2])); + auto bit = + BitAddress(parseAddress(args[1]), StringTo(args[2])); det->setBitDefinition(name, bit); os << ToString(args) << '\n'; } else { @@ -1823,7 +1824,7 @@ BitAddress Caller::parseBitAddress() const { else if (argsSize == 2) { std::string bit_pos = args[1]; return BitAddress(parseAddress(addr_or_bitname), - StringTo(bit_pos)); + StringTo(bit_pos)); } else { throw RuntimeError("Command " + cmd + " expected (1-4) parameter/s but got " + diff --git a/slsSupportLib/include/sls/bit_utils.h b/slsSupportLib/include/sls/bit_utils.h index d619c017e..a8baa52ca 100644 --- a/slsSupportLib/include/sls/bit_utils.h +++ b/slsSupportLib/include/sls/bit_utils.h @@ -43,17 +43,21 @@ class RegisterAddress { class BitAddress { private: RegisterAddress addr_{0}; - int bitPos_{0}; + uint32_t bitPos_{0}; public: constexpr BitAddress() noexcept = default; - BitAddress(RegisterAddress address, int bitPosition); + BitAddress(RegisterAddress address, uint32_t bitPosition); std::string str() const; RegisterAddress address() const noexcept { return addr_; } - int bitPosition() const noexcept { return bitPos_; } + uint32_t bitPosition() const noexcept { return bitPos_; } + + void setBitPosition(uint32_t bitPos); + void setBitPosition(const std::string &bitPos); + void setAddress(RegisterAddress address) noexcept { addr_ = address; } - void setBitPosition(int bitPos) noexcept { bitPos_ = bitPos; } + void setAddress(const std::string &address); constexpr bool operator==(const BitAddress &other) const { return (addr_ == other.addr_ && bitPos_ == other.bitPos_); diff --git a/slsSupportLib/src/bit_utils.cpp b/slsSupportLib/src/bit_utils.cpp index c164c459c..841d0d263 100644 --- a/slsSupportLib/src/bit_utils.cpp +++ b/slsSupportLib/src/bit_utils.cpp @@ -16,11 +16,28 @@ RegisterAddress::RegisterAddress(const std::string &address) { std::string RegisterAddress::str() const { return ToStringHex(addr_); } -BitAddress::BitAddress(RegisterAddress address, int bitPosition) - : addr_(address), bitPos_(bitPosition) { - if (bitPosition < 0 || bitPosition > 31) { +BitAddress::BitAddress(RegisterAddress address, uint32_t bitPosition) + : addr_(address) { + setBitPosition(bitPosition); +} + +void BitAddress::setBitPosition(uint32_t bitPos) { + if (bitPos > 31) { throw RuntimeError("Bit position must be between 0 and 31."); } + bitPos_ = bitPos; +} + +void BitAddress::setBitPosition(const std::string &bitPos) { + if (!is_hex_or_dec_uint(bitPos)) { + throw RuntimeError("Bit position must be an integer value."); + } + uint32_t pos = StringTo(bitPos); + setBitPosition(pos); +} + +void BitAddress::setAddress(const std::string &address) { + addr_ = RegisterAddress(address); } std::string BitAddress::str() const { diff --git a/slsSupportLib/tests/test-bit_utils.cpp b/slsSupportLib/tests/test-bit_utils.cpp index fc09dee5c..7caf5a62c 100644 --- a/slsSupportLib/tests/test-bit_utils.cpp +++ b/slsSupportLib/tests/test-bit_utils.cpp @@ -43,7 +43,7 @@ TEST_CASE("Get set bits from 523") { REQUIRE(vec == std::vector{0, 1, 3, 9}); } -TEST_CASE("Convert RegisterAddress using classes ", "[support][bit_utils]") { +TEST_CASE("Convert RegisterAddress using classes ", "[support][.bit_utils]") { std::vector vec_addr{0x305, 0xffffffff, 0x0, 0x34550987, 0x1fff1fff}; std::vector vec_ans{"0x305", "0xffffffff", "0x0", "0x34550987", @@ -66,7 +66,7 @@ TEST_CASE("Convert RegisterAddress using classes ", "[support][bit_utils]") { } } -TEST_CASE("Convert RegisterValue using classes ", "[support][bit_utils]") { +TEST_CASE("Convert RegisterValue using classes ", "[support][.bit_utils]") { std::vector vec_addr{0x305, 0xffffffff, 0x0, 500254562, 0x1fff1fff}; std::vector vec_ans{"0x305", "0xffffffff", "0x0", "0x1dd14762", @@ -91,12 +91,13 @@ TEST_CASE("Convert RegisterValue using classes ", "[support][bit_utils]") { } } -TEST_CASE("Convert BitAddress using classes ", "[support][bit_utils]") { +TEST_CASE("Convert BitAddress using classes", "[support][.bit_utils]") { std::vector vec_addr{ RegisterAddress(0x305), RegisterAddress(0xffffffffu), RegisterAddress(0x0), RegisterAddress(0x34550987), RegisterAddress(0x1fff1fff)}; - std::vector vec_bitpos{0, 15, 31, 7, 23}; + std::vector vec_bitpos{0, 15, 31, 7, 23}; + std::vector vec_bitpos_str{"0", "15", "31", "7", "23"}; std::vector vec_ans{ "[0x305, 0]", "[0xffffffff, 15]", "[0x0, 31]", "[0x34550987, 7]", "[0x1fff1fff, 23]", @@ -107,7 +108,8 @@ TEST_CASE("Convert BitAddress using classes ", "[support][bit_utils]") { BitAddress reg1; reg1.setAddress(vec_addr[i]); - reg1.setBitPosition(vec_bitpos[i]); + reg1.setBitPosition(vec_bitpos_str[i]); + CHECK(reg1.bitPosition() == vec_bitpos[i]); auto reg2 = BitAddress(vec_addr[0], vec_bitpos[0]); @@ -118,6 +120,8 @@ TEST_CASE("Convert BitAddress using classes ", "[support][bit_utils]") { CHECK(reg1.address() == vec_addr[i]); CHECK(reg0.bitPosition() == vec_bitpos[i]); CHECK(reg1.bitPosition() == vec_bitpos[i]); + CHECK(std::to_string(reg0.bitPosition()) == vec_bitpos_str[i]); + CHECK(std::to_string(reg1.bitPosition()) == vec_bitpos_str[i]); CHECK(reg0.str() == vec_ans[i]); CHECK(reg1.str() == vec_ans[i]); @@ -125,7 +129,7 @@ TEST_CASE("Convert BitAddress using classes ", "[support][bit_utils]") { } TEST_CASE("Output operator gives same result as string", - "[support][bit_utils]") { + "[support][.bit_utils]") { { RegisterAddress addr{"0x3456af"}; std::ostringstream os; @@ -133,6 +137,13 @@ TEST_CASE("Output operator gives same result as string", CHECK(os.str() == "0x3456af"); CHECK(os.str() == addr.str()); } + { + BitAddress addr(RegisterAddress("0x3456af"), 15); + std::ostringstream os; + os << addr; + CHECK(os.str() == "[0x3456af, 15]"); + CHECK(os.str() == addr.str()); + } { RegisterValue addr{"0x3456af"}; std::ostringstream os; @@ -142,7 +153,7 @@ TEST_CASE("Output operator gives same result as string", } } -TEST_CASE("Strange input throws", "[support][bit_utils]") { +TEST_CASE("Strange input throws", "[support][.bit_utils]") { REQUIRE_THROWS(RegisterAddress("hej")); // ensure correct exception message try { @@ -151,12 +162,14 @@ TEST_CASE("Strange input throws", "[support][bit_utils]") { REQUIRE(std::string(e.what()) == "Address must be an integer value."); } - REQUIRE_THROWS(RegisterValue("hej")); + BitAddress bitAddr(RegisterAddress(0x305), 0); + REQUIRE_THROWS(bitAddr.setBitPosition("hej")); // ensure correct exception message try { - RegisterValue("hej"); + bitAddr.setBitPosition("hej"); } catch (const std::exception &e) { - REQUIRE(std::string(e.what()) == "Value must be an integer value."); + REQUIRE(std::string(e.what()) == + "Bit position must be an integer value."); } REQUIRE_THROWS(BitAddress(RegisterAddress(0x305), 32)); @@ -167,10 +180,18 @@ TEST_CASE("Strange input throws", "[support][bit_utils]") { REQUIRE(std::string(e.what()) == "Bit position must be between 0 and 31."); } + + REQUIRE_THROWS(RegisterValue("hej")); + // ensure correct exception message + try { + RegisterValue("hej"); + } catch (const std::exception &e) { + REQUIRE(std::string(e.what()) == "Value must be an integer value."); + } } TEST_CASE("Implicitly convert to uint for sending over network", - "[support][bit_utils]") { + "[support][.bit_utils]") { RegisterAddress addr{0x305}; uint64_t a = addr; CHECK(a == 0x305);