feat(pipette): Detect mount at boot and assign node id #246

sfoster1 · 2022-02-15T22:17:52Z

Bring up the ability for the pipette to decide which node ID it should use based on the tool mount voltage it senses, reusing code from the similar implementation for head.

For code reusability, the first couple commits commonize first the tool detection (using some fun ranges and filter views which don't work with clang at all, see the comments and the commit message) and then a basic adc interface since the needs are so similar. Once those are done, the work to actually do the sensing on the pipette is surprisingly small.

~~Still some todos, but definitely reviewable.~~ Does the bootloader thing now.

TODO:

~~Test on hardware~~ works great! moving a pipette back and forth between the mounts and probing shows it reliably switching.
~~Figure out what to do about the bootloaders~~ add separate implementation and regression tests to avoid dealing with different enum types

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 llvm/llvm-project#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.

There's some ADC stuff that's different between devices (well, a lot of ADC stuff) but the reading->mv transition isn't, so we might as well commonize it. The core element here is an ADC channel, which is templatized with the conversion variables. That lets it do some nice conversions, and with some required implementations in the child we can have a nice generalizable interface. There isn't an ADC interface in common, because that's going to go a bunch of different places - the head has one, for instance, that specifically has gripper, z, and a channels - nothing else needs that. It relies on the common channel implementation, and the simulation and test versions override as appropriate.

Based on the voltage present on the tool sense pin and using the ADC and mount detection utilities previously added to common/, we can now pretty easily decide which mount we're on and assign ourselves a canbus id based on that.

Laura-Danielle · 2022-02-15T22:23:56Z

common/core/tool_detection.cpp

+
+using namespace tool_detection;
+
+constexpr static auto pipette_384_chan_z_bounds =


I thought we were only going to be detecting differences between gripper vs pipette vs no instrument attached

We have the means to figure out the type of pipette -this sheet shows the different expected voltages

@Laura-Danielle we'll probably go that direction eventually but this part was already implemented so I stuck with it

furthermore, a is right and z is left

This unfortunately reimplements the mount detection algorithm in C but it will work just fine (and is checked by unit tests). Bootloaders should now properly decide which mount they're on.

This flash command had the wrong openocd config value

amitlissack · 2022-02-16T20:11:06Z

bootloader/firmware/stm32L5/CMakeLists.txt

@@ -83,7 +84,7 @@ macro(target_stm32L5 TARGET)

    # Runs openocd to flash the board (without using a debugger)
    add_custom_target(${TARGET}-flash
-            COMMAND "${OpenOCD_EXECUTABLE}" "-f" "${COMMON_EXECUTABLE_DIR}/STM32G491RETx/stm32g4discovery.cfg" "-c" "program $<TARGET_FILE:${TARGET}>;reset;exit"
+            COMMAND "${OpenOCD_EXECUTABLE}" "-f" "${COMMON_EXECUTABLE_DIR}/STM32L562RETx/stm32l5discovery.cfg" "-c" "program $<TARGET_FILE:${TARGET}>;reset;exit"


amitlissack · 2022-02-16T20:15:19Z

common/tests/test_tool_detection.cpp

+
+// Filter tests - no BDD here because these macros don't have
+// BDD equivalents
+TEMPLATE_TEST_CASE_SIG("tool list by pipette type", "",


amitlissack · 2022-02-16T20:17:49Z

include/common/core/tool_detection.hpp

+    [[nodiscard]] auto within_bounds(millivolts_t reading) const -> bool {
+        return ((reading >= this->bounds.lower) &&
+                (reading < this->bounds.upper));
+        return false;


Why two returns? Am I crazy?

What how did that get there why did no tooling care about it gah

amitlissack · 2022-02-16T20:19:08Z

include/common/core/tool_detection.hpp

+//
+// Can specify your own base lookup table (including, possibly, the output of
+// another filter function call) or load from the default one.
+template <ToolOrCarrier FilterTarget>


You are a real macher, aren't you?

amitlissack · 2022-02-16T20:21:02Z

pipettes/firmware/main.cpp

@@ -136,7 +139,7 @@ auto main() -> int {
    pipettes_tasks::start_tasks(can_bus_1, pipette_motor.motion_controller,
                                pipette_motor.driver, i2c_comms,
                                // TODO: Load from mount interface


You don't need to TODO this anymore, innit?

sure don't!

amitlissack

Lovely

Laura-Danielle

Looks great

Laura-Danielle · 2022-02-16T21:42:33Z

common/tests/test_tool_detection.cpp

+                          return a.bounds.upper < b.bounds.upper;
+                      });
+            auto trailer =
+                std::ranges::subrange(copied.begin(), --copied.end());


home, home on the std::range

sfoster1 added 4 commits February 15, 2022 14:59

implement adc stuff for pipette

323ff6d

feat(pipettes): Detect current mount at boot

14bd9e8

Based on the voltage present on the tool sense pin and using the ADC and mount detection utilities previously added to common/, we can now pretty easily decide which mount we're on and assign ourselves a canbus id based on that.

sfoster1 requested review from sotaoverride and a team February 15, 2022 22:17

Laura-Danielle reviewed Feb 15, 2022

View reviewed changes

sfoster1 added 3 commits February 16, 2022 10:08

fixup: z is left and a is right

4f0eb36

furthermore, a is right and z is left

feat(bootloader): add pipette mount detection

f1f5777

This unfortunately reimplements the mount detection algorithm in C but it will work just fine (and is checked by unit tests). Bootloaders should now properly decide which mount they're on.

fix(bootloader): allow flashing pipettes

72833f3

This flash command had the wrong openocd config value

sfoster1 requested a review from amitlissack February 16, 2022 17:58

amitlissack reviewed Feb 16, 2022

View reviewed changes

sfoster1 added 3 commits February 16, 2022 15:26

????????

1861969

to-DONE

d3dd1e4

formatting

099fb49

amitlissack approved these changes Feb 16, 2022

View reviewed changes

Laura-Danielle approved these changes Feb 16, 2022

View reviewed changes

sfoster1 merged commit 27e76e9 into main Feb 16, 2022

sfoster1 deleted the add_pipette_mount_detect branch February 16, 2022 22:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pipette): Detect mount at boot and assign node id #246

feat(pipette): Detect mount at boot and assign node id #246

sfoster1 commented Feb 15, 2022 •

edited

Loading

Laura-Danielle Feb 15, 2022

sotaoverride Feb 16, 2022

sfoster1 Feb 16, 2022

amitlissack Feb 16, 2022

amitlissack Feb 16, 2022

amitlissack Feb 16, 2022

sfoster1 Feb 16, 2022

amitlissack Feb 16, 2022

amitlissack Feb 16, 2022

sfoster1 Feb 16, 2022

amitlissack left a comment

Laura-Danielle left a comment

Laura-Danielle Feb 16, 2022

sfoster1 Feb 16, 2022


		using namespace tool_detection;

		constexpr static auto pipette_384_chan_z_bounds =

feat(pipette): Detect mount at boot and assign node id #246

feat(pipette): Detect mount at boot and assign node id #246

Conversation

sfoster1 commented Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitlissack left a comment

Choose a reason for hiding this comment

Laura-Danielle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfoster1 commented Feb 15, 2022 •

edited

Loading