From 42c9fcdfdf07f74e3b68e509e3c25379035353f9 Mon Sep 17 00:00:00 2001 From: Hayodea Hekol Date: Sun, 14 Jun 2026 11:00:30 -0400 Subject: [PATCH] Refactor intrinThresholdParams to use shared DAP helpers. Delegate threshold param parsing to the new DeviceAttachmentSpec primitives so intrin modules stay aligned with stimBuff param conventions. Co-authored-by: Cursor --- .../attachmentSupport/tests/CMakeLists.txt | 22 ++ .../tests/intrinThresholdParams_tests.cpp | 331 ++++++++++++++++++ include/user/intrinThresholdParams.h | 88 ++--- 3 files changed, 375 insertions(+), 66 deletions(-) create mode 100644 commonLibs/attachmentSupport/tests/intrinThresholdParams_tests.cpp diff --git a/commonLibs/attachmentSupport/tests/CMakeLists.txt b/commonLibs/attachmentSupport/tests/CMakeLists.txt index 7f6e35f..721aa67 100644 --- a/commonLibs/attachmentSupport/tests/CMakeLists.txt +++ b/commonLibs/attachmentSupport/tests/CMakeLists.txt @@ -19,3 +19,25 @@ add_dependencies(deviceAttachmentSpecParams_tests gtest_main) add_test( NAME deviceAttachmentSpecParams_tests COMMAND deviceAttachmentSpecParams_tests) + +add_executable(intrinThresholdParams_tests + intrinThresholdParams_tests.cpp +) + +target_include_directories(intrinThresholdParams_tests PRIVATE + ${CMAKE_SOURCE_DIR}/include + ${CMAKE_BINARY_DIR}/include + ${CMAKE_SOURCE_DIR}/libspinscale/tests +) + +target_link_libraries(intrinThresholdParams_tests + gtest_main + spinscale_test_support + ${Boost_LIBRARIES} +) + +add_dependencies(intrinThresholdParams_tests gtest_main) + +add_test( + NAME intrinThresholdParams_tests + COMMAND intrinThresholdParams_tests) diff --git a/commonLibs/attachmentSupport/tests/intrinThresholdParams_tests.cpp b/commonLibs/attachmentSupport/tests/intrinThresholdParams_tests.cpp new file mode 100644 index 0000000..c14d41f --- /dev/null +++ b/commonLibs/attachmentSupport/tests/intrinThresholdParams_tests.cpp @@ -0,0 +1,331 @@ +#include +#include +#include +#include + +namespace smo { +namespace intrin { +namespace { + +using ParamList = std::vector>; + +ParamList makeParams(std::initializer_list> entries) +{ + ParamList params; + for (const auto& entry : entries) + { + params.emplace_back(entry.first, entry.second); + } + + return params; +} + +TEST(IntrinThresholdArrayContainsTest, MatchesCanonicalNames) +{ + EXPECT_TRUE(arrayContains(kIntrinInterestPcNames, "interest-pc")); + EXPECT_TRUE(arrayContains(kIntrinInterestThrNames, "interest-thr")); + EXPECT_FALSE(arrayContains(kForbiddenUnitlessIntrinStems, "interest-pc")); +} + +TEST(IntrinThresholdIsKnownParamNameTest, RecognizesAllCanonicalFamilies) +{ + EXPECT_TRUE(isKnownIntrinThresholdParamName("interest-pc")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("interest-threshold")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("distraction-percentage")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("distraction-thr")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("stupefying-pc")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("stupefaction-thresh")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("intolerable-pc")); + EXPECT_TRUE(isKnownIntrinThresholdParamName("intolerable-threshold")); + + EXPECT_FALSE(isKnownIntrinThresholdParamName("interest")); + EXPECT_FALSE(isKnownIntrinThresholdParamName("passband-count-gt-val")); +} + +TEST(IntrinThresholdValidateNoIntrinParamsOnQualeIfaceTest, RejectsKnownIntrinNames) +{ + const ParamList params = makeParams({{"interest-pc", "85"}}); + + try { + validateNoIntrinParamsOnQualeIface("pcloudLightAmbience", params); + FAIL() << "Expected std::runtime_error"; + } + catch (const std::runtime_error& exception) + { + sscl::tests::expectExceptionMessageContains( + exception, "not valid on qualeIfaceApi"); + } +} + +TEST(IntrinThresholdValidateNoIntrinParamsOnQualeIfaceTest, RejectsUnitlessStems) +{ + const ParamList params = makeParams({{"distraction", "10"}}); + + try { + validateNoIntrinParamsOnQualeIface("mesh", params); + FAIL() << "Expected std::runtime_error"; + } + catch (const std::runtime_error& exception) + { + sscl::tests::expectExceptionMessageContains( + exception, "invalid without a unit suffix"); + } +} + +TEST(IntrinThresholdValidateNoIntrinParamsOnQualeIfaceTest, RejectsFromStimbuffMarker) +{ + const ParamList params = makeParams({{"from-stimbuff", ""}}); + + try { + validateNoIntrinParamsOnQualeIface("mesh", params); + FAIL() << "Expected std::runtime_error"; + } + catch (const std::runtime_error& exception) + { + sscl::tests::expectExceptionMessageContains( + exception, "from-stimbuff"); + } +} + +TEST(IntrinThresholdValidateNoIntrinParamsOnQualeIfaceTest, AllowsSensoryParams) +{ + const ParamList params = makeParams({ + {"passband-count-gt-val", "12"}, + {"histbuff-ms", "30000"}, + }); + + EXPECT_NO_THROW(validateNoIntrinParamsOnQualeIface( + "pcloudLightAmbience", params)); +} + +TEST(IntrinThresholdValidateIntrinSegmentParamsTest, RejectsUnitlessStems) +{ + const ParamList params = makeParams({{"stupefaction", "5"}}); + + try { + validateIntrinSegmentParams("postrin", params); + FAIL() << "Expected std::runtime_error"; + } + catch (const std::runtime_error& exception) + { + sscl::tests::expectExceptionMessageContains( + exception, "invalid without a unit suffix"); + } +} + +TEST(IntrinThresholdValidateIntrinSegmentParamsTest, AllowsSuffixedNames) +{ + const ParamList params = makeParams({ + {"interest-pc", "85"}, + {"distraction-thr", "12"}, + }); + + EXPECT_NO_THROW(validateIntrinSegmentParams("negtrin", params)); +} + +TEST(IntrinThresholdParseOptionalThresholdParamTest, ReturnsDefaultWhenAbsent) +{ + const ParamList params = makeParams({{"other-param", "1"}}); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + /*defaultValue=*/7, + /*defaultUnit=*/ThresholdUnit::Absolute); + + EXPECT_FALSE(parsed.wasSpecified); + EXPECT_EQ(parsed.value, 7); + EXPECT_EQ(parsed.unit, ThresholdUnit::Absolute); + EXPECT_TRUE(parsed.matchedName.empty()); +} + +TEST(IntrinThresholdParseOptionalThresholdParamTest, ParsesPercentageParam) +{ + const ParamList params = makeParams({{"interest-percentage", "50"}}); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Absolute); + + EXPECT_TRUE(parsed.wasSpecified); + EXPECT_EQ(parsed.value, 50); + EXPECT_EQ(parsed.unit, ThresholdUnit::Percentage); + EXPECT_EQ(parsed.matchedName, "interest-percentage"); +} + +TEST(IntrinThresholdParseOptionalThresholdParamTest, ParsesAbsoluteParam) +{ + const ParamList params = makeParams({{"interest-threshold", "42"}}); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Percentage); + + EXPECT_TRUE(parsed.wasSpecified); + EXPECT_EQ(parsed.value, 42); + EXPECT_EQ(parsed.unit, ThresholdUnit::Absolute); + EXPECT_EQ(parsed.matchedName, "interest-threshold"); +} + +TEST(IntrinThresholdParseOptionalThresholdParamTest, LattermostMatchingParamWins) +{ + const ParamList params = makeParams({ + {"interest-pc", "10"}, + {"interest-threshold", "99"}, + }); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Percentage); + + EXPECT_TRUE(parsed.wasSpecified); + EXPECT_EQ(parsed.value, 99); + EXPECT_EQ(parsed.unit, ThresholdUnit::Absolute); + EXPECT_EQ(parsed.matchedName, "interest-threshold"); +} + +TEST(IntrinThresholdParseOptionalThresholdParamTest, LattermostPercentageWinsOverEarlierAbsolute) +{ + const ParamList params = makeParams({ + {"interest-thr", "12"}, + {"interest-pc", "75"}, + }); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Absolute); + + EXPECT_TRUE(parsed.wasSpecified); + EXPECT_EQ(parsed.value, 75); + EXPECT_EQ(parsed.unit, ThresholdUnit::Percentage); + EXPECT_EQ(parsed.matchedName, "interest-pc"); +} + +TEST(IntrinThresholdParseOptionalThresholdParamTest, InvalidIntegerThrows) +{ + const ParamList params = makeParams({{"interest-pc", "not-int"}}); + + try { + parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Absolute); + FAIL() << "Expected std::runtime_error"; + } + catch (const std::runtime_error& exception) + { + sscl::tests::expectExceptionMessageContains( + exception, "Failed to parse 'interest-pc'"); + } +} + +TEST(IntrinThresholdResolveThresholdValueTest, PercentageUsesBase) +{ + const ParsedThresholdParam percentageParam = { + .value = 50, + .unit = ThresholdUnit::Percentage, + .matchedName = "interest-pc", + .wasSpecified = true, + }; + + EXPECT_EQ(resolveThresholdValue(percentageParam, 84), 42u); +} + +TEST(IntrinThresholdResolveThresholdValueTest, AbsolutePassesThrough) +{ + const ParsedThresholdParam absoluteParam = { + .value = 17, + .unit = ThresholdUnit::Absolute, + .matchedName = "interest-thr", + .wasSpecified = true, + }; + + EXPECT_EQ(resolveThresholdValue(absoluteParam, 84), 17u); +} + +TEST(IntrinThresholdResolveThresholdValueTest, PercentageTruncatesTowardZero) +{ + const ParsedThresholdParam percentageParam = { + .value = 33, + .unit = ThresholdUnit::Percentage, + .matchedName = "interest-pc", + .wasSpecified = true, + }; + + EXPECT_EQ(resolveThresholdValue(percentageParam, 100), 33u); + EXPECT_EQ(resolveThresholdValue(percentageParam, 10), 3u); +} + +TEST(IntrinThresholdSynonymCoverageTest, AllThrSynonymsParseAsAbsolute) +{ + const std::array thrNames = { + "interest-threshold", + "interest-thresh", + "interest-thr", + }; + + for (const std::string_view thrName : thrNames) + { + const ParamList params = makeParams({ + {thrName.data(), "11"}, + }); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Percentage); + + EXPECT_TRUE(parsed.wasSpecified); + EXPECT_EQ(parsed.unit, ThresholdUnit::Absolute); + EXPECT_EQ(parsed.value, 11); + EXPECT_EQ(parsed.matchedName, thrName); + } +} + +TEST(IntrinThresholdSynonymCoverageTest, AllPcSynonymsParseAsPercentage) +{ + const std::array pcNames = { + "interest-percentage", + "interest-pc", + }; + + for (const std::string_view pcName : pcNames) + { + const ParamList params = makeParams({ + {pcName.data(), "25"}, + }); + + const ParsedThresholdParam parsed = parseOptionalThresholdParam( + params, + kIntrinInterestPcNames, + kIntrinInterestThrNames, + 0, + ThresholdUnit::Absolute); + + EXPECT_TRUE(parsed.wasSpecified); + EXPECT_EQ(parsed.unit, ThresholdUnit::Percentage); + EXPECT_EQ(parsed.value, 25); + EXPECT_EQ(parsed.matchedName, pcName); + } +} + +} // namespace +} // namespace intrin +} // namespace smo diff --git a/include/user/intrinThresholdParams.h b/include/user/intrinThresholdParams.h index 5075c37..d6b4421 100644 --- a/include/user/intrinThresholdParams.h +++ b/include/user/intrinThresholdParams.h @@ -11,6 +11,8 @@ #include #include +#include + namespace smo::intrin { enum class ThresholdUnit @@ -97,35 +99,21 @@ inline bool arrayContains( const std::array& names, std::string_view candidate) { - for (const auto& name : names) { - if (name == candidate) { return true; } - } - - return false; -} - -template -inline bool namesContain( - const NameCollectionT& names, - std::string_view candidate) -{ - for (const auto& name : names) { - if (name == candidate) { return true; } - } - - return false; + return device::DeviceAttachmentSpec::namesContain(names, candidate); } inline bool isKnownIntrinThresholdParamName(std::string_view name) { - return namesContain(kIntrinInterestPcNames, name) - || namesContain(kIntrinInterestThrNames, name) - || namesContain(kIntrinDistractionPcNames, name) - || namesContain(kIntrinDistractionThrNames, name) - || namesContain(kIntrinStupefactionPcNames, name) - || namesContain(kIntrinStupefactionThrNames, name) - || namesContain(kIntrinIntolerablePcNames, name) - || namesContain(kIntrinIntolerableThrNames, name); + using device::DeviceAttachmentSpec; + + return DeviceAttachmentSpec::namesContain(kIntrinInterestPcNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinInterestThrNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinDistractionPcNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinDistractionThrNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinStupefactionPcNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinStupefactionThrNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinIntolerablePcNames, name) + || DeviceAttachmentSpec::namesContain(kIntrinIntolerableThrNames, name); } /* Intrin threshold params (interest-*, distraction-*, stupefaction-*, @@ -194,30 +182,6 @@ inline void validateIntrinSegmentParams( } } -inline std::optional> findLastMatchingIntParam( - const std::vector>& params, - const auto& synonymNames) -{ - for (auto paramIt = params.rbegin(); paramIt != params.rend(); ++paramIt) - { - const auto& [name, value] = *paramIt; - if (!namesContain(synonymNames, name)) - { continue; } - - try { - return std::pair(name, std::stoi(value)); - } - catch (const std::exception& e) - { - throw std::runtime_error( - "Failed to parse '" + name + "' param value '" + value - + "' as integer: " + e.what()); - } - } - - return std::nullopt; -} - inline ParsedThresholdParam parseOptionalThresholdParam( const std::vector>& params, const auto& percentageNames, @@ -230,28 +194,20 @@ inline ParsedThresholdParam parseOptionalThresholdParam( const auto& [name, value] = *paramIt; ThresholdUnit unit; - if (namesContain(percentageNames, name)) + if (device::DeviceAttachmentSpec::namesContain(percentageNames, name)) { unit = ThresholdUnit::Percentage; } - else if (namesContain(absoluteNames, name)) + else if (device::DeviceAttachmentSpec::namesContain(absoluteNames, name)) { unit = ThresholdUnit::Absolute; } else { continue; } - try - { - return ParsedThresholdParam{ - .value = std::stoi(value), - .unit = unit, - .matchedName = name, - .wasSpecified = true, - }; - } - catch (const std::exception& e) - { - throw std::runtime_error( - "Failed to parse '" + name + "' param value '" + value - + "' as integer: " + e.what()); - } + return ParsedThresholdParam{ + .value = device::DeviceAttachmentSpec::parseParamValueAsInt( + name, value), + .unit = unit, + .matchedName = name, + .wasSpecified = true, + }; } return ParsedThresholdParam{