diff --git a/.github/workflows/build_sensor_fw.yaml b/.github/workflows/build_sensor_fw.yaml new file mode 100644 index 000000000..d6e4e6d52 --- /dev/null +++ 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 index bc6798960..355e067ac 100644 --- 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 index f550a95a5..19900ef61 100644 --- 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 index de644b8f5..6dbf1c62d 100644 --- 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 index bbd118211..10e6078b9 100644 --- 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 index 2ffd1c398..f04ce6a97 100644 --- 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 index 7b5a04c63..634249d8c 100644 --- 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 index bca7e0f5c..ebf45ad6e 100644 --- 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(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(0x3) // sync and report - }; - send_to_capacitive_sensor_queue(msg); + auto binding = static_cast(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(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(can::ids::SensorOutputBinding::sync)}; - send_to_capacitive_sensor_queue(stop_msg); + auto binding = + static_cast(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(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::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::send_to_pressure_sensor_queue_front(sensor_client, m); + } + void send_to_capacitive_sensor_queue_rear( can::messages::BindSensorOutputRequest& m) { - SensorClientHelper::send_to_pressure_sensor_queue(sensor_client, m); + SensorClientHelper::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::send_to_capacitive_sensor_queue(sensor_client, m); + // send to both queues, they will handle their own gating based on sensor id + SensorClientHelper::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 index 225ff7b90..7cee57b34 100644 --- 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 index 38b406201..c92c4d8c6 100644 --- 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 index b031c5693..776952889 100644 --- 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 p_buff; +#ifdef USE_TWO_BUFFERS +static std::array 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, diff --git a/pipettes/firmware/motor_configurations.cpp b/pipettes/firmware/motor_configurations.cpp index 2a43d0f1f..70c96da5e 100644 --- a/pipettes/firmware/motor_configurations.cpp +++ b/pipettes/firmware/motor_configurations.cpp @@ -207,6 +207,11 @@ auto motor_configs::hardware_config_by_axis(TMC2160PipetteAxis which) .port = GPIOB, .pin = GPIO_PIN_9, .active_setting = GPIO_PIN_RESET}, + .diag0 = + {// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + .port = GPIOB, + .pin = GPIO_PIN_6, + .active_setting = GPIO_PIN_RESET}, }; case TMC2160PipetteAxis::left_gear_motor: return motor_hardware::HardwareConfig{ @@ -237,6 +242,11 @@ auto motor_configs::hardware_config_by_axis(TMC2160PipetteAxis which) .port = GPIOB, .pin = GPIO_PIN_9, .active_setting = GPIO_PIN_RESET}, + .diag0 = + {// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + .port = GPIOB, + .pin = GPIO_PIN_6, + .active_setting = GPIO_PIN_RESET}, }; case TMC2160PipetteAxis::linear_motor: default: diff --git a/push b/push index 5a35197ca..c0d319d1f 100755 --- a/push +++ b/push @@ -59,8 +59,11 @@ def _build_fw(zip_path, apps_path): for fname in os.listdir(apps_path): zf.write(os.path.join(apps_path, fname), fname) -def _transfer_firmware(host, repo_path, scp, ssh): - apps_path = os.path.join(repo_path, 'dist', 'applications') +def _transfer_firmware(host, repo_path, scp, ssh, sensors): + dist_dir = "dist" + if sensors: + dist_dir = dist_dir+"-sensor" + apps_path = os.path.join(repo_path, dist_dir, 'applications') with _controlled_tempdir() as td: local_zip_path = os.path.join(td, 'fw.zip') robot_zip_path = '/tmp/fw.zip' @@ -69,9 +72,14 @@ def _transfer_firmware(host, repo_path, scp, ssh): _ssh(ssh, host, 'unzip -o {zip_path} -d /usr/lib/firmware/'.format(zip_path=robot_zip_path)) _ssh(ssh, host, 'rm {zip_path}'.format(zip_path=robot_zip_path)) -def _prep_firmware(repo_path, cmake): - _cmd([cmake, '--build', '--preset=firmware-g4', '--target', 'firmware-applications'], cwd=repo_path) - _cmd([cmake, '--install', './build-cross', '--component', 'Applications'], cwd=repo_path) +def _prep_firmware(repo_path, cmake, sensors): + preset = "firmware-g4" + working_dir = "./build-cross" + if sensors: + preset = preset+"-sensors" + working_dir = working_dir+"-sensor" + _cmd([cmake, '--build', f'--preset={preset}', '--target', 'firmware-applications', 'firmware-images'], cwd=repo_path) + _cmd([cmake, '--install', f'{working_dir}', '--component', 'Applications'], cwd=repo_path) @contextmanager def _prep_robot(host, ssh): @@ -96,19 +104,19 @@ def _find_utils(): def _restart_robot(host, ssh): _ssh(ssh, host, 'nohup systemctl restart opentrons-robot-server &') -def _do_push(host, repo_path, build, restart): +def _do_push(host, repo_path, build, restart, sensors): ssh, scp, cmake = _find_utils() if build: - _prep_firmware(repo_path, cmake) + _prep_firmware(repo_path, cmake, sensors) with _prep_robot(host, ssh): - _transfer_firmware(host, repo_path, scp, ssh) + _transfer_firmware(host, repo_path, scp, ssh, sensors) if restart: _restart_robot(host, ssh) -def push(host, repo_path=None, build=True, restart=True): +def push(host, repo_path=None, build=True, restart=True, sensors=False): repo = repo_path or os.dirname(__file__) try: - _do_push(host, repo, build, restart) + _do_push(host, repo, build, restart, sensors) return 0 except subprocess.CalledProcessError as e: print( @@ -125,7 +133,7 @@ def _push_from_argparse(args): if args.key: _SSH_EXTRA_OPTS.append('-i') _SSH_EXTRA_OPTS.append(args.key) - return push(args.host, os.path.abspath(args.repo_path), not args.no_build, not args.no_restart) + return push(args.host, os.path.abspath(args.repo_path), not args.no_build, not args.no_restart, args.sensors) def _arg_parser(parent=None): parents = [] @@ -156,6 +164,11 @@ def _arg_parser(parent=None): type=str, help='Private SSH key to use' ) + parser.add_argument( + '--sensors', + action='store_true', + help='Private SSH key to use' + ) return parser def _main():