Merge branch 'main' into EXEC-343-lld-cap-probe-logging

Opentrons · Apr 29, 2024 · c3fbd2a · c3fbd2a
2 parents 3649494 + e0ea2e1
commit c3fbd2a
Show file tree

Hide file tree

Showing 13 changed files with 186 additions and 62 deletions.
diff --git a/.github/workflows/build_sensor_fw.yaml b/.github/workflows/build_sensor_fw.yaml
@@ -0,0 +1,44 @@
+name: "Build firmware sensor variant bundles"
+on:
+  push:
+    branches:
+      - '*'
+    tags:
+      - 'v*'
+  workflow_dispatch:
+
+env:
+  ci: 1
+
+defaults:
+  run:
+    shell: bash
+
+jobs:
+  build:
+    name: "Build"
+    runs-on: "ubuntu-latest"
+    timeout-minutes: 20
+    steps:
+      - uses: "actions/checkout@v2"
+        with:
+          fetch-depth: 0
+      - uses: "actions/cache@v3"
+        with:
+          path: "./stm32-tools"
+          key: ${{ runner.os }}-${{ hashFiles('**/cmake/*') }}-${{ secrets.CACHE_VERSION }}
+      - name: 'CMake configure'
+        run: |
+          cmake --preset=cross-sensor-buffer . -DCMAKE_BUILD_TYPE=RelWithDebInfo
+      - name: 'Build images'
+        run: |
+          cmake --build --preset=firmware-g4-sensors --target firmware-applications firmware-images
+      - name: 'Prep images for upload'
+        run: |
+          cmake --install ./build-cross-sensor
+      - name: 'Upload application artifact'
+        uses: actions/upload-artifact@v3
+        with:
+          name: 'firmware-applications-${{github.ref_name}}'
+          path: |
+            dist-sensor/applications/*
diff --git a/.github/workflows/cross-compile-special-sensors.yaml b/.github/workflows/cross-compile-special-sensors.yaml
@@ -1,4 +1,4 @@
-name: "cross-compile/format/lint all targets"
+name: "cross-compile-sensors/format/lint all targets"
 on:
   push:
     branches:
@@ -48,10 +48,10 @@ jobs:
           cache-version: ${{ secrets.CACHE_VERSION }}
 
       - name: Configure
-        run: cmake --preset=cross . -DUSE_SENSOR_MOVE=true
+        run: cmake --preset=cross-sensor-buffer .
 
       - name: Build all STM32G4 applications
-        run: cmake --build --preset=${{ matrix.target }} --target ${{ matrix.target }}-images ${{ matrix.target }}-applications
+        run: cmake --build ./build-cross-sensor --target ${{ matrix.target }}-images ${{ matrix.target }}-applications
 
   format:
     runs-on: "ubuntu-20.04"
@@ -77,10 +77,10 @@ jobs:
           cache-version: ${{ secrets.CACHE_VERSION }}
 
       - name: Configure
-        run: cmake --preset=cross . -DUSE_SENSOR_MOVE=true
+        run: cmake --preset=cross-sensor-buffer .
 
       - name: Format
-        run: cmake --build ./build-cross --target format-ci
+        run: cmake --build ./build-cross-sensor --target format-ci
 
   lint:
     runs-on: "ubuntu-20.04"
@@ -112,7 +112,7 @@ jobs:
           cache-version: ${{ secrets.CACHE_VERSION }}
 
       - name: Configure
-        run: cmake --preset=cross . -DUSE_SENSOR_MOVE=true
+        run: cmake --preset=cross-sensor-buffer .
 
       - name: Format
-        run: cmake --build ./build-cross --target ${{ matrix.target }}-lint
+        run: cmake --build ./build-cross-sensor --target ${{ matrix.target }}-lint
diff --git a/.gitignore b/.gitignore
@@ -5,6 +5,7 @@ CMakeUserPresets.json
 *.pyc
 build-cross/
 build-cross-pipettes/
+build-cross-sensor/
 stm32-tools/
 build-host/
 cmake-build-debug/
@@ -17,3 +18,4 @@ state_manager/coverage.xml
 state_manager/dist/
 state_manager/poetry.toml
 dist/
+dist-sensor/
diff --git a/CMakePresets.json b/CMakePresets.json
@@ -35,6 +35,17 @@
       "binaryDir": "${sourceDir}/build-cross",
       "inherits": "cross-no-directory-reqs"
     },
+    {
+      "name": "cross-sensor-buffer",
+      "displayName": "STM32 G4 OT-3 cross-compilation with sensor data buffers",
+      "description": "Build application firmware for OT-3 systems that use STM32, for flashing onto boards",
+      "installDir": "${sourceDir}/dist-sensor",
+      "binaryDir": "${sourceDir}/build-cross-sensor",
+      "cacheVariables": {
+        "USE_PRESSURE_MOVE": true
+      },
+      "inherits": "cross"
+    },
     {
       "name": "host",
       "displayName": "STM32 OT-3 host compilation for tests",
@@ -88,6 +99,17 @@
         "firmware-images"
       ]
     },
+    {
+      "name": "firmware-g4-sensors",
+      "displayName": "All G4 Firmwares With Sensor Data Buffers",
+      "description": "Environment to build all g4 firmware - see firmware-l5",
+      "configurePreset": "cross-sensor-buffer",
+      "jobs": 4,
+      "targets": [
+        "firmware-applications",
+        "firmware-images"
+      ]
+    },
     {
       "name": "pipettes",
       "displayName": "pipettes binaries",

diff --git a/include/bootloader/core/ids.h b/include/bootloader/core/ids.h
@@ -194,6 +194,8 @@ typedef enum {
 typedef enum {
     can_sensorid_s0 = 0x0,
     can_sensorid_s1 = 0x1,
+    can_sensorid_unused = 0x2,
+    can_sensorid_both = 0x3,
 } CANSensorId;
 
 /** Links sensor threshold triggers to pins. */
@@ -238,6 +240,7 @@ typedef enum {
     can_movestopcondition_stall = 0x10,
     can_movestopcondition_ignore_stalls = 0x20,
     can_movestopcondition_limit_switch_backoff = 0x40,
+    can_movestopcondition_sensor_report = 0x80,
 } CANMoveStopCondition;
 
 /** A bit field of the arbitration id parts. */

diff --git a/include/can/core/ids.hpp b/include/can/core/ids.hpp
@@ -209,6 +209,7 @@ enum class SensorId {
     S0 = 0x0,
     S1 = 0x1,
     UNUSED = 0x2,
+    BOTH = 0x3,
 };
 
 /** Links sensor threshold triggers to pins. */
@@ -253,6 +254,7 @@ enum class MoveStopCondition {
     stall = 0x10,
     ignore_stalls = 0x20,
     limit_switch_backoff = 0x40,
+    sensor_report = 0x80,
 };
 
 /** High-level type of pipette. */

diff --git a/include/motor-control/core/brushed_motor/brushed_motor_interrupt_handler.hpp b/include/motor-control/core/brushed_motor/brushed_motor_interrupt_handler.hpp
@@ -121,6 +121,7 @@ class BrushedMotorInterruptHandler {
                         true, AckMessageId::complete_without_condition);
                 }
                 break;
+            case MoveStopCondition::sensor_report:
             case MoveStopCondition::ignore_stalls:
             case MoveStopCondition::limit_switch_backoff:
             case MoveStopCondition::sync_line:
@@ -301,6 +302,7 @@ class BrushedMotorInterruptHandler {
             case MoveStopCondition::sync_line:
             case MoveStopCondition::ignore_stalls:
             case MoveStopCondition::limit_switch_backoff:
+            case MoveStopCondition::sensor_report:
                 // this is an unused move stop condition for the brushed motor
                 // just return with no condition
                 // TODO creat can bus error messages and send that instead

diff --git a/include/motor-control/core/stepper_motor/motor_interrupt_handler.hpp b/include/motor-control/core/stepper_motor/motor_interrupt_handler.hpp
@@ -415,7 +415,28 @@ class MotorInterruptHandler {
          */
         return tick_count < buffered_move.duration;
     }
-
+#ifdef USE_SENSOR_MOVE
+    auto send_bind_message(can::ids::SensorType sensor_type, can::ids::SensorId sensor_id, uint8_t binding) -> void {
+        auto msg = can::messages::BindSensorOutputRequest{
+            .message_index = buffered_move.message_index,
+            .sensor = sensor_type,
+            .sensor_id = sensor_id,
+            .binding = binding};
+        if (sensor_type == can::ids::SensorType::pressure) {
+            if (sensor_id == can::ids::SensorId::S0) {
+                send_to_pressure_sensor_queue_rear(msg);
+            } else {
+                send_to_pressure_sensor_queue_front(msg);
+            }
+        } else if (sensor_type == can::ids::SensorType::capacitive) {
+            if (sensor_id == can::ids::SensorId::S0) {
+                send_to_capacitive_sensor_queue_rear(msg);
+            } else {
+                send_to_capacitive_sensor_queue_front(msg);
+            }
+        }
+    }
+#endif
     void update_move() {
         _has_active_move = move_queue.try_read_isr(&buffered_move);
         if (_has_active_move) {
@@ -424,22 +445,12 @@ class MotorInterruptHandler {
                 hardware.get_encoder_pulses();
 #ifdef USE_SENSOR_MOVE
             if (buffered_move.sensor_id != can::ids::SensorId::UNUSED) {
-                if (buffered_move.sensor_type == can::ids::SensorType::pressure) {
-                    auto msg = can::messages::BindSensorOutputRequest{
-                        .message_index = buffered_move.message_index,
-                        .sensor = can::ids::SensorType::pressure,
-                        .sensor_id = buffered_move.sensor_id,
-                        .binding = static_cast<uint8_t>(0x3)  // sync and report
-                    };
-                    send_to_pressure_sensor_queue(msg);
-                } else if (buffered_move.sensor_type == can::ids::SensorType::capacitive) {
-                    auto msg = can::messages::BindSensorOutputRequest{
-                        .message_index = buffered_move.message_index,
-                        .sensor = can::ids::SensorType::capacitive,
-                        .sensor_id = buffered_move.sensor_id,
-                        .binding = static_cast<uint8_t>(0x3)  // sync and report
-                    };
-                    send_to_capacitive_sensor_queue(msg);
+                auto binding = static_cast<uint8_t>(0x3);  // sync and report
+                if (buffered_move.sensor_id == can::ids::SensorId::BOTH) {
+                    send_bind_message(buffered_move.sensor_type, can::ids::SensorId::S0, binding);
+                    send_bind_message(buffered_move.sensor_type, can::ids::SensorId::S1, binding);
+                } else {
+                    send_bind_message(buffered_move.sensor_type, buffered_move.sensor_id, binding);
                 }
             }
 #endif
@@ -532,22 +543,13 @@ class MotorInterruptHandler {
         build_and_send_ack(ack_msg_id);
 #ifdef USE_SENSOR_MOVE
         if (buffered_move.sensor_id != can::ids::SensorId::UNUSED) {
-            if (buffered_move.sensor_type == can::ids::SensorType::pressure) {
-                auto stop_msg = can::messages::BindSensorOutputRequest{
-                    .message_index = buffered_move.message_index,
-                    .sensor = can::ids::SensorType::pressure,
-                    .sensor_id = buffered_move.sensor_id,
-                    .binding =
-                        static_cast<uint8_t>(can::ids::SensorOutputBinding::sync)};
-                send_to_pressure_sensor_queue(stop_msg);
-            } else if (buffered_move.sensor_type == can::ids::SensorType::pressure) {
-                auto stop_msg = can::messages::BindSensorOutputRequest{
-                    .message_index = buffered_move.message_index,
-                    .sensor = can::ids::SensorType::capacitive,
-                    .sensor_id = buffered_move.sensor_id,
-                    .binding =
-                        static_cast<uint8_t>(can::ids::SensorOutputBinding::sync)};
-                send_to_capacitive_sensor_queue(stop_msg);
+            auto binding =
+                static_cast<uint8_t>(can::ids::SensorOutputBinding::sync);
+            if (buffered_move.sensor_id == can::ids::SensorId::BOTH) {
+                send_bind_message(buffered_move.sensor_type, can::ids::SensorId::S0, binding);
+                send_bind_message(buffered_move.sensor_type, can::ids::SensorId::S1, binding);
+            } else {
+                send_bind_message(buffered_move.sensor_type, buffered_move.sensor_id, binding);
             }
         }
 #endif
@@ -694,13 +696,23 @@ class MotorInterruptHandler {
             static_cast<uint32_t>(position_tracker >> 31));
     }
 #ifdef USE_SENSOR_MOVE
-    void send_to_pressure_sensor_queue(
+    void send_to_pressure_sensor_queue_rear(
+        can::messages::BindSensorOutputRequest& m) {
+        SensorClientHelper<SensorClient>::send_to_pressure_sensor_queue_rear(sensor_client, m);
+    }
+    void send_to_pressure_sensor_queue_front(
+        can::messages::BindSensorOutputRequest& m) {
+        // send to both queues, they will handle their own gating based on sensor id
+        SensorClientHelper<SensorClient>::send_to_pressure_sensor_queue_front(sensor_client, m);
+    }
+    void send_to_capacitive_sensor_queue_rear(
         can::messages::BindSensorOutputRequest& m) {
-        SensorClientHelper<SensorClient>::send_to_pressure_sensor_queue(sensor_client, m);
+        SensorClientHelper<SensorClient>::send_to_capacitive_sensor_queue_rear(sensor_client, m);
     }
-    void send_to_capacitive_sensor_queue(
+    void send_to_capacitive_sensor_queue_front(
         can::messages::BindSensorOutputRequest& m) {
-        SensorClientHelper<SensorClient>::send_to_capacitive_sensor_queue(sensor_client, m);
+        // send to both queues, they will handle their own gating based on sensor id
+        SensorClientHelper<SensorClient>::send_to_capacitive_sensor_queue_front(sensor_client, m);
     }
 #endif
     uint64_t tick_count = 0x0;

diff --git a/motor-control/firmware/stepper_motor/motor_hardware.cpp b/motor-control/firmware/stepper_motor/motor_hardware.cpp
@@ -18,14 +18,14 @@ void MotorHardware::activate_motor() {
     gpio::set(pins.enable);
     if (pins.ebrake.has_value()) {
         gpio::reset(pins.ebrake.value());
+        motor_hardware_delay(20);
     }
 }
 void MotorHardware::deactivate_motor() {
     if (pins.ebrake.has_value()) {
         gpio::set(pins.ebrake.value());
-        motor_hardware_delay(10);
+        motor_hardware_delay(20);
     }
-    motor_hardware_delay(10);
     gpio::reset(pins.enable);
 }
 void MotorHardware::start_timer_interrupt() {

diff --git a/pipettes/core/CMakeLists.txt b/pipettes/core/CMakeLists.txt
@@ -28,7 +28,8 @@ function(target_pipettes_core_multi TARGET REVISION)
     target_pipettes_core_common(${TARGET} ${REVISION})
     target_compile_definitions(${TARGET} PUBLIC PIPETTE_TYPE_DEFINE=EIGHT_CHANNEL)
     if(${USE_SENSOR_MOVE})
-    target_compile_definitions(${TARGET} PUBLIC P_BUFF_SIZE=2800)
+    target_compile_definitions(${TARGET} PUBLIC P_BUFF_SIZE=1800)
+    target_compile_definitions(${TARGET} PUBLIC USE_TWO_BUFFERS=true)
     endif()
     target_sources(${TARGET} PUBLIC
             ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/can_task_low_throughput.cpp)
@@ -38,7 +39,8 @@ function(target_pipettes_core_96 TARGET REVISION)
     target_pipettes_core_common(${TARGET} ${REVISION})
     target_compile_definitions(${TARGET} PUBLIC PIPETTE_TYPE_DEFINE=NINETY_SIX_CHANNEL)
     if(${USE_SENSOR_MOVE})
-    target_compile_definitions(${TARGET} PUBLIC P_BUFF_SIZE=1800)
+    target_compile_definitions(${TARGET} PUBLIC P_BUFF_SIZE=900)
+    target_compile_definitions(${TARGET} PUBLIC USE_TWO_BUFFERS=true)
     endif()
     target_sources(${TARGET} PUBLIC
             ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/can_task_high_throughput.cpp)
@@ -48,7 +50,8 @@ function(target_pipettes_core_384 TARGET REVISION)
     target_pipettes_core_common(${TARGET} ${REVISION})
     target_compile_definitions(${TARGET} PUBLIC PIPETTE_TYPE_DEFINE=THREE_EIGHTY_FOUR_CHANNEL)
     if(${USE_SENSOR_MOVE})
-    target_compile_definitions(${TARGET} PUBLIC P_BUFF_SIZE=1800)
+    target_compile_definitions(${TARGET} PUBLIC P_BUFF_SIZE=1500)
+    target_compile_definitions(${TARGET} PUBLIC USE_TWO_BUFFERS=true)
     endif()
     target_sources(${TARGET} PUBLIC
             ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/can_task_high_throughput.cpp)

diff --git a/pipettes/core/sensor_tasks.cpp b/pipettes/core/sensor_tasks.cpp
@@ -7,6 +7,9 @@
 static auto tasks = sensor_tasks::Tasks{};
 static auto queue_client = sensor_tasks::QueueClient{};
 static std::array<float, SENSOR_BUFFER_SIZE> p_buff;
+#ifdef USE_TWO_BUFFERS
+static std::array<float, SENSOR_BUFFER_SIZE> p_buff_front;
+#endif
 static auto eeprom_task_builder =
     freertos_task::TaskStarter<512, eeprom::task::EEPromTask>{};
 
@@ -20,7 +23,7 @@ static auto capacitive_sensor_task_builder_rear =
 
 static auto capacitive_sensor_task_builder_front =
     freertos_task::TaskStarter<512, sensors::tasks::CapacitiveSensorTask,
-                               can::ids::SensorId>(can::ids::SensorId::S0);
+                               can::ids::SensorId>(can::ids::SensorId::S1);
 
 static auto pressure_sensor_task_builder_rear =
     freertos_task::TaskStarter<512, sensors::tasks::PressureSensorTask,
@@ -140,7 +143,15 @@ void sensor_tasks::start_tasks(
     auto& pressure_sensor_task_front = pressure_sensor_task_builder_front.start(
         5, "pressure sensor s1", secondary_pressure_i2c_client,
         secondary_pressure_i2c_poller, queues, sensor_hardware_secondary,
-        sensor_version, p_buff);
+        sensor_version,
+#ifdef USE_TWO_BUFFERS
+        p_buff_front);
+#else
+        // we don't want to build a second buffer for the single channel, but if
+        // we dont pass in the correct sized array here, compilation fails
+        // this doesn't matter though cause single channels will never call this
+        p_buff);
+#endif
     auto& capacitive_sensor_task_rear =
         capacitive_sensor_task_builder_rear.start(
             5, "capacitive sensor s0", i2c3_task_client, i2c3_poller_client,