From 31de101d53320dbcf23f39c869901bcca2324341 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Mon, 14 Feb 2022 17:29:08 -0500 Subject: [PATCH] refactor(head,common): commonize tool detect The tool detection will also be needed on the pipette. In addition to therefore needing the tool detection code to live in common/ somewhere, the pipettes also need to interact with the code in a different way - they don't care what kind of tool they are (or maybe do only as a debug), just what carrier they're on. In the same way, the head doesn't really care what carrier it reads, since it knows which pins it's looking at, just which tool it is. So we can provide separate filtering options for each and let each use of the code become simpler. Unfortunately ranges and views just don't work in clang at all because of https://github.com/llvm/llvm-project/issues/44178 so even though it all compiles correctly we have to do a pretty ugly hack that removes functionality when linting. It would be nice to not have this. --- common/CMakeLists.txt | 1 + common/core/CMakeLists.txt | 19 ++ common/core/tool_detection.cpp | 74 +++++++ common/tests/CMakeLists.txt | 3 +- common/tests/test_tool_detection.cpp | 107 ++++++++++ head/core/CMakeLists.txt | 10 +- head/core/tool_list.cpp | 37 ---- head/firmware/CMakeLists.txt | 2 +- head/firmware/main.cpp | 6 +- head/tests/CMakeLists.txt | 4 +- head/tests/test_presence_sensing_driver.cpp | 39 +--- head/tests/test_voltage_conversion.cpp | 7 +- include/common/core/tool_detection.hpp | 191 ++++++++++++++++++ include/head/core/attached_tools.hpp | 35 ++++ include/head/core/presence_sensing_driver.hpp | 17 +- .../tasks/presence_sensing_driver_task.hpp | 7 +- include/head/core/tool_list.hpp | 95 --------- 17 files changed, 458 insertions(+), 196 deletions(-) create mode 100644 common/core/CMakeLists.txt create mode 100644 common/core/tool_detection.cpp create mode 100644 common/tests/test_tool_detection.cpp delete mode 100644 head/core/tool_list.cpp create mode 100644 include/common/core/tool_detection.hpp create mode 100644 include/head/core/attached_tools.hpp delete mode 100644 include/head/core/tool_list.hpp diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 351bc9580..de8cb01f3 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -1,3 +1,4 @@ +add_subdirectory(core) if (${CMAKE_CROSSCOMPILING}) add_subdirectory(firmware) else() diff --git a/common/core/CMakeLists.txt b/common/core/CMakeLists.txt new file mode 100644 index 000000000..e188d5dac --- /dev/null +++ b/common/core/CMakeLists.txt @@ -0,0 +1,19 @@ +add_library(common-core STATIC tool_detection.cpp) +target_include_directories(common-core PUBLIC ${CMAKE_SOURCE_DIR}/include) +target_link_libraries(common-core PUBLIC can-core) + +set_target_properties(common-core + PROPERTIES CXX_STANDARD 20 + CXX_STANDARD_REQUIRED TRUE) + +target_compile_options(common-core + PRIVATE + -Wall + -Werror + -Weffc++ + -Wreorder + -Wsign-promo + -Wextra-semi + -Wctor-dtor-privacy + -fno-rtti + ) diff --git a/common/core/tool_detection.cpp b/common/core/tool_detection.cpp new file mode 100644 index 000000000..12f4e8a23 --- /dev/null +++ b/common/core/tool_detection.cpp @@ -0,0 +1,74 @@ +#include "common/core/tool_detection.hpp" + +#include +#include + +using namespace tool_detection; + +constexpr static auto pipette_384_chan_z_bounds = + ToolCheckBounds{.upper = 1883, .lower = 1851}; + +constexpr static auto pipette_96_chan_z_bounds = + ToolCheckBounds{.upper = 1434, .lower = 1400}; + +constexpr static auto pipette_single_chan_z_bounds = + ToolCheckBounds{.upper = 460, .lower = 444}; + +constexpr static auto pipette_multiple_chan_z_bounds = + ToolCheckBounds{.upper = 945, .lower = 918}; + +constexpr static auto pipette_single_chan_a_bounds = + ToolCheckBounds{.upper = 2389, .lower = 2362}; + +constexpr static auto pipette_multiple_chan_a_bounds = + ToolCheckBounds{.upper = 2860, .lower = 2844}; + +constexpr static auto nothing_connected_z_bounds = + ToolCheckBounds{.upper = 3, .lower = 1}; + +constexpr static auto nothing_connected_a_bounds = + ToolCheckBounds{.upper = 54, .lower = 16}; + +// revisit these, not sure if EE has a calculation for gripper carrier bounds +constexpr static auto nothing_connected_gripper_bounds = + ToolCheckBounds{.upper = 54, .lower = 16}; + +constexpr static auto gripper_bounds = + ToolCheckBounds{.upper = 999, .lower = 536}; + +constexpr static auto undefined_bounds = + ToolCheckBounds{.upper = 0, .lower = 0}; + +static std::array tool_list(std::to_array( + {Tool{.tool_type = can_ids::ToolType::pipette_384_chan, + .tool_carrier = Z_CARRIER, + .bounds = pipette_384_chan_z_bounds}, + Tool{.tool_type = can_ids::ToolType::pipette_96_chan, + .tool_carrier = Z_CARRIER, + .bounds = pipette_96_chan_z_bounds}, + Tool{.tool_type = can_ids::ToolType::pipette_single_chan, + .tool_carrier = Z_CARRIER, + .bounds = pipette_single_chan_z_bounds}, + Tool{.tool_type = can_ids::ToolType::pipette_multi_chan, + .tool_carrier = Z_CARRIER, + .bounds = pipette_multiple_chan_z_bounds}, + Tool{.tool_type = can_ids::ToolType::nothing_attached, + .tool_carrier = Z_CARRIER, + .bounds = nothing_connected_z_bounds}, + Tool{.tool_type = can_ids::ToolType::pipette_single_chan, + .tool_carrier = A_CARRIER, + .bounds = pipette_single_chan_a_bounds}, + Tool{.tool_type = can_ids::ToolType::pipette_multi_chan, + .tool_carrier = A_CARRIER, + .bounds = pipette_multiple_chan_a_bounds}, + Tool{.tool_type = can_ids::ToolType::nothing_attached, + .tool_carrier = A_CARRIER, + .bounds = nothing_connected_a_bounds}, + Tool{.tool_type = can_ids::ToolType::gripper, + .tool_carrier = GRIPPER_CARRIER, + .bounds = gripper_bounds}, + Tool{.tool_type = can_ids::ToolType::nothing_attached, + .tool_carrier = GRIPPER_CARRIER, + .bounds = nothing_connected_gripper_bounds}})); + +std::span tool_detection::lookup_table() { return std::span(tool_list); } diff --git a/common/tests/CMakeLists.txt b/common/tests/CMakeLists.txt index ba64097c0..271d9f541 100644 --- a/common/tests/CMakeLists.txt +++ b/common/tests/CMakeLists.txt @@ -10,6 +10,7 @@ add_executable( test_bit_utils.cpp test_synchronization.cpp test_spi.cpp + test_tool_detection.cpp ) target_include_directories(common PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/../../include) @@ -28,7 +29,7 @@ target_compile_options(common -Wextra-semi -Wctor-dtor-privacy -fno-rtti) -target_link_libraries(common Catch2::Catch2 common-simulation) +target_link_libraries(common Catch2::Catch2 common-simulation common-core) catch_discover_tests(common) add_build_and_test_target(common) diff --git a/common/tests/test_tool_detection.cpp b/common/tests/test_tool_detection.cpp new file mode 100644 index 000000000..d5b62067e --- /dev/null +++ b/common/tests/test_tool_detection.cpp @@ -0,0 +1,107 @@ +#include +#include +#include + +#include "can/core/ids.hpp" +#include "catch2/catch.hpp" +#include "common/core/tool_detection.hpp" + +// Filter tests - no BDD here because these macros don't have +// BDD equivalents +TEMPLATE_TEST_CASE_SIG("tool list by pipette type", "", + ((can_ids::ToolType TT), TT), + (can_ids::ToolType::pipette_96_chan), + (can_ids::ToolType::pipette_384_chan), + (can_ids::ToolType::pipette_single_chan), + (can_ids::ToolType::pipette_multi_chan), + (can_ids::ToolType::gripper), + (can_ids::ToolType::nothing_attached)) { + SECTION("lookup table filtering") { + auto tool_only = tool_detection::lookup_table_filtered(TT); + bool checked_anything = false; + for (auto tool_elem : tool_only) { + checked_anything = true; + REQUIRE(tool_elem.tool_type == TT); + } + CHECK(checked_anything); + } +} + +TEMPLATE_TEST_CASE_SIG("tool list by carrier", "", + ((tool_detection::Carrier C), C), + (tool_detection::Carrier::GRIPPER_CARRIER), + (tool_detection::Carrier::A_CARRIER), + (tool_detection::Carrier::Z_CARRIER)) { + auto carrier_only = tool_detection::lookup_table_filtered(C); + bool checked_anything = false; + for (auto tool_elem : carrier_only) { + checked_anything = true; + REQUIRE(tool_elem.tool_carrier == C); + } + CHECK(checked_anything); +} + +SCENARIO("ot3 tools list for tool detection") { + GIVEN("ot3 tools lower and upper voltage bounds for tool detection") { + auto tools = tool_detection::lookup_table(); + WHEN("when lower bound compared with upper bound") { + THEN("lower bound is always found lower or equal to upper bounds") { + for (auto& element : tools) { + REQUIRE(element.bounds.lower <= element.bounds.upper); + } + } + } + WHEN("you pass in an out-of-bounds read") { + uint16_t reading = 0; + THEN("the tool and carrier are undefined") { + REQUIRE(tool_detection::tooltype_from_reading(reading) == + can_ids::ToolType::undefined_tool); + REQUIRE(tool_detection::carrier_from_reading(reading) == + tool_detection::Carrier::UNKNOWN); + } + } + WHEN("checking holes in the bounds set for tool mounts") { + auto which_tool = GENERATE(tool_detection::Carrier::A_CARRIER, + tool_detection::Carrier::Z_CARRIER); + // can't modify the lookup table, gotta copy + auto filtered = tool_detection::lookup_table_filtered(which_tool); + auto copied = std::vector(filtered.begin(), filtered.end()); + std::sort(copied.begin(), copied.end(), + [](const tool_detection::Tool& a, + const tool_detection::Tool& b) { + return a.bounds.upper < b.bounds.upper; + }); + auto trailer = + std::ranges::subrange(copied.begin(), --copied.end()); + auto leader = std::ranges::subrange(++copied.begin(), copied.end()); + for (auto trailer_it = trailer.begin(), leader_it = leader.begin(); + trailer_it != trailer.end() && leader_it != leader.end(); + trailer_it++, leader_it++) { + INFO("Checking tool detection for carrier " + << static_cast(which_tool) << ", tool type " + << static_cast(trailer_it->tool_type) << " (" + << trailer_it->bounds.lower << "->" + << trailer_it->bounds.upper << "mV) -> " + << static_cast(leader_it->tool_type) << " (" + << leader_it->bounds.lower << "->" + << leader_it->bounds.upper << "mV)"); + // Just above the upper range of the first + REQUIRE(tool_detection::tooltype_from_reading( + trailer_it->bounds.upper, filtered) == + can_ids::ToolType::undefined_tool); + // Just below the lower range of the second + REQUIRE(tool_detection::tooltype_from_reading( + leader_it->bounds.lower - 1, filtered) == + can_ids::ToolType::undefined_tool); + // Dead-on in the middle + auto mid_value = + (trailer_it->bounds.upper + leader_it->bounds.lower) / 2; + INFO("Bounds gap mid value " << mid_value + << " matched to a tool"); + REQUIRE(tool_detection::tooltype_from_reading(mid_value, + filtered) == + can_ids::ToolType::undefined_tool); + } + } + } +} diff --git a/head/core/CMakeLists.txt b/head/core/CMakeLists.txt index 601e4850f..606f6c729 100644 --- a/head/core/CMakeLists.txt +++ b/head/core/CMakeLists.txt @@ -1,13 +1,7 @@ function(target_head_core TARGET) target_sources(${TARGET} PUBLIC - ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/tool_list.cpp ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/tasks.cpp ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/can_task.cpp) - target_include_directories(${TARGET} PUBLIC ${CMAKE_CURRENT_FUNCTION_LIST_DIR}) -endfunction() - -function(target_head_core_tool_list TARGET) - target_sources(${TARGET} PUBLIC - ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/tool_list.cpp) - target_include_directories(${TARGET} PUBLIC ${CMAKE_CURRENT_FUNCTION_LIST_DIR}) + target_include_directories(${TARGET} PUBLIC ${CMAKE_CURRENT_FUNCTION_LIST_DIR}) + target_link_libraries(${TARGET} PUBLIC common-core) endfunction() diff --git a/head/core/tool_list.cpp b/head/core/tool_list.cpp deleted file mode 100644 index 2c301ec1d..000000000 --- a/head/core/tool_list.cpp +++ /dev/null @@ -1,37 +0,0 @@ - - -#include "head/core/tool_list.hpp" -using namespace ot3_tool_list; - -static ot3_tool_list::OT3Tools _list{ - {{.tool_type = can_ids::ToolType::pipette_384_chan, - .tool_carrier = Z_CARRIER, - .bounds = pipette_384_chan_z_bounds}, - {.tool_type = can_ids::ToolType::pipette_96_chan, - .tool_carrier = Z_CARRIER, - .bounds = pipette_96_chan_z_bounds}, - {.tool_type = can_ids::ToolType::pipette_single_chan, - .tool_carrier = Z_CARRIER, - .bounds = pipette_single_chan_z_bounds}, - {.tool_type = can_ids::ToolType::pipette_multi_chan, - .tool_carrier = Z_CARRIER, - .bounds = pipette_multiple_chan_z_bounds}, - {.tool_type = can_ids::ToolType::pipette_single_chan, - .tool_carrier = A_CARRIER, - .bounds = pipette_single_chan_a_bounds}, - {.tool_type = can_ids::ToolType::pipette_multi_chan, - .tool_carrier = A_CARRIER, - .bounds = pipette_multiple_chan_a_bounds}, - {.tool_type = can_ids::ToolType::gripper, - .tool_carrier = GRIPPER_CARRIER, - .bounds = gripper_bounds}, - {.tool_type = can_ids::ToolType::nothing_attached, - .tool_carrier = Z_CARRIER, - .bounds = nothing_connected_z_bounds}, - {.tool_type = can_ids::ToolType::nothing_attached, - .tool_carrier = A_CARRIER, - .bounds = nothing_connected_a_bounds}, - {.tool_type = can_ids::ToolType::nothing_attached, - .tool_carrier = GRIPPER_CARRIER, - .bounds = nothing_connected_gripper_bounds}}}; -auto ot3_tool_list::get_tool_list() -> const OT3Tools& { return _list; } \ No newline at end of file diff --git a/head/firmware/CMakeLists.txt b/head/firmware/CMakeLists.txt index 20409b9d9..f7087785e 100644 --- a/head/firmware/CMakeLists.txt +++ b/head/firmware/CMakeLists.txt @@ -129,7 +129,7 @@ list(TRANSFORM CLANG_EXTRA_ARGS PREPEND --extra-arg=-I) list(APPEND CLANG_EXTRA_ARGS "--extra-arg=-frelaxed-template-template-args") add_custom_target(head-lint ALL - COMMAND ${Clang_CLANGTIDY_EXECUTABLE} ${CLANG_EXTRA_ARGS} -p ${CMAKE_BINARY_DIR} ${HEAD_FW_LINTABLE_SRCS} ${CORE_LINTABLE_SOURCES}) + COMMAND ${Clang_CLANGTIDY_EXECUTABLE} "--extra-arg=-DOPENTRONS_CLANG_TIDY_WORKAROUND_44178" ${CLANG_EXTRA_ARGS} -p ${CMAKE_BINARY_DIR} ${HEAD_FW_LINTABLE_SRCS} ${CORE_LINTABLE_SOURCES}) list(APPEND LINT_TARGETS head-lint) set(LINT_TARGETS ${LINT_TARGETS} PARENT_SCOPE) # Runs cross-gdb (since CMAKE_CROSSCOMPILING_EMULATOR is set in an diff --git a/head/firmware/main.cpp b/head/firmware/main.cpp index e0cf8a75f..45d65d03a 100644 --- a/head/firmware/main.cpp +++ b/head/firmware/main.cpp @@ -23,7 +23,6 @@ #include "common/firmware/spi_comms.hpp" #include "head/core/presence_sensing_driver.hpp" #include "head/core/tasks.hpp" -#include "head/core/tool_list.hpp" #include "head/firmware/adc_comms.hpp" #include "motor-control/core/linear_motion_system.hpp" #include "motor-control/core/motor.hpp" @@ -193,10 +192,7 @@ adc::ADC_interface ADC_intf1 = { static auto ADC_comms = adc::ADC(ADC_intf1, ADC_intf2); -static auto attached_tools = ot3_tool_list::AttachedTool{}; - -static auto psd = - presence_sensing_driver::PresenceSensingDriver{ADC_comms, attached_tools}; +static auto psd = presence_sensing_driver::PresenceSensingDriver{ADC_comms}; auto timer_for_notifier = freertos_timer::FreeRTOSTimer( "timer for notifier", ([] { diff --git a/head/tests/CMakeLists.txt b/head/tests/CMakeLists.txt index 45e751d56..ac8e3e8a8 100644 --- a/head/tests/CMakeLists.txt +++ b/head/tests/CMakeLists.txt @@ -30,9 +30,7 @@ target_compile_options(head -Wctor-dtor-privacy -fno-rtti) -target_head_core_tool_list(head) - -target_link_libraries(head Catch2::Catch2 common-simulation) +target_link_libraries(head PUBLIC Catch2::Catch2 common-simulation common-core) catch_discover_tests(head) add_build_and_test_target(head) diff --git a/head/tests/test_presence_sensing_driver.cpp b/head/tests/test_presence_sensing_driver.cpp index a164adcaa..b1ac89b4f 100644 --- a/head/tests/test_presence_sensing_driver.cpp +++ b/head/tests/test_presence_sensing_driver.cpp @@ -1,4 +1,5 @@ #include "catch2/catch.hpp" +#include "common/core/tool_detection.hpp" #include "head/core/presence_sensing_driver.hpp" #include "head/tests/mock_adc.hpp" @@ -8,9 +9,7 @@ SCENARIO("get readings called on presence sensing driver") { adc::RawADCReadings{.z_motor = 666, .a_motor = 666, .gripper = 666}; static auto adc_comms = adc::MockADC{raw_readings}; - auto at = ot3_tool_list::AttachedTool{}; - - auto ps = presence_sensing_driver::PresenceSensingDriver(adc_comms, at); + auto ps = presence_sensing_driver::PresenceSensingDriver(adc_comms); WHEN("get_readings func is called") { auto voltage_readings = ps.get_readings(); @@ -40,10 +39,9 @@ SCENARIO("get_tool called on presence sensing driver") { auto raw_readings = adc::RawADCReadings{ .z_motor = 2332, .a_motor = 3548, .gripper = 666}; static auto adc_comms = adc::MockADC{raw_readings}; - auto ps = presence_sensing_driver::PresenceSensingDriver( - adc_comms, ot3_tool_list::AttachedTool{}); - auto tools = ot3_tool_list::AttachedTool( - ps.get_readings(), ot3_tool_list::get_tool_list()); + auto ps = presence_sensing_driver::PresenceSensingDriver(adc_comms); + auto tools = attached_tools::AttachedTools( + ps.get_readings(), tool_detection::lookup_table()); THEN("Tools mapped to voltage reading") { REQUIRE(tools.gripper == can_ids::ToolType::gripper); @@ -55,10 +53,9 @@ SCENARIO("get_tool called on presence sensing driver") { auto raw_readings = adc::RawADCReadings{ .z_motor = 9999, .a_motor = 9999, .gripper = 9999}; static auto adc_comms = adc::MockADC{raw_readings}; - auto ps = presence_sensing_driver::PresenceSensingDriver( - adc_comms, ot3_tool_list::AttachedTool{}); - auto tools = ot3_tool_list::AttachedTool( - ps.get_readings(), ot3_tool_list::get_tool_list()); + auto ps = presence_sensing_driver::PresenceSensingDriver(adc_comms); + auto tools = attached_tools::AttachedTools( + ps.get_readings(), tool_detection::lookup_table()); THEN("Tools mapped to voltage reading") { REQUIRE(tools.gripper == can_ids::ToolType::undefined_tool); @@ -72,10 +69,9 @@ SCENARIO("get_tool called on presence sensing driver") { auto raw_readings = adc::RawADCReadings{.z_motor = 2, .a_motor = 20, .gripper = 20}; static auto adc_comms = adc::MockADC{raw_readings}; - auto ps = presence_sensing_driver::PresenceSensingDriver( - adc_comms, ot3_tool_list::AttachedTool{}); - auto tools = ot3_tool_list::AttachedTool( - ps.get_readings(), ot3_tool_list::get_tool_list()); + auto ps = presence_sensing_driver::PresenceSensingDriver(adc_comms); + auto tools = attached_tools::AttachedTools( + ps.get_readings(), tool_detection::lookup_table()); THEN("Tools mapped to voltage reading") { REQUIRE(tools.gripper == can_ids::ToolType::nothing_attached); @@ -85,16 +81,3 @@ SCENARIO("get_tool called on presence sensing driver") { } } } - -SCENARIO("ot3 tools list for tool detection") { - GIVEN("ot3 tools lower and upper voltage bounds for tool detection") { - auto tools = ot3_tool_list::get_tool_list(); - WHEN("when lower bound compared with upper bound") { - THEN("lower bound is always found lower or equal to upper bounds") { - for (auto& element : tools) { - REQUIRE(element.bounds.lower <= element.bounds.upper); - } - } - } - } -} \ No newline at end of file diff --git a/head/tests/test_voltage_conversion.cpp b/head/tests/test_voltage_conversion.cpp index 44e6c13d8..4fb09a03c 100644 --- a/head/tests/test_voltage_conversion.cpp +++ b/head/tests/test_voltage_conversion.cpp @@ -2,15 +2,12 @@ #include "head/core/presence_sensing_driver.hpp" #include "head/tests/mock_adc.hpp" -using namespace ot3_tool_list; - SCENARIO("Raw ADC readings to voltage conversions") { GIVEN("Raw readings and a PresenceSensingDriver instance") { auto raw_readings = adc::RawADCReadings{.z_motor = 666, .a_motor = 666, .gripper = 666}; static auto adc_comms = adc::MockADC{raw_readings}; - auto ps = presence_sensing_driver::PresenceSensingDriver( - adc_comms, AttachedTool{}); + auto ps = presence_sensing_driver::PresenceSensingDriver(adc_comms); WHEN("get_readings function is called") { auto voltage_readings = ps.get_readings(); @@ -21,4 +18,4 @@ SCENARIO("Raw ADC readings to voltage conversions") { } } } -} \ No newline at end of file +} diff --git a/include/common/core/tool_detection.hpp b/include/common/core/tool_detection.hpp new file mode 100644 index 000000000..4616caa95 --- /dev/null +++ b/include/common/core/tool_detection.hpp @@ -0,0 +1,191 @@ +#pragma once +#include +#include +#include + +#include "can/core/ids.hpp" + +namespace tool_detection { + +// A quick specialization that is syntactically meaningless but a check +// to the programmer +using millivolts_t = uint16_t; + +// Contains the bounds for value checking +struct ToolCheckBounds { + millivolts_t upper; + millivolts_t lower; +}; + +// Which carrier a device is attached to +enum Carrier : uint8_t { + GRIPPER_CARRIER = 0x00, + Z_CARRIER = 0x01, + A_CARRIER = 0x02, + UNKNOWN = 0x03, +}; + +// A full specification of an attached tool, including its bounds +struct Tool { + can_ids::ToolType tool_type; + Carrier tool_carrier; + ToolCheckBounds bounds; + [[nodiscard]] auto within_bounds(millivolts_t reading) const -> bool { + return ((reading >= this->bounds.lower) && + (reading < this->bounds.upper)); + return false; + } +}; + +// Get a view on the lookup table for bounds -> tool values, including versions +// with filtering for cases where you only care about one carrier or tool type. +// This can be useful because if you only have a specific set of physically +// reasonable outcomes - for instance, it's not reasonable to see a tool +// physically attached to the A mount pins having Z mount values - the +// filtering prevents such strange values from happening +auto lookup_table() -> std::span; + + +#if !defined(OPENTRONS_CLANG_TIDY_WORKAROUND_44178) +// Clang 12 and 13 and head currently have a bug in the way they handle specific +// uses of template concepts that stems from a disagreement about interpretation +// of standards language between them, and, well, everybody else. This bug +// completely breaks the compilation of half of std::ranges, which we use extensively +// here. The bug: https://github.com/llvm/llvm-project/issues/44178 +// Since we only ever compile tests and firmware and simulation with gcc, it's not +// really a problem, but it does break linting, which uses clang-tidy and therefore +// clang. Our options are limited here; the only way I could see to actually get +// to use this code is to have dummy versions of the functions here that get used +// during lint. + +// Limit filter options to one of the two available pieces of metadata +template +concept ToolOrCarrier = + std::is_same_v || std::is_same_v; + +// Looks like a linear container and contains a Tool +template +concept ToolList = std::ranges::view && + std::is_same_v>, Tool>; + +// Lookup table retrieval with filtering. Does the same thing as lookup_table(), +// but with a return type that is annoying enough to deal with that we let it be +// fully auto deduced. +// +// Can specify your own base lookup table (including, possibly, the output of +// another filter function call) or load from the default one. +template +auto lookup_table_filtered(FilterTarget filter_by) { + return filter_lookup_table(filter_by, lookup_table()); +} + +// This is a template overload set, where the template argument decides the +// lookup table type (necessary to deal with the std::views stuff) and the +// overloading handles the two different filter types, which need to access +// different struct members +template +auto filter_lookup_table(can_ids::ToolType filter_by, Lookup from_table) { + return std::views::filter(from_table, [filter_by](const auto& elem) { + return elem.tool_type == filter_by; + }); +}; + +template +auto filter_lookup_table(Carrier filter_by, Lookup from_table) { + return std::views::filter(from_table, [filter_by](const auto& elem) { + return elem.tool_carrier == filter_by; + }); +} + +// Get a full tool specification. If the lookup table isn't specified, the +// full builtin table is used. +template +auto tool_from_reading(millivolts_t reading, Lookup with_lookup) -> Tool { + for (const auto& element : with_lookup) { + if (element.within_bounds(reading)) { + return element; + } + } + return Tool{.tool_type = can_ids::ToolType::undefined_tool, + .tool_carrier = Carrier::UNKNOWN, + .bounds = {0, 0}}; +} + +inline auto tool_from_reading(millivolts_t reading) -> Tool { + return tool_from_reading(reading, lookup_table()); +} + +// Turn a reading into specifically the carrier holding the tool. If the +// lookup table isn't specified, the full built in table is used. +template +auto carrier_from_reading(millivolts_t reading, Lookup with_lookup) -> Carrier { + return tool_from_reading(reading, with_lookup).tool_carrier; +}; +inline auto carrier_from_reading(millivolts_t reading) -> Carrier { + return carrier_from_reading(reading, lookup_table()); +} + +// Turn a reading into specifically the tool type attached. If the +// lookup table isn't specified, the full built in table is used. +template +auto tooltype_from_reading(millivolts_t reading, + Lookup with_lookup) -> can_ids::ToolType { + return tool_from_reading(reading, with_lookup).tool_type; +}; + +inline auto tooltype_from_reading(millivolts_t reading) -> can_ids::ToolType { + return tooltype_from_reading(reading, lookup_table()); +} +#else // defined(OPENTRONS_CLANG_TIDY_WORKAROUND_44178) +template +auto lookup_table_filtered(FilterTarget filter_by) { + return filter_lookup_table(filter_by, lookup_table()); +} + +template +auto filter_lookup_table(can_ids::ToolType filter_by, Lookup from_table) { + return from_table; +}; + +template +auto filter_lookup_table(Carrier filter_by, Lookup from_table) { + return from_table; +} + +template +auto tool_from_reading(millivolts_t reading, Lookup with_lookup) -> Tool { + for (const auto& element : with_lookup) { + if (element.within_bounds(reading)) { + return element; + } + } + return Tool{.tool_type = can_ids::ToolType::undefined_tool, + .tool_carrier = Carrier::UNKNOWN, + .bounds = {0, 0}}; +} + +inline auto tool_from_reading(millivolts_t reading) -> Tool { + return tool_from_reading(reading, lookup_table()); +} + +template +auto carrier_from_reading(millivolts_t reading, Lookup with_lookup) -> Carrier { + return tool_from_reading(reading, with_lookup).tool_carrier; +}; +inline auto carrier_from_reading(millivolts_t reading) -> Carrier { + return carrier_from_reading(reading, lookup_table()); +} + +template +auto tooltype_from_reading(millivolts_t reading, + Lookup with_lookup) -> can_ids::ToolType { + return tool_from_reading(reading, with_lookup).tool_type; +}; + +inline auto tooltype_from_reading(millivolts_t reading) -> can_ids::ToolType { + return tooltype_from_reading(reading, lookup_table()); +} +#endif // defined(OPENTRONS_CLANG_TIDY_WORKAROUND_44178) + + +}; // namespace tool_detection diff --git a/include/head/core/attached_tools.hpp b/include/head/core/attached_tools.hpp new file mode 100644 index 000000000..442d3d0fb --- /dev/null +++ b/include/head/core/attached_tools.hpp @@ -0,0 +1,35 @@ +#pragma once +#include +#include + +#include "can/core/ids.hpp" +#include "common/core/tool_detection.hpp" +#include "head/core/adc.hpp" + +namespace attached_tools { + +using namespace tool_detection; + +struct AttachedTools { + can_ids::ToolType z_motor = can_ids::ToolType::undefined_tool; + can_ids::ToolType a_motor = can_ids::ToolType::undefined_tool; + can_ids::ToolType gripper = can_ids::ToolType::undefined_tool; + AttachedTools() = default; + AttachedTools(adc::MillivoltsReadings reading) + : z_motor(tooltype_from_reading( + reading.z_motor, lookup_table_filtered(Carrier::Z_CARRIER))), + a_motor(tooltype_from_reading( + reading.a_motor, lookup_table_filtered(Carrier::A_CARRIER))), + gripper(tooltype_from_reading( + reading.gripper, + lookup_table_filtered(Carrier::GRIPPER_CARRIER))) {} + + AttachedTools(adc::MillivoltsReadings reading, std::span arr) + : z_motor(tooltype_from_reading(reading.z_motor, arr)), + a_motor(tooltype_from_reading(reading.a_motor, arr)), + gripper(tooltype_from_reading(reading.gripper, arr)) { + {} + } +}; + +} // namespace attached_tools diff --git a/include/head/core/presence_sensing_driver.hpp b/include/head/core/presence_sensing_driver.hpp index fa9f78a07..6950f996f 100644 --- a/include/head/core/presence_sensing_driver.hpp +++ b/include/head/core/presence_sensing_driver.hpp @@ -5,16 +5,17 @@ #include "can/core/ids.hpp" #include "common/core/bit_utils.hpp" #include "head/core/adc.hpp" -#include "head/core/tool_list.hpp" +#include "head/core/attached_tools.hpp" -using namespace ot3_tool_list; namespace presence_sensing_driver { using namespace can_ids; class PresenceSensingDriver { public: + explicit PresenceSensingDriver(adc::BaseADC& adc) + : PresenceSensingDriver(adc, attached_tools::AttachedTools{}) {} PresenceSensingDriver(adc::BaseADC& adc, - ot3_tool_list::AttachedTool current_tools) + attached_tools::AttachedTools current_tools) : adc_comms(adc), current_tools(current_tools) {} auto get_readings() -> adc::MillivoltsReadings { auto RawReadings = adc_comms.get_readings(); @@ -33,19 +34,17 @@ class PresenceSensingDriver { adc::ADC_FULLSCALE_OUTPUT)}; return voltage_read; } - auto get_current_tools() -> ot3_tool_list::AttachedTool { + auto get_current_tools() -> attached_tools::AttachedTools { return this->current_tools; } - void set_current_tools(ot3_tool_list::AttachedTool tmp) { - this->current_tools.z_motor = tmp.z_motor; - this->current_tools.a_motor = tmp.a_motor; - this->current_tools.gripper = tmp.gripper; + void set_current_tools(attached_tools::AttachedTools tools) { + this->current_tools = tools; } private: adc::BaseADC& adc_comms; - ot3_tool_list::AttachedTool current_tools; + attached_tools::AttachedTools current_tools; }; } // namespace presence_sensing_driver diff --git a/include/head/core/tasks/presence_sensing_driver_task.hpp b/include/head/core/tasks/presence_sensing_driver_task.hpp index e12dcc9f0..9e5ced1c4 100644 --- a/include/head/core/tasks/presence_sensing_driver_task.hpp +++ b/include/head/core/tasks/presence_sensing_driver_task.hpp @@ -7,10 +7,9 @@ #include "can/core/messages.hpp" #include "common/core/logging.hpp" #include "head/core/adc.hpp" +#include "head/core/attached_tools.hpp" #include "head/core/presence_sensing_driver.hpp" -#include "head/core/tool_list.hpp" -using namespace ot3_tool_list; namespace presence_sensing_driver_task { using TaskMessage = @@ -59,7 +58,7 @@ class PresenceSensingDriverMessageHandler { } void visit(can_messages::AttachedToolsRequest& m) { - auto tools = AttachedTool(driver.get_readings(), get_tool_list()); + auto tools = attached_tools::AttachedTools(driver.get_readings()); if (tools.z_motor != driver.get_current_tools().z_motor || tools.a_motor != driver.get_current_tools().a_motor || tools.gripper != driver.get_current_tools().gripper) { @@ -130,4 +129,4 @@ concept TaskClient = requires(Client client, const TaskMessage& m) { {client.send_presence_sensing_driver_queue(m)}; }; -} // namespace presence_sensing_driver_task \ No newline at end of file +} // namespace presence_sensing_driver_task diff --git a/include/head/core/tool_list.hpp b/include/head/core/tool_list.hpp deleted file mode 100644 index 9f89ff00d..000000000 --- a/include/head/core/tool_list.hpp +++ /dev/null @@ -1,95 +0,0 @@ -#pragma once -#include -#include - -#include "can/core/ids.hpp" -#include "head/core/adc.hpp" - -namespace ot3_tool_list { - -using namespace can_ids; - -struct ToolCheckBounds { - uint16_t upper; - uint16_t lower; -}; - -constexpr auto pipette_384_chan_z_bounds = - ToolCheckBounds{.upper = 1883, .lower = 1851}; - -constexpr auto pipette_96_chan_z_bounds = - ToolCheckBounds{.upper = 1434, .lower = 1400}; - -constexpr auto pipette_single_chan_z_bounds = - ToolCheckBounds{.upper = 460, .lower = 444}; - -constexpr auto pipette_multiple_chan_z_bounds = - ToolCheckBounds{.upper = 945, .lower = 918}; - -constexpr auto pipette_single_chan_a_bounds = - ToolCheckBounds{.upper = 2389, .lower = 2362}; - -constexpr auto pipette_multiple_chan_a_bounds = - ToolCheckBounds{.upper = 2860, .lower = 2844}; - -constexpr auto nothing_connected_z_bounds = - ToolCheckBounds{.upper = 3, .lower = 1}; - -constexpr auto nothing_connected_a_bounds = - ToolCheckBounds{.upper = 54, .lower = 16}; - -// revisit these, not sure if EE has a calculation for gripper carrier bounds -constexpr auto nothing_connected_gripper_bounds = - ToolCheckBounds{.upper = 54, .lower = 16}; - -constexpr auto gripper_bounds = ToolCheckBounds{.upper = 999, .lower = 536}; - -constexpr auto undefined_bounds = ToolCheckBounds{.upper = 0, .lower = 0}; - -enum Carrier : uint8_t { - GRIPPER_CARRIER = 0x00, - Z_CARRIER = 0x01, - A_CARRIER = 0x02, -}; - -struct Tool { - ToolType tool_type; - Carrier tool_carrier; - ToolCheckBounds bounds; - [[nodiscard]] auto within_bounds(uint16_t reading) const -> bool { - return ((reading >= this->bounds.lower) && - (reading < this->bounds.upper)); - return false; - } -}; - -using OT3Tools = std::array; - -auto get_tool_list() -> const OT3Tools&; -struct AttachedTool { - ToolType z_motor{}; - ToolType a_motor{}; - ToolType gripper{}; - AttachedTool() - : z_motor(can_ids::ToolType::undefined_tool), - a_motor(can_ids::ToolType::undefined_tool), - gripper(can_ids::ToolType::undefined_tool) {} - AttachedTool(adc::MillivoltsReadings reading, const OT3Tools& arr) { - for (const auto& element : arr) { - if (element.within_bounds(reading.z_motor) && - (element.tool_carrier == Z_CARRIER)) { - this->z_motor = element.tool_type; - } - if (element.within_bounds(reading.a_motor) && - (element.tool_carrier == A_CARRIER)) { - this->a_motor = element.tool_type; - } - if (element.within_bounds(reading.gripper) && - (element.tool_carrier == GRIPPER_CARRIER)) { - this->gripper = element.tool_type; - } - } - } -}; - -} // namespace ot3_tool_list \ No newline at end of file