Add IC engine control module #24055

sfuhrer · 2024-11-29T10:22:43Z

The RPM based functionality requires #24041

Solved Problem

Adding fuel engine support to PX4, including:

starting logic
RPM based re-start logic
expose actuators as functions

Solution

Adding now module to handle ICE acutators. It gets fed by user inputs (for now manual control) and actuator_motors (TODO: add identifier which actuator_motor instances are fuel engines), and outputs the 4 fuel engine actuators: ignition, throttle, choke, starter motor.

A flow chart of the solution:

The proposed state machine:

Changelog Entry

For release notes:

Feature: Add IC engine control module

Alternatives / Follow-ups

Couple of ideas for followup work:

Idle RPM governor
allow start/stop over CMD_DO_ENGINE_CONTROL
- publish engine status over MAVLink EFI_STATUS
make "flight time remaining" compatible with fuel engines (needs extraction from battery_status)
Fuel status MAVLink message support

Test coverage

Tested on bench.

Context

Note: This feature is not enabled by default and needs to be configured together with the rpm capture driver: CONFIG_MODULES_INTERNAL_COMBUSTION_ENGINE_CONTROL=y
CONFIG_DRIVERS_RPM_CAPTURE=y

Below is a plot from the test bench:

During the starting and running state there was a fault and ICE control was able to recover.

github-actions · 2024-11-29T10:28:14Z

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 680 byte (0.03 %)]

    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +616  +0.0%    +616    .text
  +3.9%    +268  +3.9%    +268    ../../src/lib/mixer_module/mixer_module.cpp
  +0.1%    +175  +0.1%    +175    [section .text]
  +0.1%    +149  +0.1%    +149    ROMFS/nsh_romfsimg.c
  +0.5%     +24  +0.5%     +24    ../../src/modules/logger/logged_topics.cpp
  [NEW]     +16  [NEW]     +16    msg/topics_sources/internal_combustion_engine_control.cpp
  +0.2%      +4  +0.2%      +4    ../../platforms/common/uORB/uORBDeviceMaster.cpp
  -2.6%      -4  -2.6%      -4    ../../platforms/common/uORB/Subscription.cpp
  -0.2%      -4  -0.2%      -4    ../../src/lib/geo/geo.cpp
  -3.4%      -4  -3.4%      -4    stdlib/lib_srand.c
  -0.6%      -8  -0.6%      -8    ../../platforms/common/uORB/uORBManager.cpp
[ = ]       0  +0.1%     +64    .bss
  [ = ]       0  +6.8%     +52    [section .bss]
  [ = ]       0  +8.8%     +12    ../../src/lib/mixer_module/mixer_module.cpp
  [ = ]       0  +3.1%      +4    chip/stm32_exti_gpio.c
  [ = ]       0  -0.1%      -4    ../../platforms/nuttx/src/px4/common/board_dma_alloc.c
+0.0%    +972  [ = ]       0    .debug_abbrev
  +0.5%     +16  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
  [NEW]    +900  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
+0.0%     +64  [ = ]       0    .debug_aranges
  +4.8%     +40  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
  [NEW]     +32  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
+0.0%    +176  [ = ]       0    .debug_frame
+0.1% +15.5Ki  [ = ]       0    .debug_info
  +0.1%      +7  [ = ]       0    ../../platforms/common/uORB/Subscription.cpp
  +0.1%      +7  [ = ]       0    ../../platforms/common/uORB/SubscriptionInterval.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORB.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORBDeviceMaster.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORBDeviceNode.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORBManager.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/nuttx/src/px4/common/gpio/mcp23009/mcp23009.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/adc/ads1115/ads1115_main.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/adc/board_adc/ADC.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/barometer/bmp388/bmp388.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/barometer/ms5611/ms5611.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/camera_capture/camera_capture.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/camera_trigger/camera_trigger.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/differential_pressure/ms4525do/MS4525DO.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/differential_pressure/ms5525dso/MS5525DSO.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/differential_pressure/sdp3x/SDP3X.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/distance_sensor/cm8jl65/CM8JL65.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_i2c/lightware_laser_i2c.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_serial/lightware_laser_serial.cpp
 -99.9% +15.4Ki  [ = ]       0    [482 Others]
+0.0% +2.23Ki  [ = ]       0    .debug_line
  -0.5%      -8  [ = ]       0    ../../platforms/common/uORB/Subscription.cpp
  -0.4%     -16  [ = ]       0    ../../platforms/common/uORB/uORBManager.cpp
  +0.3%     +38  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +0.6%     +38  [ = ]       0    ../../src/drivers/pwm_out/PWMOut.cpp
  +0.3%     +40  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.4%     +40  [ = ]       0    ../../src/drivers/uavcan/actuators/esc.cpp
  +0.6%     +40  [ = ]       0    ../../src/drivers/uavcan/actuators/servo.cpp
  +0.0%     +40  [ = ]       0    ../../src/drivers/uavcan/uavcan_main.cpp
  +0.1%     +40  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  +3.5%    +597  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.4%     +24  [ = ]       0    ../../src/modules/logger/logged_topics.cpp
  +0.7%     +38  [ = ]       0    ../../src/modules/simulation/pwm_out_sim/PWMSim.cpp
  -0.0%      -6  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  [NEW] +1.33Ki  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
  +0.7%     +40  [ = ]       0    msg/topics_sources/uORBTopics.cpp
  +0.4%      +4  [ = ]       0    task/task_cancelpt.c
+0.0%    +939  [ = ]       0    .debug_loc
  -0.1%     -15  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.2%     +15  [ = ]       0    ../../src/drivers/rc/crsf_rc/CrsfRc.cpp
  -0.4%     -15  [ = ]       0    ../../src/drivers/telemetry/frsky_telemetry/frsky_data.cpp
  -0.0%     -15  [ = ]       0    ../../src/drivers/uavcan/uavcan_main.cpp
  -0.0%      -2  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  -0.1%     -15  [ = ]       0    ../../src/lib/avoidance/ObstacleAvoidance.cpp
  +4.0%    +711  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +0.0%      +1  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/externalChecks.cpp
  +1.2%     +15  [ = ]       0    ../../src/modules/commander/airspeed_calibration.cpp
  +0.1%     +13  [ = ]       0    ../../src/modules/commander/failure_detector/FailureDetector.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Auto/FlightTaskAuto.cpp
  +0.0%      +3  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/AutoFollowTarget/FlightTaskAutoFollowTarget.cpp
  +0.2%     +15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Transition/FlightTaskTransition.cpp
  +0.2%     +15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Utility/Sticks.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/fw_att_control/FixedwingAttitudeControl.cpp
  -0.0%     -16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/fw_rate_control/FixedwingRateControl.cpp
  -0.3%     -15  [ = ]       0    ../../src/modules/land_detector/FixedwingLandDetector.cpp
  +0.9%     +91  [ = ]       0    ../../src/modules/logger/logged_topics.cpp
 -100.0%    +123  [ = ]       0    [9 Others]
+0.0%    +217  [ = ]       0    .debug_ranges
  +3.3%    +208  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  [NEW]     +16  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
  +1.6%      +1  [ = ]       0    task/task_cancelpt.c
+0.0%    +784  [ = ]       0    .debug_str
  +0.1%     +58  [ = ]       0    
  +0.2%     +35  [ = ]       0    ../../src/drivers/adc/ads1115/ads1115_main.cpp
  +0.4%     +78  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +6.5%    +568  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  +0.5%      +8  [ = ]       0    ../../src/modules/control_allocator/VehicleActuatorEffectiveness/ActuatorEffectivenessMCTilt.cpp
  [NEW]     +37  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
-0.5%      -1  [ = ]       0    .shstrtab
+0.0%    +337  [ = ]       0    .strtab
  +5.3%    +232  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +0.1%     +96  [ = ]       0    [section .strtab]
  [NEW]     +41  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
+0.1%    +320  [ = ]       0    .symtab
  +1.3%     +16  [ = ]       0    ../../src/drivers/rc/dsm_rc/DsmRc.cpp
  +5.5%    +224  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.1%     +80  [ = ]       0    [section .symtab]
  [NEW]     +48  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
-5.5%    -616  [ = ]       0    [Unmapped]
+0.0% +21.5Ki  +0.0%    +680    TOTAL

px4_fmu-v6x [Total VM Diff: 632 byte (0.03 %)]

    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +568  +0.0%    +568    .text
  +3.9%    +268  +3.9%    +268    ../../src/lib/mixer_module/mixer_module.cpp
  +0.1%    +157  +0.1%    +157    [section .text]
  +0.1%    +118  +0.1%    +118    ROMFS/nsh_romfsimg.c
  +0.5%     +24  +0.5%     +24    ../../src/modules/logger/logged_topics.cpp
  [NEW]     +16  [NEW]     +16    msg/topics_sources/internal_combustion_engine_control.cpp
  +0.2%      +4  +0.2%      +4    ../../platforms/common/uORB/uORBDeviceMaster.cpp
  -0.2%      -3  -0.2%      -3    ../../src/systemcmds/ver/ver.cpp
  -2.6%      -4  -2.6%      -4    ../../platforms/common/uORB/Subscription.cpp
  -0.1%      -4  -0.1%      -4    ../../src/drivers/uavcan/uavcan_drivers/stm32h7/driver/src/uc_stm32h7_can.cpp
  -0.6%      -8  -0.6%      -8    ../../platforms/common/uORB/uORBManager.cpp
[ = ]       0  +0.1%     +64    .bss
  [ = ]       0  +6.9%     +60    [section .bss]
  [ = ]       0  +9.1%     +12    ../../src/lib/mixer_module/mixer_module.cpp
  [ = ]       0  -0.1%      -4    ../../platforms/nuttx/src/px4/common/board_dma_alloc.c
  [ = ]       0  -3.1%      -4    ../../src/drivers/uavcan/uavcan_drivers/stm32h7/driver/src/uc_stm32h7_clock.cpp
+0.0%    +972  [ = ]       0    .debug_abbrev
  +0.5%     +16  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
  [NEW]    +900  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
+0.0%     +64  [ = ]       0    .debug_aranges
  +4.8%     +40  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
  [NEW]     +32  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
+0.0%    +176  [ = ]       0    .debug_frame
+0.1% +15.4Ki  [ = ]       0    .debug_info
  +0.1%      +7  [ = ]       0    ../../platforms/common/uORB/Subscription.cpp
  +0.1%      +7  [ = ]       0    ../../platforms/common/uORB/SubscriptionInterval.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORB.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORBDeviceMaster.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORBDeviceNode.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/common/uORB/uORBManager.cpp
  +0.0%      +7  [ = ]       0    ../../platforms/nuttx/src/px4/common/gpio/mcp23009/mcp23009.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/adc/ads1115/ads1115_main.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/adc/board_adc/ADC.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/barometer/bmp388/bmp388.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/barometer/invensense/icp201xx/ICP201XX.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/barometer/ms5611/ms5611.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/camera_capture/camera_capture.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/camera_trigger/camera_trigger.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/differential_pressure/ms4525do/MS4525DO.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/differential_pressure/ms5525dso/MS5525DSO.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/differential_pressure/sdp3x/SDP3X.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/distance_sensor/cm8jl65/CM8JL65.cpp
  +0.0%      +7  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_i2c/lightware_laser_i2c.cpp
 -99.9% +15.2Ki  [ = ]       0    [466 Others]
+0.0% +2.19Ki  [ = ]       0    .debug_line
  -0.5%      -8  [ = ]       0    ../../platforms/common/uORB/Subscription.cpp
  -0.4%     -16  [ = ]       0    ../../platforms/common/uORB/uORBManager.cpp
  +0.3%     +38  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +0.6%     +38  [ = ]       0    ../../src/drivers/pwm_out/PWMOut.cpp
  +0.3%     +40  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.4%     +40  [ = ]       0    ../../src/drivers/uavcan/actuators/esc.cpp
  +0.6%     +40  [ = ]       0    ../../src/drivers/uavcan/actuators/servo.cpp
  +0.0%     +40  [ = ]       0    ../../src/drivers/uavcan/uavcan_main.cpp
  +0.1%     +40  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  +3.5%    +597  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.4%     +24  [ = ]       0    ../../src/modules/logger/logged_topics.cpp
  -0.0%      -6  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  [NEW] +1.33Ki  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
  +0.7%     +40  [ = ]       0    msg/topics_sources/uORBTopics.cpp
  +0.1%      +1  [ = ]       0    task/task_cancelpt.c
+0.0%    +773  [ = ]       0    .debug_loc
  +0.4%     +15  [ = ]       0    ../../src/drivers/adc/board_adc/ADC.cpp
  +0.2%     +15  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  -0.1%     -15  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  -0.0%     -15  [ = ]       0    ../../src/drivers/uavcan/uavcan_main.cpp
  +0.0%     +10  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  +0.1%     +15  [ = ]       0    ../../src/lib/avoidance/ObstacleAvoidance.cpp
  +0.1%     +15  [ = ]       0    ../../src/lib/collision_prevention/CollisionPrevention.cpp
  +4.1%    +727  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -0.2%     -15  [ = ]       0    ../../src/lib/rtl/rtl_time_estimator.cpp
  -0.0%     -15  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/batteryCheck.cpp
  -0.3%     -15  [ = ]       0    ../../src/modules/commander/HomePosition.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/commander/accelerometer_calibration.cpp
  -1.0%     -15  [ = ]       0    ../../src/modules/events/status_display.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/FlightModeManager.cpp
  -0.0%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Auto/FlightTaskAuto.cpp
  -0.0%     -16  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/AutoFollowTarget/FlightTaskAutoFollowTarget.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Transition/FlightTaskTransition.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Utility/Sticks.cpp
  -0.0%     -16  [ = ]       0    ../../src/modules/fw_autotune_attitude_control/fw_autotune_attitude_control.cpp
 -100.0%    +158  [ = ]       0    [13 Others]
+0.0%    +215  [ = ]       0    .debug_ranges
  +3.3%    +208  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  [NEW]     +16  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
  -1.5%      -1  [ = ]       0    task/task_cancelpt.c
+0.0%    +784  [ = ]       0    .debug_str
  +0.1%     +58  [ = ]       0    
  +0.2%     +35  [ = ]       0    ../../src/drivers/adc/ads1115/ads1115_main.cpp
  +0.4%     +78  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +6.5%    +576  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  [NEW]     +37  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
-0.5%      -1  [ = ]       0    .shstrtab
+0.1%    +337  [ = ]       0    .strtab
  +5.3%    +232  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +0.1%     +96  [ = ]       0    [section .strtab]
  [NEW]     +41  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
+0.1%    +320  [ = ]       0    .symtab
  +1.7%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_ncp5623c/rgbled_ncp5623c.cpp
  +5.4%    +224  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.1%     +80  [ = ]       0    [section .symtab]
  [NEW]     +48  [ = ]       0    msg/topics_sources/internal_combustion_engine_control.cpp
-0.8%    -568  [ = ]       0    [Unmapped]
+0.0% +21.1Ki  +0.0%    +632    TOTAL

Updated: 2025-02-13T12:26:46

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

src/modules/internal_combustion_engine_control/module.yaml

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

DronecodeBot · 2024-12-09T16:13:35Z

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/gas-engine/42729/9

src/lib/mixer_module/functions/FunctionICEngineControl.hpp

src/modules/internal_combustion_engine_control/CMakeLists.txt

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.hpp

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.hpp

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

Perrrewi

I made some comments, but in general it looks good to me

sfuhrer

Beside the small comments I would have one "feature" request: To add a 1s delay between the starting retries. That should help to reduce wear on the starter motor and makes the starter retries audibly distinguishable from each other.

ROMFS/px4fmu_common/init.d/rcS

boards/px4/sitl/default.px4board

msg/InternalCombustionEngineControl.msg

src/lib/mixer_module/functions/FunctionICEngineControl.hpp

src/modules/internal_combustion_engine_control/module.yaml

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

Signed-off-by: Silvan Fuhrer <[email protected]>

…starting instead of fault

sfuhrer · 2025-02-12T15:26:21Z

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

+
+	const hrt_abstime now = hrt_absolute_time();
+
+	UserOnOffRequest user_request = UserOnOffRequest::None;


Suggested change

UserOnOffRequest user_request = UserOnOffRequest::None;

UserOnOffRequest user_request = UserOnOffRequest::Off;

We could remove the "None" enum value then.

sfuhrer · 2025-02-12T15:30:43Z

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

+				// without RPM feedback we assume the engine is running after the
+				// starting procedure but only switch state if fault detection is enabled
+				_state = State::Starting;
+				PX4_WARN("ICE: Running Fault detected");


This should probably be an event as well.

sfuhrer · 2025-02-13T09:25:19Z

src/modules/internal_combustion_engine_control/module.yaml

+
+      ICE_ON_SOURCE:
+        description:
+          short: Source of input to activate engine


Suggested change

short: Source of input to activate engine

short: Engine start/stop input source

sfuhrer · 2025-02-13T09:27:41Z

src/modules/internal_combustion_engine_control/module.yaml

+        type: enum
+        default: 0
+        values:
+          0: No activation


Suggested change

0: No activation

I don't think we need that option actually. And lets keep arming-disarming as default.

hamishwillee · 2025-02-13T21:30:24Z

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

+)DESCR_STR");
+
+	PRINT_MODULE_USAGE_NAME("internal_combustion_engine_control", "system");
+	PRINT_MODULE_USAGE_COMMAND_DESCR("start", "Start the background task");


What background task?

hamishwillee · 2025-02-13T21:41:48Z

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp

+	PRINT_MODULE_DESCRIPTION(
+		R"DESCR_STR(
+### Description
+ICE controls.
+)DESCR_STR");


This needs to be any docs you want to appear in the module docs you created in PX4/PX4-user_guide#3574 - you will still need to add the images in the issue, but not the docs, which get generated from here.

You'll need to test the generation, but to do your current docs this will look something like

(see, follow on suggestion in next comment)

Suggested change

PRINT_MODULE_DESCRIPTION(

R"DESCR_STR(

### Description

ICE controls.

)DESCR_STR");

PRINT_MODULE_DESCRIPTION(

R"DESCR_STR(

### Description

::: note

This feature is not enabled by default and needs to be configured with the rpm capture driver.

:::

The module controls the internal combustion engine ICE as described in the following architecture diagram:

![Architecture](../../assets/diagrams/ice_control_diagram.png)

The module publishes [InternalCombustionEngineControl.msg](../msg_docs/InternalCombustionEngineControl.md) containing:

| Message | Values | Explanation |

| ------------------------ | ---------- | -------------------------------------------------------- |

| `ignition_on` | True/False | Activate/deactivate |

| `throttle_control` | [0, 1] | Motor should idle with 0. Includes slew rate if enabled. |

| `choke_control` | [0, 1] | 1 closes the air inlet |

| `starter_engine_control` | [0, 1] | 1 if engine start should be delayed |

| `user_request` | On/Off | User intent |

### Implementation

The ICE is implemented with a (4) state machine:

![Architecture](../../assets/diagrams/ice_control_state_machine.png)

The state machine

- checks if [Rpm.msg](../msg_docs/Rpm.md) is updated to know if the engine is running

- allows for user inputs from

- AUX{N}

- Arming state in [VehicleStatus.msg(../msg_docs/VehicleStatus.md)

- MAVLink (TBD)

<a id="internal_combustion_engine_control_usage"></a>

)DESCR_STR");

What does it mean

This feature is not enabled by default and needs to be configured with the rpm capture driver.

How?

As part of this I would state that the module is started with ICE_EN (link to param and configured using the other ICE_ params (link to the group).

You might not need the table after:

The module publishes InternalCombustionEngineControl.msg.

Because now you are linking it. BUT, you should make sure that the information in this table is captured in the uorb topic (I added a comment)

What is the "user request" option?

But if you do that then you need to say a little more about what it does, so add info like that in:

The module controls the internal combustion engine ICE as described in the following architecture diagram:

So it might be more like

Suggested change

PRINT_MODULE_DESCRIPTION(

R"DESCR_STR(

### Description

ICE controls.

)DESCR_STR");

PRINT_MODULE_DESCRIPTION(

R"DESCR_STR(

### Description

The module controls internal combustion engine (ICE) features including: ignition (on/off), throttle and choke level, starter engine delay, and user request.

The module publishes [InternalCombustionEngineControl.msg](../msg_docs/InternalCombustionEngineControl.md).

The architecture is as shown below.:

![Architecture](../../assets/diagrams/ice_control_diagram.png)

### Enabling

This feature is not enabled by default and needs to be configured with the rpm capture driver.

[YOUR INSTRUCTIONS here, covering ICE_EN, links to other params, any kconfig etc etc]

### Implementation

The ICE is implemented with a (4) state machine:

![Architecture](../../assets/diagrams/ice_control_state_machine.png)

The state machine:

- checks if [Rpm.msg](../msg_docs/Rpm.md) is updated to know if the engine is running

- allows for user inputs from

- AUX{N}

- Arming state in [VehicleStatus.msg(../msg_docs/VehicleStatus.md)

- MAVLink (TBD)

<a id="internal_combustion_engine_control_usage"></a>

)DESCR_STR");

How much you do here vs in docs depends on what else you need /want to say about ICE. I think this is about right for module docs. However it might be nice to provide more context for ICE in the docs Hardware section

Note sure where, but places to consider:

https://docs.px4.io/main/en/power_systems/

https://docs.px4.io/main/en/actuators/ (i.e motors)

hamishwillee · 2025-02-13T21:46:13Z

msg/InternalCombustionEngineControl.msg

+uint64 timestamp        		# time since system start (microseconds)
+
+bool ignition_on          		# activate/deactivate ignition
+float32 throttle_control		# [0,1] - Motor should idle with 0. Includes slew rate if enabled.
+float32 choke_control			# [0,1]
+float32 starter_engine_control		# [0,1]
+
+uint8 user_request			# user intent for on/off


sfuhrer requested a review from RomanBapst November 29, 2024 10:22

bresch reviewed Nov 29, 2024

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp Outdated Show resolved Hide resolved

bresch reviewed Nov 29, 2024

View reviewed changes

src/modules/internal_combustion_engine_control/module.yaml Outdated Show resolved Hide resolved

RomanBapst reviewed Nov 29, 2024

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp Outdated Show resolved Hide resolved

RomanBapst requested review from KonradRudin and removed request for KonradRudin November 29, 2024 15:21

github-actions bot added the stale label Jan 9, 2025

Perrrewi reviewed Feb 4, 2025

View reviewed changes

src/lib/mixer_module/functions/FunctionICEngineControl.hpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 4, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/CMakeLists.txt Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 4, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 4, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.hpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 5, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 5, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.hpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 5, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 5, 2025

View reviewed changes

src/modules/internal_combustion_engine_control/InternalCombustionEngineControl.cpp Outdated Show resolved Hide resolved

Perrrewi reviewed Feb 5, 2025

View reviewed changes

Perrrewi force-pushed the pr-ice-control-module-main branch 2 times, most recently from 8eb6dfb to 00375b9 Compare February 5, 2025 12:21

Perrrewi mentioned this pull request Feb 7, 2025

Adding ice control description PX4/PX4-user_guide#3574

Open

1 task

Perrrewi force-pushed the pr-ice-control-module-main branch from d810d24 to c5f8416 Compare February 7, 2025 09:23

sfuhrer changed the title ~~[WIP] Add IC engine control module~~ Add IC engine control module Feb 7, 2025

sfuhrer commented Feb 7, 2025

View reviewed changes

sfuhrer and others added 6 commits February 12, 2025 08:03

Adding ICE control module

2997710

Signed-off-by: Silvan Fuhrer <[email protected]>

ICE: add throttle control

645059f

Signed-off-by: Silvan Fuhrer <[email protected]>

ICE: add slew rate to throttle

d933be0

Signed-off-by: Silvan Fuhrer <[email protected]>

ICE: WARN-->INFO where possible

a70a11b

Signed-off-by: Silvan Fuhrer <[email protected]>

ICE: only chocke on cold start and add delay

52d8a1b

Signed-off-by: Silvan Fuhrer <[email protected]>

enumeration for switch case

f1f1952

Perrrewi added 11 commits February 12, 2025 08:03

clarify that variable belongs to the class

f62b904

small fixes from review

33a4acc

cast the param to ICE source

2a4caea

choke on fault

ba9cfa1

add module name in module yaml

b4fcc2d

remove debugging msgs and small cleaning

2cedf18

update name of topic

96f6048

only call current time once per run, and added a pause between startups

1bb1e95

This avoid the risk of sending empty msgs

652c13b

Improve readability

e299465

adding curly brackets in case switch

5a28e64

Perrrewi force-pushed the pr-ice-control-module-main branch from c5f8416 to 5a28e64 Compare February 12, 2025 12:56

Remove Aux3 and aux4

8dd666f

Perrrewi force-pushed the pr-ice-control-module-main branch from c43a32b to f39fbb1 Compare February 12, 2025 16:04

Only add rest in the starting state, and let running state switch to …

7ea59c3

…starting instead of fault

Perrrewi force-pushed the pr-ice-control-module-main branch from f39fbb1 to 7ea59c3 Compare February 13, 2025 09:17

Perrrewi added 2 commits February 13, 2025 13:01

Reset retries after running fault

36bbf81

Updating values based on experiment

dd0cd98

Perrrewi force-pushed the pr-ice-control-module-main branch from 7217f52 to dd0cd98 Compare February 13, 2025 12:21

sfuhrer commented Feb 13, 2025

View reviewed changes

hamishwillee reviewed Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IC engine control module #24055

Add IC engine control module #24055

sfuhrer commented Nov 29, 2024 •

edited by Perrrewi

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading

DronecodeBot commented Dec 9, 2024

Perrrewi left a comment

sfuhrer left a comment

sfuhrer Feb 12, 2025

sfuhrer Feb 12, 2025

sfuhrer Feb 13, 2025

sfuhrer Feb 13, 2025

hamishwillee Feb 13, 2025

hamishwillee Feb 13, 2025 •

edited

Loading

hamishwillee Feb 13, 2025

hamishwillee Feb 13, 2025

hamishwillee Feb 13, 2025


		const hrt_abstime now = hrt_absolute_time();

		UserOnOffRequest user_request = UserOnOffRequest::None;

	short: Source of input to activate engine
	short: Engine start/stop input source

-	PRINT_MODULE_DESCRIPTION(
-		R"DESCR_STR(
-### Description
-ICE controls.
-)DESCR_STR");
+	PRINT_MODULE_DESCRIPTION(
+		R"DESCR_STR(
+### Description
+::: note
+This feature is not enabled by default and needs to be configured with the rpm capture driver.
+:::
+The module controls the internal combustion engine ICE as described in the following architecture diagram:
+![Architecture](../../assets/diagrams/ice_control_diagram.png)
+The module publishes [InternalCombustionEngineControl.msg](../msg_docs/InternalCombustionEngineControl.md) containing:
+| Message                  | Values     | Explanation                                              |
+| ------------------------ | ---------- | -------------------------------------------------------- |
+| `ignition_on`            | True/False | Activate/deactivate                                      |
+| `throttle_control`       | [0, 1]     | Motor should idle with 0. Includes slew rate if enabled. |
+| `choke_control`          | [0, 1]     | 1 closes the air inlet                                   |
+| `starter_engine_control` | [0, 1]     | 1 if engine start should be delayed                      |
+| `user_request`           | On/Off     | User intent                                              |
+### Implementation
+The ICE is implemented with a (4) state machine:
+![Architecture](../../assets/diagrams/ice_control_state_machine.png)
+The state machine
+- checks if [Rpm.msg](../msg_docs/Rpm.md) is updated to know if the engine is running
+- allows for user inputs from
+  - AUX{N}
+  - Arming state in [VehicleStatus.msg(../msg_docs/VehicleStatus.md)
+  - MAVLink (TBD)
+<a id="internal_combustion_engine_control_usage"></a>
+)DESCR_STR");

Add IC engine control module #24055

Are you sure you want to change the base?

Add IC engine control module #24055

Conversation

sfuhrer commented Nov 29, 2024 • edited by Perrrewi Loading

Solved Problem

Solution

Changelog Entry

Alternatives / Follow-ups

Test coverage

Context

github-actions bot commented Nov 29, 2024 • edited Loading

🔎 FLASH Analysis

DronecodeBot commented Dec 9, 2024

Perrrewi left a comment

Choose a reason for hiding this comment

sfuhrer left a 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

hamishwillee Feb 13, 2025 • 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

sfuhrer commented Nov 29, 2024 •

edited by Perrrewi

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading

hamishwillee Feb 13, 2025 •

edited

Loading