From 1da978686c6013f355e2a3aac9d8682f6f23e498 Mon Sep 17 00:00:00 2001 From: jankiefer Date: Wed, 26 Mar 2025 15:01:22 +0100 Subject: [PATCH 01/38] Added targetr for ATOMRC F405 NAVI MINI with blackbox flash storage --- src/main/target/ATOMRCF405MINI/CMakeLists.txt | 1 + src/main/target/ATOMRCF405MINI/target.c | 49 +++++ src/main/target/ATOMRCF405MINI/target.h | 195 ++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 src/main/target/ATOMRCF405MINI/CMakeLists.txt create mode 100644 src/main/target/ATOMRCF405MINI/target.c create mode 100644 src/main/target/ATOMRCF405MINI/target.h diff --git a/src/main/target/ATOMRCF405MINI/CMakeLists.txt b/src/main/target/ATOMRCF405MINI/CMakeLists.txt new file mode 100644 index 00000000000..8568fed4c79 --- /dev/null +++ b/src/main/target/ATOMRCF405MINI/CMakeLists.txt @@ -0,0 +1 @@ +target_stm32f405xg(ATOMRCF405MINI) diff --git a/src/main/target/ATOMRCF405MINI/target.c b/src/main/target/ATOMRCF405MINI/target.c new file mode 100644 index 00000000000..7ff7ac3c39d --- /dev/null +++ b/src/main/target/ATOMRCF405MINI/target.c @@ -0,0 +1,49 @@ +/* + * This file is part of INAV Project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. + * + * Alternatively, the contents of this file may be used under the terms + * of the GNU General Public License Version 3, as described below: + * + * This file is free software: you may copy, redistribute and/or modify + * it under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 3 of the License, or (at your + * option) any later version. + * + * This file is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include + +#include "platform.h" + +#include "drivers/bus.h" +#include "drivers/io.h" +#include "drivers/pwm_mapping.h" +#include "drivers/timer.h" +#include "drivers/pinio.h" +#include "drivers/sensor.h" + +timerHardware_t timerHardware[] = { + DEF_TIM(TIM8, CH3, PC8, TIM_USE_OUTPUT_AUTO, 0, 0), // S1 + DEF_TIM(TIM8, CH4, PC9, TIM_USE_OUTPUT_AUTO, 0, 0), // S2 + DEF_TIM(TIM1, CH1, PA8, TIM_USE_OUTPUT_AUTO, 0, 0), // S3 + DEF_TIM(TIM1, CH3N, PB1, TIM_USE_OUTPUT_AUTO, 0, 0), // S4 + DEF_TIM(TIM2, CH1, PA15, TIM_USE_OUTPUT_AUTO, 0, 0), // S5 + DEF_TIM(TIM2, CH3, PB10, TIM_USE_OUTPUT_AUTO, 0, 0), // S6 + DEF_TIM(TIM2, CH4, PB11, TIM_USE_OUTPUT_AUTO, 0, 0), // S7 + DEF_TIM(TIM3, CH3, PB0, TIM_USE_OUTPUT_AUTO, 0, 0), // S8 + + DEF_TIM(TIM4, CH2, PB7, TIM_USE_LED, 0, 0), // LED_STRIP +}; + +const int timerHardwareCount = sizeof(timerHardware) / sizeof(timerHardware[0]); diff --git a/src/main/target/ATOMRCF405MINI/target.h b/src/main/target/ATOMRCF405MINI/target.h new file mode 100644 index 00000000000..c7d00067474 --- /dev/null +++ b/src/main/target/ATOMRCF405MINI/target.h @@ -0,0 +1,195 @@ +/* + * This file is part of INAV Project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. + * + * Alternatively, the contents of this file may be used under the terms + * of the GNU General Public License Version 3, as described below: + * + * This file is free software: you may copy, redistribute and/or modify + * it under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 3 of the License, or (at your + * option) any later version. + * + * This file is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#pragma once +#define TARGET_BOARD_IDENTIFIER "ATKJ" + +#define USBD_PRODUCT_STRING "ATOMRCF405MINI" + +#define LED0 PA13 +#define LED1 PA14 + +#define BEEPER PC5 +#define BEEPER_INVERTED + +/* + * SPI defines + */ +#define USE_SPI + +#define USE_SPI_DEVICE_1 +#define SPI1_SCK_PIN PA5 +#define SPI1_MISO_PIN PA6 +#define SPI1_MOSI_PIN PA7 + +#define USE_SPI_DEVICE_2 +#define SPI2_SCK_PIN PB13 +#define SPI2_MISO_PIN PB14 +#define SPI2_MOSI_PIN PB15 + +#define USE_SPI_DEVICE_3 +#define SPI3_SCK_PIN PB3 +#define SPI3_MISO_PIN PB4 +#define SPI3_MOSI_PIN PB5 + +/* + * I2C defines + */ +#define USE_I2C +#define DEFAULT_I2C_BUS BUS_I2C1 + +#define USE_I2C_DEVICE_1 +#define I2C1_SCL PB8 +#define I2C1_SDA PB9 + +//BMI270 +#define USE_IMU_BMI270 +#define IMU_BMI270_ALIGN CW180_DEG +#define BMI270_SPI_BUS BUS_SPI1 +#define BMI270_CS_PIN PA4 + +/* + * Magnetometer + */ +#define USE_MAG +#define MAG_I2C_BUS DEFAULT_I2C_BUS +#define USE_MAG_ALL + +/* + * Barometer + */ +#define USE_BARO +#define BARO_I2C_BUS DEFAULT_I2C_BUS +#define USE_BARO_BMP280 +#define USE_BARO_MS5611 +#define USE_BARO_BMP085 +#define USE_BARO_DPS310 +#define USE_BARO_SPL06 + +/* + * Serial ports + */ +#define USE_VCP +#define USE_UART1 +#define USE_UART2 +#define USE_UART3 +#define USE_UART4 +#define USE_UART5 +#define USE_UART6 + +#define UART1_TX_PIN PA9 +#define UART1_RX_PIN PA10 + +#define UART2_TX_PIN PA2 +#define UART2_RX_PIN PA3 + +#define UART3_TX_PIN PC10 +#define UART3_RX_PIN PC11 + +#define UART4_TX_PIN PA0 +#define UART4_RX_PIN PA1 + +#define UART5_TX_PIN PC12 +#define UART5_RX_PIN PD2 + +#define UART6_TX_PIN PC6 +#define UART6_RX_PIN PC7 + +#define SERIAL_PORT_COUNT 7 + +/* + * ADC + */ +#define USE_ADC +#define ADC_INSTANCE ADC1 +#define ADC1_DMA_STREAM DMA2_Stream0 + +#define ADC_CHANNEL_1_PIN PC0 +#define ADC_CHANNEL_2_PIN PC1 +#define ADC_CHANNEL_3_PIN PC2 + +#define VBAT_ADC_CHANNEL ADC_CHN_3 +#define CURRENT_METER_ADC_CHANNEL ADC_CHN_2 +#define RSSI_ADC_CHANNEL ADC_CHN_1 + +#define CURRENT_METER_SCALE 320 + +/* + * OSD + */ +#define USE_MAX7456 +#define MAX7456_SPI_BUS BUS_SPI2 +#define MAX7456_CS_PIN PB12 + +/* + * SD Card + */ +/* +#define USE_SDCARD +#define USE_SDCARD_SPI +#define SDCARD_SPI_BUS BUS_SPI3 +#define SDCARD_CS_PIN PB6 +#define ENABLE_BLACKBOX_LOGGING_ON_SDCARD_BY_DEFAULT*/ + +/* start kijan Flash chip usage */ +#define USE_FLASHFS +#define USE_FLASH_M25P16 +#define M25P16_SPI_BUS BUS_SPI3 +#define M25P16_CS_PIN PB6 +#define ENABLE_BLACKBOX_LOGGING_ON_SPIFLASH_BY_DEFAULT +/* end kijan*/ + +/* + * LED Strip + */ +#define USE_LED_STRIP +#define WS2811_PIN PB7 + +/* + * Other configs + */ +#define DEFAULT_FEATURES (FEATURE_TX_PROF_SEL | FEATURE_BLACKBOX | FEATURE_TELEMETRY | FEATURE_OSD | FEATURE_CURRENT_METER | FEATURE_VBAT ) +#define DEFAULT_RX_TYPE RX_TYPE_SERIAL +#define SERIALRX_PROVIDER SERIALRX_SBUS +#define SERIALRX_UART SERIAL_PORT_USART2 + +#define USE_RANGEFINDER +#define USE_RANGEFINDER_MSP +#define USE_OPFLOW +#define USE_OPFLOW_MSP + +#define RANGEFINDER_I2C_BUS DEFAULT_I2C_BUS +#define TEMPERATURE_I2C_BUS DEFAULT_I2C_BUS +#define PITOT_I2C_BUS DEFAULT_I2C_BUS + +#define USE_DSHOT +#define USE_ESC_SENSOR +#define USE_SERIAL_4WAY_BLHELI_INTERFACE + +#define MAX_PWM_OUTPUT_PORTS 8 + +#define TARGET_IO_PORTA 0xffff +#define TARGET_IO_PORTB 0xffff +#define TARGET_IO_PORTC 0xffff +#define TARGET_IO_PORTD (BIT(2)) From 0388998f5981deba306416b25863654ecbb19f4d Mon Sep 17 00:00:00 2001 From: Jan K Date: Sat, 29 Nov 2025 09:46:35 +0100 Subject: [PATCH 02/38] TARGET_BOARD_IDENTIFIER updated to match other AtomRC devices --- src/main/target/ATOMRCF405MINI/target.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/target/ATOMRCF405MINI/target.h b/src/main/target/ATOMRCF405MINI/target.h index c7d00067474..d55864a4c3b 100644 --- a/src/main/target/ATOMRCF405MINI/target.h +++ b/src/main/target/ATOMRCF405MINI/target.h @@ -23,7 +23,7 @@ */ #pragma once -#define TARGET_BOARD_IDENTIFIER "ATKJ" +#define TARGET_BOARD_IDENTIFIER "AT4M" #define USBD_PRODUCT_STRING "ATOMRCF405MINI" From 1702746625ae4267bf2d9e7d045ddb4a7e7dca56 Mon Sep 17 00:00:00 2001 From: Emiliyan Martinov Date: Tue, 26 May 2026 09:08:50 +0300 Subject: [PATCH 03/38] feat(msp): add MSP2_INAV_WIND (0x2231) to expose wind estimator --- docs/development/msp/msp_messages.json | 29 ++++++++++++++++++++++++++ src/main/fc/fc_msp.c | 20 ++++++++++++++++++ src/main/msp/msp_protocol_v2_inav.h | 4 +++- 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/docs/development/msp/msp_messages.json b/docs/development/msp/msp_messages.json index e1b128f305d..1a0c8709fe1 100644 --- a/docs/development/msp/msp_messages.json +++ b/docs/development/msp/msp_messages.json @@ -11032,6 +11032,35 @@ "notes": "CH1-CH12 (index 0-11) are protected and will return `MSP_RESULT_ERROR`. Payload size must be 2-49 bytes. Constraint: `startChannel + channelCount <= 32`. Values persist until overwritten; no timeout. Applied as a post-RX overlay in `calculateRxChannelsAndUpdateFailsafe()` after MSP RC Override but before failsafe. Does not require `USE_RX_MSP` or MSP-RC-OVERRIDE flight mode. Does not affect failsafe detection. When MSP is the primary RX provider, channels covered by `MSP_SET_RAW_RC` are automatically skipped. Channels in the `mspOverrideChannels` bitmask are skipped when MSP RC Override mode is active. Recommended to send with `MSP_FLAG_DONT_REPLY` (flags=0x01) to save bandwidth on telemetry passthrough links. 16-bit mode requires even number of data bytes and values are clamped to 750-2250us.", "description": "Bandwidth-efficient auxiliary RC channel update. Sets CH13-CH32 with configurable resolution (2/4/8/16-bit) without affecting primary flight controls. Designed for extending channel count beyond native RC link capacity via MSP passthrough." }, + "MSP2_INAV_WIND": { + "code": 8753, + "mspv": 2, + "request": null, + "reply": { + "payload": [ + { + "name": "windSpeed", + "ctype": "uint16_t", + "desc": "Estimated horizontal wind speed (`getEstimatedHorizontalWindSpeed()`). 0 if unavailable.", + "units": "cm/s" + }, + { + "name": "windAngle", + "ctype": "uint16_t", + "desc": "Estimated wind direction in degrees (0–359, 0 = North). Derived from centidegree value divided by 100. 0 if unavailable.", + "units": "degrees" + }, + { + "name": "flags", + "ctype": "uint8_t", + "desc": "Validity flags. Bit 0: wind estimate valid (`isEstimatedWindSpeedValid()`). Remaining bits reserved.", + "units": "" + } + ] + }, + "notes": "Requires `USE_WIND_ESTIMATOR`; returns zeroes when wind estimation is not compiled in or not yet valid. Check bit 0 of `flags` before using speed/angle values.", + "description": "Retrieves the estimated horizontal wind speed and direction from the internal wind estimator." + }, "MSP2_BETAFLIGHT_BIND": { "code": 12288, "mspv": 2, diff --git a/src/main/fc/fc_msp.c b/src/main/fc/fc_msp.c index 3ec3f89f3f0..92eecfc88b5 100644 --- a/src/main/fc/fc_msp.c +++ b/src/main/fc/fc_msp.c @@ -133,6 +133,9 @@ #include "sensors/opflow.h" #include "sensors/temperature.h" #include "sensors/esc_sensor.h" +#ifdef USE_WIND_ESTIMATOR +#include "flight/wind_estimator.h" +#endif #include "telemetry/telemetry.h" @@ -1597,6 +1600,23 @@ static bool mspFcProcessOutCommand(uint16_t cmdMSP, sbuf_t *dst, mspPostProcessF #endif break; + case MSP2_INAV_WIND: +#ifdef USE_WIND_ESTIMATOR + { + uint16_t windAngle = 0; + uint16_t windSpeed = (uint16_t)getEstimatedHorizontalWindSpeed(&windAngle); + uint8_t windFlags = isEstimatedWindSpeedValid() ? 1 : 0; + sbufWriteU16(dst, windSpeed); + sbufWriteU16(dst, windAngle / 100); + sbufWriteU8(dst, windFlags); + } +#else + sbufWriteU16(dst, 0); + sbufWriteU16(dst, 0); + sbufWriteU8(dst, 0); +#endif + break; + case MSP2_INAV_MIXER: sbufWriteU8(dst, mixerConfig()->motorDirectionInverted); sbufWriteU8(dst, 0); diff --git a/src/main/msp/msp_protocol_v2_inav.h b/src/main/msp/msp_protocol_v2_inav.h index e50115d99ed..b38cc0e38f9 100755 --- a/src/main/msp/msp_protocol_v2_inav.h +++ b/src/main/msp/msp_protocol_v2_inav.h @@ -132,4 +132,6 @@ #define MSP2_INAV_SET_WP_INDEX 0x2221 //in message jump to waypoint N during active WP mission; payload: U8 wp_index (0-based, relative to mission start) #define MSP2_INAV_SET_CRUISE_HEADING 0x2223 //in message set heading while in Cruise/Course Hold mode; payload: I32 heading_centidegrees (0-35999) -#define MSP2_INAV_SET_AUX_RC 0x2230 \ No newline at end of file +#define MSP2_INAV_SET_AUX_RC 0x2230 + +#define MSP2_INAV_WIND 0x2231 From 4978a4c407a2c159f274757ec00b0dba8b117711 Mon Sep 17 00:00:00 2001 From: RunnyCow Date: Thu, 18 Jun 2026 11:44:07 +0800 Subject: [PATCH 04/38] Add target ACCTON_GFF4(STM32F405RGT6) --- src/main/target/ACCTON_GFF4/CMakeLists.txt | 1 + src/main/target/ACCTON_GFF4/config.c | 29 +++++ src/main/target/ACCTON_GFF4/target.c | 33 +++++ src/main/target/ACCTON_GFF4/target.h | 144 +++++++++++++++++++++ 4 files changed, 207 insertions(+) create mode 100644 src/main/target/ACCTON_GFF4/CMakeLists.txt create mode 100644 src/main/target/ACCTON_GFF4/config.c create mode 100644 src/main/target/ACCTON_GFF4/target.c create mode 100644 src/main/target/ACCTON_GFF4/target.h diff --git a/src/main/target/ACCTON_GFF4/CMakeLists.txt b/src/main/target/ACCTON_GFF4/CMakeLists.txt new file mode 100644 index 00000000000..6f671454546 --- /dev/null +++ b/src/main/target/ACCTON_GFF4/CMakeLists.txt @@ -0,0 +1 @@ +target_stm32f405xg(ACCTON_GFF4) diff --git a/src/main/target/ACCTON_GFF4/config.c b/src/main/target/ACCTON_GFF4/config.c new file mode 100644 index 00000000000..08906266f40 --- /dev/null +++ b/src/main/target/ACCTON_GFF4/config.c @@ -0,0 +1,29 @@ +/* + * This file is part of INAV. + * + * INAV is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * INAV is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with INAV. If not, see . + */ + +#include + +#include "platform.h" + +#include "drivers/serial.h" +#include "io/serial.h" + +void targetConfiguration(void) +{ + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART4)].functionMask = FUNCTION_RX_SERIAL; + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART3)].functionMask = FUNCTION_GPS; +} diff --git a/src/main/target/ACCTON_GFF4/target.c b/src/main/target/ACCTON_GFF4/target.c new file mode 100644 index 00000000000..0702b547365 --- /dev/null +++ b/src/main/target/ACCTON_GFF4/target.c @@ -0,0 +1,33 @@ +/* + * This file is part of INAV. + * + * INAV is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * INAV is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with INAV. If not, see . + */ + +#include + +#include "platform.h" +#include "drivers/io.h" +#include "drivers/timer.h" +#include "drivers/pwm_mapping.h" +#include "drivers/bus.h" + +timerHardware_t timerHardware[] = { + DEF_TIM(TIM8, CH1, PC6, TIM_USE_OUTPUT_AUTO, 0, 1), // S1 - DMA2_S2_Ch7 + DEF_TIM(TIM8, CH2, PC7, TIM_USE_OUTPUT_AUTO, 0, 1), // S2 - DMA2_S3_Ch7 + DEF_TIM(TIM8, CH3, PC8, TIM_USE_OUTPUT_AUTO, 0, 1), // S3 - DMA2_S4_Ch7 + DEF_TIM(TIM8, CH4, PC9, TIM_USE_OUTPUT_AUTO, 0, 0), // S4 - DMA2_S7_Ch7 +}; + +const int timerHardwareCount = sizeof(timerHardware) / sizeof(timerHardware[0]); diff --git a/src/main/target/ACCTON_GFF4/target.h b/src/main/target/ACCTON_GFF4/target.h new file mode 100644 index 00000000000..aa2d9b9d0f4 --- /dev/null +++ b/src/main/target/ACCTON_GFF4/target.h @@ -0,0 +1,144 @@ +/* + * This file is part of INAV. + * + * INAV is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * INAV is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with INAV. If not, see . + */ + +#pragma once + +#define USE_TARGET_CONFIG + +#define TARGET_BOARD_IDENTIFIER "G-FF4" +#define USBD_PRODUCT_STRING "ACCT" + +// ******** Board LEDs ********************** +#define LED0 PB14 +#define LED0_INVERTED +#define LED1 PB3 +#define LED1_INVERTED + +// ******* Beeper *********** +#define BEEPER PB15 +#define BEEPER_INVERTED + +// ******* GYRO and ACC ******** +#define USE_IMU_ICM42605 +#define IMU_ICM42605_ALIGN CW0_DEG +#define ICM42605_SPI_BUS BUS_SPI1 +#define ICM42605_CS_PIN PA4 + +// *************** I2C *************************** +#define USE_I2C +#define USE_I2C_DEVICE_1 + +#define I2C1_SCL PB6 +#define I2C1_SDA PB7 +#define DEFAULT_I2C_BUS BUS_I2C1 + +// *************** Baro ************************** +#define USE_BARO +#define BARO_I2C_BUS DEFAULT_I2C_BUS +#define USE_BARO_DPS310 +#define DPS310_I2C_ADDR (0x77) + +// *********** Magnetometer / Compass ************* +#define USE_MAG +#define MAG_I2C_BUS DEFAULT_I2C_BUS +#define USE_MAG_IST8310 + +// ******* SERIAL ******** +#define USE_VCP + +#define USE_UART1 +#define UART1_TX_PIN PA9 +#define UART1_RX_PIN PA10 + +#define USE_UART2 +#define UART2_TX_PIN PA2 +#define UART2_RX_PIN PA3 + +#define USE_UART3 +#define UART3_TX_PIN PB10 +#define UART3_RX_PIN PB11 + +#define USE_UART4 +#define UART4_TX_PIN PA0 +#define UART4_RX_PIN PA1 + +#define SERIAL_PORT_COUNT 5 + +// ******* SPI ******** +#define USE_SPI + +#define USE_SPI_DEVICE_1 +#define SPI1_NSS_PIN PA4 +#define SPI1_SCK_PIN PA5 +#define SPI1_MISO_PIN PA6 +#define SPI1_MOSI_PIN PA7 + +#define USE_SPI_DEVICE_2 +#define SPI2_NSS_PIN PB12 +#define SPI2_SCK_PIN PB13 +#define SPI2_MISO_PIN PC2 +#define SPI2_MOSI_PIN PC3 + +#define USE_SPI_DEVICE_3 +#define SPI3_NSS_PIN PA15 +#define SPI3_SCK_PIN PC10 +#define SPI3_MISO_PIN PC11 +#define SPI3_MOSI_PIN PC12 + +// ******* ADC ******** +#define USE_ADC +#define ADC_CHANNEL_1_PIN PC4 +#define ADC_CHANNEL_2_PIN PC5 + +#define VBAT_ADC_CHANNEL ADC_CHN_1 +#define CURRENT_METER_ADC_CHANNEL ADC_CHN_2 + +#define VBAT_SCALE_DEFAULT 841 +#define CURRENT_METER_SCALE 250 + +// ******* OSD ******** +#define USE_OSD +#define USE_MAX7456 +#define MAX7456_SPI_BUS BUS_SPI2 +#define MAX7456_CS_PIN PB12 + +// ******* FLASH ******** +#define USE_FLASHFS +#define USE_FLASH_W25N01G +#define W25N01G_SPI_BUS BUS_SPI3 +#define W25N01G_CS_PIN PA15 +#define ENABLE_BLACKBOX_LOGGING_ON_SPIFLASH_BY_DEFAULT + +// ******* FEATURES ******** +#define DEFAULT_RX_TYPE RX_TYPE_SERIAL +#define SERIALRX_UART SERIAL_PORT_USART4 +#define SERIALRX_PROVIDER SERIALRX_CRSF + +#define DEFAULT_FEATURES (FEATURE_OSD | FEATURE_TELEMETRY | FEATURE_CURRENT_METER | FEATURE_VBAT | FEATURE_TX_PROF_SEL) + +#define TARGET_IO_PORTA 0xffff +#define TARGET_IO_PORTB 0xffff +#define TARGET_IO_PORTC 0xffff +#define TARGET_IO_PORTD (BIT(2)) + +#define MAX_PWM_OUTPUT_PORTS 4 + +// ESC-related features +#define USE_DSHOT +#define USE_SERIALSHOT +#define USE_ESC_SENSOR +#define USE_SERIAL_4WAY_BLHELI_INTERFACE From 1dcdfd926da2119747da52de449e0f518f6ce1a8 Mon Sep 17 00:00:00 2001 From: orbittechnologyx Date: Wed, 8 Jul 2026 17:57:21 +0300 Subject: [PATCH 05/38] Added target ORBITH743v2 --- src/main/target/ORBITH743v2/CMakeLists.txt | 1 + src/main/target/ORBITH743v2/config.c | 51 ++++++ src/main/target/ORBITH743v2/target.c | 54 ++++++ src/main/target/ORBITH743v2/target.h | 201 +++++++++++++++++++++ 4 files changed, 307 insertions(+) create mode 100644 src/main/target/ORBITH743v2/CMakeLists.txt create mode 100644 src/main/target/ORBITH743v2/config.c create mode 100644 src/main/target/ORBITH743v2/target.c create mode 100644 src/main/target/ORBITH743v2/target.h diff --git a/src/main/target/ORBITH743v2/CMakeLists.txt b/src/main/target/ORBITH743v2/CMakeLists.txt new file mode 100644 index 00000000000..d33b7c6d1c3 --- /dev/null +++ b/src/main/target/ORBITH743v2/CMakeLists.txt @@ -0,0 +1 @@ +target_stm32h743xi(ORBITH743v2) diff --git a/src/main/target/ORBITH743v2/config.c b/src/main/target/ORBITH743v2/config.c new file mode 100644 index 00000000000..80de8a800ec --- /dev/null +++ b/src/main/target/ORBITH743v2/config.c @@ -0,0 +1,51 @@ +/* + * This file is part of Cleanflight. + * + * Cleanflight is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Cleanflight is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Cleanflight. If not, see . + */ + +#include +#include +#include +#include "fc/fc_msp_box.h" +#include "io/piniobox.h" +#include "io/serial.h" +#include "common/axis.h" +#include "config/config_master.h" +#include "config/feature.h" +#include "drivers/sensor.h" +#include "drivers/pwm_esc_detect.h" +#include "drivers/pwm_output.h" +#include "drivers/pwm_mapping.h" +#include "drivers/serial.h" +#include "fc/rc_controls.h" +#include "flight/failsafe.h" +#include "flight/mixer.h" +#include "flight/pid.h" +#include "rx/rx.h" +#include "io/serial.h" +#include "sensors/battery.h" +#include "sensors/sensors.h" +#include "telemetry/telemetry.h" + + +void targetConfiguration(void) +{ + pinioBoxConfigMutable()->permanentId[0] = BOX_PERMANENT_ID_USER1; + pinioBoxConfigMutable()->permanentId[1] = BOX_PERMANENT_ID_USER2; + + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART1)].functionMask = FUNCTION_ESCSERIAL; + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART6)].functionMask = FUNCTION_GPS; + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART5)].functionMask = FUNCTION_RX_SERIAL; +} diff --git a/src/main/target/ORBITH743v2/target.c b/src/main/target/ORBITH743v2/target.c new file mode 100644 index 00000000000..ae8dd3db42b --- /dev/null +++ b/src/main/target/ORBITH743v2/target.c @@ -0,0 +1,54 @@ +/* + * This file is part of INAV. + * + * INAV is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * INAV is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with INAV. If not, see . + */ + +#include + +#include "platform.h" + +#include "drivers/bus.h" +#include "drivers/io.h" +#include "drivers/pwm_mapping.h" +#include "drivers/timer.h" +#include "drivers/pinio.h" +#include "drivers/sensor.h" + +BUSDEV_REGISTER_SPI_TAG(busdev_icm42688_1, DEVHW_ICM42605, ICM42688_SPI_BUS_1, ICM42688_CS_PIN_1, NONE, 0, DEVFLAGS_NONE, IMU_ICM42688_ALIGN_1); +//BUSDEV_REGISTER_SPI_TAG(busdev_icm40609, DEVHW_ICM42605, ICM40609_SPI_BUS, ICM40609_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_ICM40609_ALIGN); + + +timerHardware_t timerHardware[] = { + DEF_TIM(TIM5, CH1, PA0, TIM_USE_OUTPUT_AUTO, 0, 0), // S1 + DEF_TIM(TIM5, CH2, PA1, TIM_USE_OUTPUT_AUTO, 0, 1), // S2 + DEF_TIM(TIM5, CH3, PA2, TIM_USE_OUTPUT_AUTO, 0, 2), // S3 + DEF_TIM(TIM5, CH4, PA3, TIM_USE_OUTPUT_AUTO, 0, 3), // S4 + + DEF_TIM(TIM3, CH3, PB0, TIM_USE_OUTPUT_AUTO, 0, 4), // S5 + DEF_TIM(TIM3, CH4, PB1, TIM_USE_OUTPUT_AUTO, 0, 5), // S6 + DEF_TIM(TIM3, CH1, PB4, TIM_USE_OUTPUT_AUTO, 0, 6), // S7 + DEF_TIM(TIM3, CH2, PB5, TIM_USE_OUTPUT_AUTO, 0, 7), // S8 + + DEF_TIM(TIM2, CH1, PA15, TIM_USE_OUTPUT_AUTO, 0, 0), // S9 + DEF_TIM(TIM2, CH2, PB3, TIM_USE_OUTPUT_AUTO, 0, 0), // S10 DMA_NONE + DEF_TIM(TIM4, CH1, PD12, TIM_USE_OUTPUT_AUTO, 0, 0), // S11 + DEF_TIM(TIM4, CH2, PD13, TIM_USE_OUTPUT_AUTO, 0, 0), // S12 DMA_NONE + + DEF_TIM(TIM1, CH1, PA8, TIM_USE_LED, 0, 14), // LED_2812 + DEF_TIM(TIM12, CH2, PB15, TIM_USE_PPM, 0, 0), // PPM + +}; + +const int timerHardwareCount = sizeof(timerHardware) / sizeof(timerHardware[0]); diff --git a/src/main/target/ORBITH743v2/target.h b/src/main/target/ORBITH743v2/target.h new file mode 100644 index 00000000000..5d7d48a9137 --- /dev/null +++ b/src/main/target/ORBITH743v2/target.h @@ -0,0 +1,201 @@ +/* + * This file is part of INAV. + * + * INAV is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * INAV is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with INAV. If not, see . + */ + + +#pragma once + +#define TARGET_BOARD_IDENTIFIER "ORB7V2" +#define USBD_PRODUCT_STRING "ORBITH743v2" + +#define USE_TARGET_CONFIG + +#define LED0 PE3 //Blue +#define LED1 PE4 //Red +#define LED2 PC4 //Green + +#define BEEPER PD7 +#define BEEPER_INVERTED + + +// *************** SPI ***************************** +#define USE_SPI +#define USE_SPI_DEVICE_1 +#define USE_SPI_DEVICE_2 +#define USE_SPI_DEVICE_3 +#define USE_SPI_DEVICE_4 + +#define SPI1_SCK_PIN PA5 +#define SPI1_MISO_PIN PA6 +#define SPI1_MOSI_PIN PA7 + +#define SPI2_SCK_PIN PA9 +#define SPI2_MISO_PIN PC2 +#define SPI2_MOSI_PIN PC3 + +#define SPI3_SCK_PIN PC10 +#define SPI3_MISO_PIN PC11 +#define SPI3_MOSI_PIN PC12 + +#define SPI4_SCK_PIN PE12 +#define SPI4_MISO_PIN PE13 +#define SPI4_MOSI_PIN PE14 + +// *************** I2C ************************** +#define USE_I2C +#define USE_I2C_DEVICE_1 +#define USE_I2C_DEVICE_2 + +#define I2C1_SCL PB6 +#define I2C1_SDA PB7 + +#define I2C2_SCL PB10 +#define I2C2_SDA PB11 + +// *************** UART ***************************** +#define USE_VCP +#define USE_UART1 +#define USE_UART2 +#define USE_UART3 +#define USE_UART4 +#define USE_UART5 +#define USE_UART6 +#define USE_UART7 +#define USE_UART8 + +#define UART1_TX_PIN PB14 +#define UART1_RX_PIN PA10 + +#define UART2_TX_PIN PD5 +#define UART2_RX_PIN PD6 + +#define UART3_TX_PIN PD8 +#define UART3_RX_PIN PD9 + +#define UART4_TX_PIN PD1 +#define UART4_RX_PIN PD0 + +#define UART5_TX_PIN PB13 +#define UART5_RX_PIN PB12 + +#define UART6_TX_PIN PC6 +#define UART6_RX_PIN PC7 + +#define UART7_TX_PIN PE8 +#define UART7_RX_PIN PE7 + +#define UART8_TX_PIN PE1 +#define UART8_RX_PIN PE0 + +#define SERIAL_PORT_COUNT 9 //VCP, UART1, UART2, UART3, UART4, UART5, UART6, UART7, UART8 + +#define DEFAULT_RX_TYPE RX_TYPE_SERIAL +#define SERIALRX_PROVIDER SERIALRX_CRSF +#define SERIALRX_UART SERIAL_PORT_USART5 + + +// *************** Gyro & ACC ********************** +//#define USE_DUAL_GYRO +#define USE_TARGET_IMU_HARDWARE_DESCRIPTORS + +#define USE_IMU_ICM42605 + +#define IMU_ICM42688_ALIGN_1 CW270_DEG +#define ICM42688_CS_PIN_1 PE11 +#define ICM42688_SPI_BUS_1 BUS_SPI1 +#define ICM42688_EXTI_PIN_1 PE10 + +//#define IMU_ICM40609_ALIGN CW270_DEG +//#define ICM40609_CS_PIN PD3 +//#define ICM40609_SPI_BUS BUS_SPI3 +//#define ICM40609_EXTI_PIN PD2 + + +// *************** OSD ***************************** +#define USE_MAX7456 +#define MAX7456_SPI_BUS BUS_SPI2 +#define MAX7456_CS_PIN PE6 + +// *************** FLASH *************************** +#define M25P16_SPI_BUS BUS_SPI4 +#define M25P16_CS_PIN PE9 + +#define USE_BLACKBOX +#define USE_FLASHFS +#define USE_FLASH_M25P16 +#define ENABLE_BLACKBOX_LOGGING_ON_SPIFLASH_BY_DEFAULT + +// *************** Baro/Mag ********************* + +#define USE_BARO +#define BARO_I2C_BUS BUS_I2C2 +#define USE_BARO_BMP280 +#define USE_BARO_MS5611 +#define USE_BARO_SPL06 +#define USE_BARO_DPS310 + +#define SPL06_I2C_ADDR 0x76 + +#define USE_MAG +#define MAG_I2C_BUS BUS_I2C1 +#define USE_MAG_ALL + +#define TEMPERATURE_I2C_BUS BUS_I2C1 +#define PITOT_I2C_BUS BUS_I2C1 + +#define USE_RANGEFINDER +#define RANGEFINDER_I2C_BUS BUS_I2C1 + + +// *************** ADC ***************************** +#define USE_ADC +#define ADC_INSTANCE ADC1 + +#define ADC_CHANNEL_1_PIN PC0 //ADC123 VBAT1 +#define ADC_CHANNEL_2_PIN PC1 //ADC123 CURR1 +#define ADC_CHANNEL_3_PIN PC5 //ADC12 RSSI + +#define VBAT_ADC_CHANNEL ADC_CHN_1 +#define CURRENT_METER_ADC_CHANNEL ADC_CHN_2 +#define RSSI_ADC_CHANNEL ADC_CHN_3 + +// *************** PINIO *************************** +#define USE_PINIO +#define USE_PINIOBOX +#define PINIO1_PIN PE2 // VTX power switcher +#define PINIO2_PIN PD4 +#define PINIO1_FLAGS PINIO_FLAGS_INVERTED + +// *************** LEDSTRIP ************************ +#define USE_LED_STRIP +#define WS2811_PIN PA8 + +#define DEFAULT_FEATURES (FEATURE_OSD | FEATURE_TELEMETRY | FEATURE_CURRENT_METER | FEATURE_VBAT | FEATURE_TX_PROF_SEL | FEATURE_BLACKBOX) +#define CURRENT_METER_SCALE 100 +#define VBAT_SCALE_DEFAULT 1100 + + +#define USE_SERIAL_4WAY_BLHELI_INTERFACE + +#define TARGET_IO_PORTA 0xffff +#define TARGET_IO_PORTB 0xffff +#define TARGET_IO_PORTC 0xffff +#define TARGET_IO_PORTD 0xffff +#define TARGET_IO_PORTE 0xffff + +#define MAX_PWM_OUTPUT_PORTS 15 +#define USE_DSHOT +#define USE_ESC_SENSOR From a725524edb4e356a6ce4336ad3533627a8a9b05c Mon Sep 17 00:00:00 2001 From: orbittechnologyx Date: Thu, 9 Jul 2026 16:26:00 +0300 Subject: [PATCH 06/38] Enabled dual gyro (ICM42688P + ICM40609D) on the ORBITH743v2 target --- src/main/target/ORBITH743v2/target.c | 2 +- src/main/target/ORBITH743v2/target.h | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/target/ORBITH743v2/target.c b/src/main/target/ORBITH743v2/target.c index ae8dd3db42b..aaa8776c74e 100644 --- a/src/main/target/ORBITH743v2/target.c +++ b/src/main/target/ORBITH743v2/target.c @@ -27,7 +27,7 @@ #include "drivers/sensor.h" BUSDEV_REGISTER_SPI_TAG(busdev_icm42688_1, DEVHW_ICM42605, ICM42688_SPI_BUS_1, ICM42688_CS_PIN_1, NONE, 0, DEVFLAGS_NONE, IMU_ICM42688_ALIGN_1); -//BUSDEV_REGISTER_SPI_TAG(busdev_icm40609, DEVHW_ICM42605, ICM40609_SPI_BUS, ICM40609_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_ICM40609_ALIGN); +BUSDEV_REGISTER_SPI_TAG(busdev_icm40609, DEVHW_ICM42605, ICM40609_SPI_BUS, ICM40609_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_ICM40609_ALIGN); timerHardware_t timerHardware[] = { diff --git a/src/main/target/ORBITH743v2/target.h b/src/main/target/ORBITH743v2/target.h index 5d7d48a9137..97d31ebb54c 100644 --- a/src/main/target/ORBITH743v2/target.h +++ b/src/main/target/ORBITH743v2/target.h @@ -108,7 +108,7 @@ // *************** Gyro & ACC ********************** -//#define USE_DUAL_GYRO +#define USE_DUAL_GYRO #define USE_TARGET_IMU_HARDWARE_DESCRIPTORS #define USE_IMU_ICM42605 @@ -118,10 +118,10 @@ #define ICM42688_SPI_BUS_1 BUS_SPI1 #define ICM42688_EXTI_PIN_1 PE10 -//#define IMU_ICM40609_ALIGN CW270_DEG -//#define ICM40609_CS_PIN PD3 -//#define ICM40609_SPI_BUS BUS_SPI3 -//#define ICM40609_EXTI_PIN PD2 +#define IMU_ICM40609_ALIGN CW270_DEG +#define ICM40609_CS_PIN PD3 +#define ICM40609_SPI_BUS BUS_SPI3 +#define ICM40609_EXTI_PIN PD2 // *************** OSD ***************************** From e27a084b5a64ffb7188783ef6221892f2e16421e Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 2 Aug 2026 21:57:47 -0500 Subject: [PATCH 07/38] Fix settings.rb check_conditions()/resolve_types() header-order disagreement check_conditions() deferred every #if defined(MACRO) condition check to the end of one combined probe file, after all group headers were already included, so it saw a macro as "on" if defined anywhere regardless of order. resolve_types() instead probes a struct field access, which is governed by that struct's own header's #ifdef guard evaluated at its actual position in the same header sequence. When a macro was only defined by a header appearing later than the struct needing it, the two functions could disagree, crashing settings generation. Interleave each group's headers with that group's own condition checks in one pass, so check_conditions() evaluates macro visibility at the same in-order position resolve_types() implicitly relies on. --- src/utils/settings.rb | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/utils/settings.rb b/src/utils/settings.rb index bbc76f9ebd5..604737c7b3f 100644 --- a/src/utils/settings.rb +++ b/src/utils/settings.rb @@ -763,10 +763,13 @@ def mktmpdir value end + # cstddef for offsetof(). Shared by every probe so they all see the same + # base macro state. + BASE_HEADERS = ["platform.h", "cstddef"] + def compile_test_file(prog) buf = StringIO.new - # cstddef for offsetof() - headers = ["platform.h", "cstddef"] + headers = BASE_HEADERS.dup @data["groups"].each do |group| gh = group["headers"] if gh @@ -780,6 +783,10 @@ def compile_test_file(prog) end buf << "\n" buf << prog.string + compile_raw(buf) + end + + def compile_raw(buf) mktmpdir do |dir| file = File.join(dir, "test.cpp") File.open(file, 'w') {|file| file.write(buf.string)} @@ -792,6 +799,10 @@ def compile_test_file(prog) def check_conditions buf = StringIO.new + BASE_HEADERS.each do |h| + buf << "#include \"#{h}\"\n" + end + conditions = Set.new add_condition = -> (c) { if c && !conditions.include?(c) @@ -822,12 +833,24 @@ def check_conditions end } - foreach_member do |group, member| + # Check each condition right after that group's own headers, matching + # the position resolve_types() implicitly relies on: a header's own + # #ifdef guard only sees macros defined by headers earlier in this + # sequence. + @data["groups"].each do |group| + gh = group["headers"] + if gh + gh.each do |h| + buf << "#include \"#{h}\"\n" if h + end + end add_condition.call(group["condition"]) - add_condition.call(member["condition"]) + group["members"].each do |member| + add_condition.call(member["condition"]) + end end - stderr = compile_test_file(buf) + stderr = compile_raw(buf) @true_conditions = Set.new stderr.scan(/#pragma message\(\"(.*)\"\)/).each do |m| @true_conditions << m[0] From 5037d6a5b4c0ce45428a088e59f8a485745c0bc9 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 2 Aug 2026 22:33:21 -0500 Subject: [PATCH 08/38] Track condition truth per group/member occurrence, not by condition string check_conditions() deduped its probes by condition string, so a condition was only ever checked once, at whichever group/member first used it. Since the probe's truth is now position-dependent (per the previous commit), the same condition string appearing on a later group at a different header- inclusion position would silently reuse the earlier, possibly-wrong result instead of being re-evaluated for its own position. Several real conditions in settings.yaml (USE_ADC, USE_BARO, USE_PITOT, ...) are reused across multiple groups, so this was live, not theoretical. Fixed by tagging each group/member occurrence with its own unique probe and storing the resolved boolean directly on that object, so occurrences of the same condition string at different positions are evaluated independently. Verified with a synthetic fixture reproducing the same condition string on two groups straddling the header that defines it: the earlier group now correctly resolves false and the later group true, where both previously resolved to the same (wrong, for one of them) value. Confirmed byte- identical generated settings_generated.h/.c for MATEKF722PX and the maths/time/msp_refactor unit-test targets, and the same 57/58 unit test pass rate as before (one pre-existing, unrelated failure). --- src/utils/settings.rb | 84 +++++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 34 deletions(-) diff --git a/src/utils/settings.rb b/src/utils/settings.rb index 604737c7b3f..19fa0e1ab3b 100644 --- a/src/utils/settings.rb +++ b/src/utils/settings.rb @@ -490,7 +490,7 @@ def write_impl_file(file) foreach_enabled_group do |group| count = 0 group["members"].each do |member| - if is_condition_enabled(member["condition"]) + if is_condition_enabled(member) count += 1 end end @@ -665,16 +665,21 @@ def resolve_range(member) return min, max end - def is_condition_enabled(cond) - return !cond || @true_conditions.include?(cond) + # Condition truth is resolved per group/member occurrence (see + # check_conditions), not by condition string, since the same condition + # string can appear on multiple groups at different header-inclusion + # positions and be true at one but not the other. + def is_condition_enabled(obj) + return true unless obj["condition"] + obj["_condition_enabled"] end def foreach_enabled_member &block enum = Enumerator.new do |yielder| groups.each do |group| - if is_condition_enabled(group["condition"]) + if is_condition_enabled(group) group["members"].each do |member| - if is_condition_enabled(member["condition"]) + if is_condition_enabled(member) yielder.yield group, member end end @@ -803,34 +808,42 @@ def check_conditions buf << "#include \"#{h}\"\n" end - conditions = Set.new - add_condition = -> (c) { - if c && !conditions.include?(c) - conditions.add(c) - buf << "#if " - in_word = false - c.split('').each do |ch| - if in_word - if !ch.match(/^[a-zA-Z0-9_]$/) - in_word = false - buf << ")" - end - buf << ch - else - if ch.match(/^[a-zA-Z_]$/) - in_word = true - buf << "defined(" - end - buf << ch - end - end + # Tag each group/member occurrence with its own unique probe, rather + # than deduping by condition string: the same condition string can + # appear on multiple groups, and since a probe's truth now depends on + # its position (see comment below), one occurrence being true doesn't + # mean another occurrence of the same string is. + tags = {} + seq = 0 + add_condition = -> (obj) { + c = obj["condition"] + next unless c + seq += 1 + tag = "COND#{seq}" + tags[tag] = obj + buf << "#if " + in_word = false + c.split('').each do |ch| if in_word - buf << ")" + if !ch.match(/^[a-zA-Z0-9_]$/) + in_word = false + buf << ")" + end + buf << ch + else + if ch.match(/^[a-zA-Z_]$/) + in_word = true + buf << "defined(" + end + buf << ch end - buf << "\n" - buf << "#pragma message(#{c.inspect})\n" - buf << "#endif\n" end + if in_word + buf << ")" + end + buf << "\n" + buf << "#pragma message(#{tag.inspect})\n" + buf << "#endif\n" } # Check each condition right after that group's own headers, matching @@ -844,16 +857,19 @@ def check_conditions buf << "#include \"#{h}\"\n" if h end end - add_condition.call(group["condition"]) + add_condition.call(group) group["members"].each do |member| - add_condition.call(member["condition"]) + add_condition.call(member) end end stderr = compile_raw(buf) - @true_conditions = Set.new + true_tags = Set.new stderr.scan(/#pragma message\(\"(.*)\"\)/).each do |m| - @true_conditions << m[0] + true_tags << m[0] + end + tags.each do |tag, obj| + obj["_condition_enabled"] = true_tags.include?(tag) end end From c29dc5401f95fbcf96fa7c42a545d110baed486c Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 2 Aug 2026 23:21:38 -0500 Subject: [PATCH 09/38] Fix MSP2_INAV_WIND returning stale non-zero values when invalid getEstimatedHorizontalWindSpeed() was called unconditionally, so once the wind estimate becomes invalid (e.g. the 15-minute stationary-altitude timeout in wind_estimator.c), MSP2_INAV_WIND kept sending the last computed windSpeed/windAngle instead of zero. estimatedWind[] is never reset when hasValidWindEstimate clears, only the flags byte reflected validity. This contradicted the message's own documentation ("returns zeroes when wind estimation is not compiled in or not yet valid") and diverged from every other consumer of this API (gps.c, osd.c, rth_estimator.c, navigation.c, imu.c, mavlink_streams.c, logic_condition.c, pitotmeter.c), all of which check isEstimatedWindSpeedValid() before reading the value rather than relying on the estimator to self-zero. Reported by Qodo's automated review on PR #11761 (a release/9.1 -> maintenance-10.x merge that carried this pre-existing bug forward, unrelated to that merge itself). Fixed at the source (release/9.1) so it flows forward on the next maintenance-10.x sync. --- src/main/fc/fc_msp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/fc/fc_msp.c b/src/main/fc/fc_msp.c index 92eecfc88b5..961d1737c95 100644 --- a/src/main/fc/fc_msp.c +++ b/src/main/fc/fc_msp.c @@ -1604,8 +1604,12 @@ static bool mspFcProcessOutCommand(uint16_t cmdMSP, sbuf_t *dst, mspPostProcessF #ifdef USE_WIND_ESTIMATOR { uint16_t windAngle = 0; - uint16_t windSpeed = (uint16_t)getEstimatedHorizontalWindSpeed(&windAngle); - uint8_t windFlags = isEstimatedWindSpeedValid() ? 1 : 0; + uint16_t windSpeed = 0; + uint8_t windFlags = 0; + if (isEstimatedWindSpeedValid()) { + windSpeed = (uint16_t)getEstimatedHorizontalWindSpeed(&windAngle); + windFlags = 1; + } sbufWriteU16(dst, windSpeed); sbufWriteU16(dst, windAngle / 100); sbufWriteU8(dst, windFlags); From f24823323ef862431bd920f3924eadecc44b35f7 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 12:16:37 -0500 Subject: [PATCH 10/38] Restore explicit resolutionType_e wire values for DJI WTFOS OSD Removing HD_3016 from the enum in commit 7ec7f0d190 (PR #10540) shifted every later member's implicit ordinal down by one, so currentOsdMode now sends HD_6022 as wire value 2 instead of 3. WTFOS goggles/VTX firmware hardcodes 3 for its 60x22 mode, so the OSD canvas renders shifted. Fixes #11764 --- src/main/io/displayport_msp_osd.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/io/displayport_msp_osd.c b/src/main/io/displayport_msp_osd.c index 6b674a300ec..50e8cf3f5a6 100644 --- a/src/main/io/displayport_msp_osd.c +++ b/src/main/io/displayport_msp_osd.c @@ -61,11 +61,15 @@ #define FONT_VERSION 3 +// Values are wire-protocol constants sent as a raw byte over MSP DisplayPort +// (MSP_DP_OPTIONS) and must stay fixed — third-party OSD firmware (e.g. +// WTFOS) hardcodes these ordinals, so don't let enum edits renumber them. typedef enum { // defines are from hdzero code - SD_3016, - HD_5018, - HD_6022, // added to support DJI wtfos 60x22 grid - HD_5320 // added to support Avatar and BetaflightHD + SD_3016 = 0, + HD_5018 = 1, + HD_3016 = 2, // unused by INAV; reserved to hold hdzero's canonical ordinal 2 + HD_6022 = 3, // DJI wtfos 60x22 grid + HD_5320 = 4 // Avatar and BetaflightHD } resolutionType_e; #define DRAW_FREQ_DENOM 4 // 60Hz From 48f7fe622f8690d28f9f67bff5ea4a7655d1c814 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 12:29:41 -0500 Subject: [PATCH 11/38] Update MSP enum reference docs for resolutionType_e Keep the published protocol reference in sync with the corrected wire values for resolutionType_e. --- docs/development/msp/inav_enums.json | 5 +++-- docs/development/msp/inav_enums_ref.md | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/development/msp/inav_enums.json b/docs/development/msp/inav_enums.json index 63f3eb2941b..a60e3ab37ac 100644 --- a/docs/development/msp/inav_enums.json +++ b/docs/development/msp/inav_enums.json @@ -3301,8 +3301,9 @@ "_source": "inav/src/main/io/displayport_msp_osd.c", "SD_3016": "0", "HD_5018": "1", - "HD_6022": "2", - "HD_5320": "3" + "HD_3016": "2", + "HD_6022": "3", + "HD_5320": "4" }, "resourceOwner_e": { "_source": "inav/src/main/drivers/resource.h", diff --git a/docs/development/msp/inav_enums_ref.md b/docs/development/msp/inav_enums_ref.md index 802e813ab43..5243753c4a9 100644 --- a/docs/development/msp/inav_enums_ref.md +++ b/docs/development/msp/inav_enums_ref.md @@ -4829,8 +4829,9 @@ |---|---:|---| | `SD_3016` | 0 | | | `HD_5018` | 1 | | -| `HD_6022` | 2 | | -| `HD_5320` | 3 | | +| `HD_3016` | 2 | | +| `HD_6022` | 3 | | +| `HD_5320` | 4 | | --- ## `resourceOwner_e` From 5c5a81ad231ff7e90f3a6479cff75e3dae6928a5 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 12:59:24 -0500 Subject: [PATCH 12/38] Regenerate MSP enum reference docs from current source Run the actual generator instead of hand-editing, so the docs match gen_docs.sh output exactly and pick up other enums that had drifted since these files were last regenerated. --- docs/development/msp/inav_enums.json | 61 +++++++++++++++++++------- docs/development/msp/inav_enums_ref.md | 61 +++++++++++++++++++++----- 2 files changed, 95 insertions(+), 27 deletions(-) diff --git a/docs/development/msp/inav_enums.json b/docs/development/msp/inav_enums.json index a60e3ab37ac..6c5ce2b96fe 100644 --- a/docs/development/msp/inav_enums.json +++ b/docs/development/msp/inav_enums.json @@ -849,13 +849,14 @@ "DEVHW_TOF10120_I2C": "48", "DEVHW_TERARANGER_EVO_I2C": "49", "DEVHW_MS4525": "50", - "DEVHW_DLVR": "51", - "DEVHW_M25P16": "52", - "DEVHW_W25N": "53", - "DEVHW_UG2864": "54", - "DEVHW_SDCARD": "55", - "DEVHW_IRLOCK": "56", - "DEVHW_PCF8574": "57" + "DEVHW_MS5525": "51", + "DEVHW_DLVR": "52", + "DEVHW_M25P16": "53", + "DEVHW_W25N": "54", + "DEVHW_UG2864": "55", + "DEVHW_SDCARD": "56", + "DEVHW_IRLOCK": "57", + "DEVHW_PCF8574": "58" }, "deviceFlags_e": { "_source": "inav/src/main/drivers/bus.h", @@ -1764,6 +1765,12 @@ "IBUS_MEAS_VALUE_GPS_LON2": "145", "IBUS_MEAS_VALUE_GPS": "253" }, + "icm42605Variant_e": { + "_source": "inav/src/main/drivers/accgyro/accgyro_icm42605.c", + "ICM42605_VARIANT_42605": "0", + "ICM42605_VARIANT_42686P": "1", + "ICM42605_VARIANT_42688P": "2" + }, "inputSource_e": { "_source": "inav/src/main/flight/servos.h", "INPUT_STABILIZED_ROLL": "0", @@ -2123,6 +2130,7 @@ "LSM6DXX_VAL_CTRL1_XL_LPF2": "1", "LSM6DXX_VAL_CTRL2_G_ODR6664": "10", "LSM6DXX_VAL_CTRL2_G_2000DPS": "3", + "LSM6DXX_VAL_CTRL3_C_BDU": "BIT(6)", "LSM6DXX_VAL_CTRL3_C_H_LACTIVE": "0", "LSM6DXX_VAL_CTRL3_C_PP_OD": "0", "LSM6DXX_VAL_CTRL3_C_SIM": "0", @@ -2140,7 +2148,21 @@ "LSM6DXX_VAL_CTRL7_G_HPM_G_65": "1", "LSM6DXX_VAL_CTRL7_G_HPM_G_260": "2", "LSM6DXX_VAL_CTRL7_G_HPM_G_1040": "3", - "LSM6DXX_VAL_CTRL9_XL_I3C_DISABLE": "BIT(1)" + "LSM6DXX_VAL_CTRL9_XL_I3C_DISABLE": "BIT(1)", + "LSM6DXX_VAL_CTRL1_XL_V_OPMODE_HIGH_ACCURACY": "1", + "LSM6DXX_VAL_CTRL1_XL_V_ODR_1000HZ_HAODR1": "9", + "LSM6DXX_VAL_CTRL2_G_V_OPMODE_HIGH_ACCURACY": "1", + "LSM6DXX_VAL_CTRL2_G_V_ODR_8000HZ_HAODR1": "12", + "LSM6DXX_VAL_HAODR_CFG_MODE1": "1", + "LSM6DXX_VAL_CTRL6_C_V_FS_G_2000DPS": "4", + "LSM6DXX_VAL_CTRL6_C_V_DSK320X_RESERVED_BIT3": "BIT(3)", + "LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_288HZ": "0", + "LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_215HZ": "1", + "LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_157HZ": "2", + "LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_455HZ": "3", + "LSM6DXX_VAL_CTRL7_G_V_LPF1_EN": "BIT(0)", + "LSM6DXX_VAL_CTRL8_XL_V_FS_16G": "3", + "LSM6DXX_VAL_CTRL4_C_V_DRDY_PULSED": "BIT(1)" }, "lsm6dxxRegister_e": { "_source": "inav/src/main/drivers/accgyro/accgyro_lsm6dxx.h", @@ -2172,7 +2194,8 @@ "LSM6DXX_REG_OUTY_L_A": "42", "LSM6DXX_REG_OUTY_H_A": "43", "LSM6DXX_REG_OUTZ_L_A": "44", - "LSM6DXX_REG_OUTZ_H_A": "45" + "LSM6DXX_REG_OUTZ_H_A": "45", + "LSM6DXX_REG_HAODR_CFG": "98" }, "ltm_frame_e": { "_source": "inav/src/main/telemetry/ltm.h", @@ -2335,7 +2358,8 @@ "MULTI_FUNC_3": "3", "MULTI_FUNC_4": "4", "MULTI_FUNC_5": "5", - "MULTI_FUNC_END": "6" + "MULTI_FUNC_6": "6", + "MULTI_FUNC_END": "7" }, "multiFunctionFlags_e": { "_source": "inav/src/main/fc/multifunction.h", @@ -3062,7 +3086,8 @@ "PITOT_VIRTUAL": "4", "PITOT_FAKE": "5", "PITOT_MSP": "6", - "PITOT_DLVR": "7" + "PITOT_DLVR": "7", + "PITOT_MS5525": "8" }, "pollType_e": { "_source": "inav/src/main/io/smartport_master.c", @@ -3470,7 +3495,7 @@ "SDCARD_OPERATION_FAILURE": "3" }, "sdcardReceiveBlockStatus_e": { - "_source": "inav/src/main/drivers/sdcard/sdcard_spi.c", + "_source": "inav/src/main/drivers/sdcard/sdcard_sdio.c", "SDCARD_RECEIVE_SUCCESS": "0", "SDCARD_RECEIVE_BLOCK_IN_PROGRESS": "1", "SDCARD_RECEIVE_ERROR": "2" @@ -3692,7 +3717,12 @@ "HITL_AIRSPEED": "(1 << 6)", "HITL_EXTENDED_FLAGS": "(1 << 7)", "HITL_GPS_TIMEOUT": "(1 << 8)", - "HITL_PITOT_FAILURE": "(1 << 9)" + "HITL_PITOT_FAILURE": "(1 << 9)", + "HITL_CURRENT_SENSOR": "(1 << 10)", + "HITL_SIM_RC_INPUT": "(1 << 11)", + "HITL_RANGEFINDER": "(1 << 12)", + "HITL_FAILSAFE_TRIGGERED": "(1 << 13)", + "HITL_SITL_MODE": "(1 << 14)" }, "smartAudioVersion_e": { "_source": "inav/src/main/io/vtx_smartaudio.h", @@ -3839,7 +3869,7 @@ "THR_HI": "(2 << (2 * THROTTLE))" }, "systemState_e": { - "_source": "inav/src/main/fc/fc_init.h", + "_source": "inav/src/main/fc/fc_init.c", "SYSTEM_STATE_INITIALISING": "0", "SYSTEM_STATE_CONFIG_LOADED": "(1 << 0)", "SYSTEM_STATE_SENSORS_READY": "(1 << 1)", @@ -3851,7 +3881,8 @@ "_source": "inav/src/main/drivers/timer.h", "TCH_DMA_IDLE": "0", "TCH_DMA_READY": "1", - "TCH_DMA_ACTIVE": "2" + "TCH_DMA_ACTIVE": "2", + "TCH_DMA_CIRCULAR": "3" }, "tempSensorType_e": { "_source": "inav/src/main/sensors/temperature.h", diff --git a/docs/development/msp/inav_enums_ref.md b/docs/development/msp/inav_enums_ref.md index 5243753c4a9..8ed0540edc9 100644 --- a/docs/development/msp/inav_enums_ref.md +++ b/docs/development/msp/inav_enums_ref.md @@ -150,6 +150,7 @@ - [ibusSensorType1_e](#enum-ibussensortype1_e) - [ibusSensorType_e](#enum-ibussensortype_e) - [ibusSensorValue_e](#enum-ibussensorvalue_e) +- [icm42605Variant_e](#enum-icm42605variant_e) - [inputSource_e](#enum-inputsource_e) - [itermRelax_e](#enum-itermrelax_e) - [led_pin_pwm_mode_e](#enum-led_pin_pwm_mode_e) @@ -1455,13 +1456,14 @@ | `DEVHW_TOF10120_I2C` | 48 | | | `DEVHW_TERARANGER_EVO_I2C` | 49 | | | `DEVHW_MS4525` | 50 | | -| `DEVHW_DLVR` | 51 | | -| `DEVHW_M25P16` | 52 | | -| `DEVHW_W25N` | 53 | | -| `DEVHW_UG2864` | 54 | | -| `DEVHW_SDCARD` | 55 | | -| `DEVHW_IRLOCK` | 56 | | -| `DEVHW_PCF8574` | 57 | | +| `DEVHW_MS5525` | 51 | | +| `DEVHW_DLVR` | 52 | | +| `DEVHW_M25P16` | 53 | | +| `DEVHW_W25N` | 54 | | +| `DEVHW_UG2864` | 55 | | +| `DEVHW_SDCARD` | 56 | | +| `DEVHW_IRLOCK` | 57 | | +| `DEVHW_PCF8574` | 58 | | --- ## `deviceFlags_e` @@ -2797,6 +2799,17 @@ | `IBUS_MEAS_VALUE_GPS_LON2` | 145 | | | `IBUS_MEAS_VALUE_GPS` | 253 | | +--- +## `icm42605Variant_e` + +> Source: ../../../src/main/drivers/accgyro/accgyro_icm42605.c + +| Enumerator | Value | Condition | +|---|---:|---| +| `ICM42605_VARIANT_42605` | 0 | | +| `ICM42605_VARIANT_42686P` | 1 | | +| `ICM42605_VARIANT_42688P` | 2 | | + --- ## `inputSource_e` @@ -3243,6 +3256,7 @@ | `LSM6DXX_VAL_CTRL1_XL_LPF2` | 1 | | | `LSM6DXX_VAL_CTRL2_G_ODR6664` | 10 | | | `LSM6DXX_VAL_CTRL2_G_2000DPS` | 3 | | +| `LSM6DXX_VAL_CTRL3_C_BDU` | BIT(6) | | | `LSM6DXX_VAL_CTRL3_C_H_LACTIVE` | 0 | | | `LSM6DXX_VAL_CTRL3_C_PP_OD` | 0 | | | `LSM6DXX_VAL_CTRL3_C_SIM` | 0 | | @@ -3261,6 +3275,20 @@ | `LSM6DXX_VAL_CTRL7_G_HPM_G_260` | 2 | | | `LSM6DXX_VAL_CTRL7_G_HPM_G_1040` | 3 | | | `LSM6DXX_VAL_CTRL9_XL_I3C_DISABLE` | BIT(1) | | +| `LSM6DXX_VAL_CTRL1_XL_V_OPMODE_HIGH_ACCURACY` | 1 | | +| `LSM6DXX_VAL_CTRL1_XL_V_ODR_1000HZ_HAODR1` | 9 | | +| `LSM6DXX_VAL_CTRL2_G_V_OPMODE_HIGH_ACCURACY` | 1 | | +| `LSM6DXX_VAL_CTRL2_G_V_ODR_8000HZ_HAODR1` | 12 | | +| `LSM6DXX_VAL_HAODR_CFG_MODE1` | 1 | | +| `LSM6DXX_VAL_CTRL6_C_V_FS_G_2000DPS` | 4 | | +| `LSM6DXX_VAL_CTRL6_C_V_DSK320X_RESERVED_BIT3` | BIT(3) | | +| `LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_288HZ` | 0 | | +| `LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_215HZ` | 1 | | +| `LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_157HZ` | 2 | | +| `LSM6DXX_VAL_CTRL6_C_V_LPF1_BW_455HZ` | 3 | | +| `LSM6DXX_VAL_CTRL7_G_V_LPF1_EN` | BIT(0) | | +| `LSM6DXX_VAL_CTRL8_XL_V_FS_16G` | 3 | | +| `LSM6DXX_VAL_CTRL4_C_V_DRDY_PULSED` | BIT(1) | | --- ## `lsm6dxxRegister_e` @@ -3298,6 +3326,7 @@ | `LSM6DXX_REG_OUTY_H_A` | 43 | | | `LSM6DXX_REG_OUTZ_L_A` | 44 | | | `LSM6DXX_REG_OUTZ_H_A` | 45 | | +| `LSM6DXX_REG_HAODR_CFG` | 98 | | --- ## `ltm_frame_e` @@ -3541,7 +3570,8 @@ | `MULTI_FUNC_3` | 3 | | | `MULTI_FUNC_4` | 4 | | | `MULTI_FUNC_5` | 5 | | -| `MULTI_FUNC_END` | 6 | | +| `MULTI_FUNC_6` | 6 | | +| `MULTI_FUNC_END` | 7 | | --- ## `multiFunctionFlags_e` @@ -4544,6 +4574,7 @@ | `PITOT_FAKE` | 5 | | | `PITOT_MSP` | 6 | | | `PITOT_DLVR` | 7 | | +| `PITOT_MS5525` | 8 | | --- ## `pollType_e` @@ -5080,7 +5111,7 @@ --- ## `sdcardReceiveBlockStatus_e` -> Source: ../../../src/main/drivers/sdcard/sdcard_sdio.c +> Source: ../../../src/main/drivers/sdcard/sdcard_spi.c | Enumerator | Value | Condition | |---|---:|---| @@ -5091,7 +5122,7 @@ --- ## `sdcardReceiveBlockStatus_e` -> Source: ../../../src/main/drivers/sdcard/sdcard_spi.c +> Source: ../../../src/main/drivers/sdcard/sdcard_sdio.c | Enumerator | Value | Condition | |---|---:|---| @@ -5422,6 +5453,11 @@ | `HITL_EXTENDED_FLAGS` | (1 << 7) | | | `HITL_GPS_TIMEOUT` | (1 << 8) | | | `HITL_PITOT_FAILURE` | (1 << 9) | | +| `HITL_CURRENT_SENSOR` | (1 << 10) | | +| `HITL_SIM_RC_INPUT` | (1 << 11) | | +| `HITL_RANGEFINDER` | (1 << 12) | | +| `HITL_FAILSAFE_TRIGGERED` | (1 << 13) | | +| `HITL_SITL_MODE` | (1 << 14) | | --- ## `smartAudioVersion_e` @@ -5637,7 +5673,7 @@ --- ## `systemState_e` -> Source: ../../../src/main/fc/fc_init.c +> Source: ../../../src/main/fc/fc_init.h | Enumerator | Value | Condition | |---|---:|---| @@ -5651,7 +5687,7 @@ --- ## `systemState_e` -> Source: ../../../src/main/fc/fc_init.h +> Source: ../../../src/main/fc/fc_init.c | Enumerator | Value | Condition | |---|---:|---| @@ -5672,6 +5708,7 @@ | `TCH_DMA_IDLE` | 0 | | | `TCH_DMA_READY` | 1 | | | `TCH_DMA_ACTIVE` | 2 | | +| `TCH_DMA_CIRCULAR` | 3 | | --- ## `tempSensorType_e` From 27e6322c2cb5896fc38ac828e94ee0b62c81134d Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 16 Aug 2026 13:00:23 -0500 Subject: [PATCH 13/38] docs: seed RAM/flash optimization guide with PR #11718 technique Stub only -- captures the buffer-decoupling and shrink-audit technique from the MSP tunnel reply buffer fix before it's lost, ahead of the tracked document-ram-flash-optimization-practices project writing the full guide and linking it from Development.md. --- .../development/ram-and-flash-optimization.md | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 docs/development/ram-and-flash-optimization.md diff --git a/docs/development/ram-and-flash-optimization.md b/docs/development/ram-and-flash-optimization.md new file mode 100644 index 00000000000..f13eae8bef3 --- /dev/null +++ b/docs/development/ram-and-flash-optimization.md @@ -0,0 +1,72 @@ +# RAM and Flash Optimization + +> **Status: seed draft.** This is a stub started while fixing a RAM overflow +> on PR #11718, so the technique wouldn't get lost before the full guide is +> written. It is not yet linked from `Development.md` and is not a complete +> guide — see the tracked project for the rest of this doc's scope. + +Static RAM headroom is a binding constraint on several boards (128KB-RAM +F4 targets in particular). Treat RAM and flash reduction as an active goal +when writing or reviewing code that adds static buffers, not just something +to check for overflow after the fact. + +## Don't size a buffer for a different subsystem's worst case + +If a buffer is reused across features, check what its size actually derives +from before assuming a new consumer needs the same size. On PR #11718 (the +MSP-over-MAVLink tunnel), a reply buffer was sized off `MSP_PORT_OUTBUF_SIZE` +(4112 bytes under `USE_FLASHFS`) simply because that was the constant already +in scope for MSP replies. But that size exists to let `MSP_DATAFLASH_READ` +return a full 4096-byte flash page in one shot for fast blackbox downloads +over USB/serial — the tunnel's own use case (MSP replies fragmented into +128-byte MAVLink `TUNNEL` payload chunks regardless of source buffer size) +never benefited from that size at all. Giving the tunnel its own, +independently-sized constant (512 bytes, matching the size every +non-FLASHFS board already uses for ordinary MSP replies) cut ~3.6KB with no +functional loss — data that used to arrive in one large read now arrives in +more, smaller reads, which costs nothing extra when it's already being +fragmented for the wire. + +**Before reusing an existing size constant for a new buffer, ask what +specifically drove that number** — it may be tied to an unrelated worst +case that doesn't apply to the new use. + +## When shrinking a shared buffer, audit what else writes into it + +A buffer's "extra" size can be silently load-bearing for a bug elsewhere, +not just wasted space. While sizing the tunnel buffer above, auditing every +MSP command reachable through it surfaced `serializeDataflashReadReply()` +computing its available-space clamp *before* writing a 4-byte address +header, then writing `address + clamped_data` — an unconditional 4-byte +overflow whenever a request is large enough to hit the clamp. On the +original 4112-byte buffer this was practically unreachable (16 bytes of +incidental headroom absorbed it for a standard 4096-byte request); shrinking +to 512 bytes with no equivalent headroom made it trivially reachable by any +normal-sized request. + +**When shrinking a buffer other code paths also write into, check whether +any of those paths were relying on the buffer being larger than their own +worst-case write** — not just whether the new size covers the new +consumer's own worst case. Fix the root cause (the clamp math, here) rather +than padding the buffer back up to paper over it. + +## Survey actual message/data sizes before assuming a buffer needs to be large + +"How big does this actually need to be" is often answerable by grep, not +guesswork — but match the audit's scope to everything that can actually +reach the buffer. A first pass on the tunnel buffer above checked only MSP2 +command handlers and found 432 bytes as the largest reply; widening to MSP1 +(legacy) handlers — reachable through the same buffer — found 512 bytes +exactly (`MSP_LED_STRIP_CONFIG`, no bounds check, no margin). Scoping an +audit to "the obviously relevant" category of code instead of everything +that can reach the buffer produces a confident-looking wrong answer. + +## Related + +- Full strategy pattern library (more entries, other PRs): + `claude/projects/active/ram-reduction-program/ram-flash-strategies.md` in + the `inav-claude` tooling repo (not part of this firmware repo). +- Tracked follow-up: `document-ram-flash-optimization-practices` project — + expands this into a full guide, links it from `Development.md`, and + updates review checklists to treat RAM/flash reduction as an active + review category. From 75aec881e7c327dc8e26ecbb601f5cb1f96de618 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 15:20:25 -0500 Subject: [PATCH 14/38] Add CI PR comment for RAM/flash usage delta vs. base branch Surfaces RAM/flash regressions and creep on every PR instead of only via a hard CI failure (motivated by PR #11718's terrain-cache RAM overflow, which was only caught because CI happened to fail on that specific board). Extracts flash/RAM usage from each build's .elf right after compiling (no second build of the base branch), persists a baseline per branch as a release asset in the pr-test-builds companion repo, and diffs 4 representative targets (one per MCU family) against it on PR builds. --- .github/scripts/extract-size-report.sh | 49 ++++ .github/scripts/merge-size-reports.sh | 19 ++ .github/scripts/size-diff-comment.js | 101 ++++++++ .github/scripts/size-diff-comment.test.js | 294 ++++++++++++++++++++++ .github/workflows/README.md | 27 ++ .github/workflows/ci-size-report.yml | 193 ++++++++++++++ .github/workflows/ci.yml | 49 ++++ 7 files changed, 732 insertions(+) create mode 100755 .github/scripts/extract-size-report.sh create mode 100755 .github/scripts/merge-size-reports.sh create mode 100644 .github/scripts/size-diff-comment.js create mode 100644 .github/scripts/size-diff-comment.test.js create mode 100644 .github/workflows/ci-size-report.yml diff --git a/.github/scripts/extract-size-report.sh b/.github/scripts/extract-size-report.sh new file mode 100755 index 00000000000..87f810154d1 --- /dev/null +++ b/.github/scripts/extract-size-report.sh @@ -0,0 +1,49 @@ +#!/bin/bash +# +# Run the size tool on every built .elf and write a small JSON report of +# flash/RAM usage per target, consumed by ci-size-report.yml. +# +# Usage: extract-size-report.sh [size-tool] +# +# flash = .text + .data (what's programmed into flash) +# ram = .data + .bss (what's reserved in RAM at runtime) + +set -euo pipefail + +BUILD_DIR=${1:?usage: extract-size-report.sh [size-tool]} +OUTPUT_JSON=${2:?usage: extract-size-report.sh [size-tool]} +SIZE_TOOL=${3:-arm-none-eabi-size} + +# CMake's RUNTIME_OUTPUT_DIRECTORY puts built executables under +# /bin/ (see cmake/main.cmake), not directly — search +# instead of assuming a fixed depth, in case that ever changes. +mapfile -t ELFS < <(find "$BUILD_DIR" -maxdepth 3 -name '*.elf' | sort) +if [ "${#ELFS[@]}" -eq 0 ]; then + echo "::warning::No .elf files found under $BUILD_DIR, writing empty size report" + echo '{}' > "$OUTPUT_JSON" + exit 0 +fi + +ENTRIES=() +for elf in "${ELFS[@]}"; do + target=$(basename "$elf" .elf) + + # Berkeley format: " text data bss dec hex filename" + read -r text data bss _dec _hex _name < <("$SIZE_TOOL" -B "$elf" | tail -n1) + + flash=$((text + data)) + ram=$((data + bss)) + + ENTRIES+=("\"$target\":{\"flash\":$flash,\"ram\":$ram}") +done + +{ + printf '{' + printf '%s' "${ENTRIES[0]}" + for entry in "${ENTRIES[@]:1}"; do + printf ',%s' "$entry" + done + printf '}' +} > "$OUTPUT_JSON" + +echo "Wrote size report for ${#ENTRIES[@]} target(s) to $OUTPUT_JSON" diff --git a/.github/scripts/merge-size-reports.sh b/.github/scripts/merge-size-reports.sh new file mode 100755 index 00000000000..fcb79457bc9 --- /dev/null +++ b/.github/scripts/merge-size-reports.sh @@ -0,0 +1,19 @@ +#!/bin/bash +# +# Merge multiple per-shard size-report.json files (see extract-size-report.sh) +# into one aggregate JSON, used by the "upload-artifacts" job in ci.yml. +# +# Usage: merge-size-reports.sh ... + +set -euo pipefail + +OUTPUT_JSON=${1:?usage: merge-size-reports.sh ...} +shift + +if [ "$#" -eq 0 ]; then + echo '{}' > "$OUTPUT_JSON" + exit 0 +fi + +jq -s 'add' "$@" > "$OUTPUT_JSON" +echo "Merged $# size report(s) into $OUTPUT_JSON" diff --git a/.github/scripts/size-diff-comment.js b/.github/scripts/size-diff-comment.js new file mode 100644 index 00000000000..0d66311cf13 --- /dev/null +++ b/.github/scripts/size-diff-comment.js @@ -0,0 +1,101 @@ +// Pure logic for the "PR RAM/Flash usage delta" comment: diffs a PR's +// size-report.json against a base-branch baseline and renders the markdown +// comment body. Deliberately has no network/filesystem/GitHub Actions +// dependency so it can be unit tested directly and reused (via require) +// from the actions/github-script step in ci-size-report.yml. +// +// Report shape: { "": { "flash": , "ram": }, ... } + +'use strict'; + +const REPRESENTATIVE_TARGETS = ['MATEKF405', 'MATEKF722', 'MATEKF765', 'MATEKH743']; + +// Below this magnitude a delta is noise (rounding/toolchain jitter), not a +// real change worth calling out. +const NOISE_THRESHOLD_BYTES = 32; + +function formatDelta(deltaBytes, baseBytes) { + const sign = deltaBytes > 0 ? '+' : deltaBytes < 0 ? '' : '±'; + const pct = baseBytes > 0 ? (deltaBytes / baseBytes) * 100 : 0; + const pctStr = baseBytes > 0 ? ` (${sign}${pct.toFixed(2)}%)` : ''; + return `${sign}${deltaBytes} B${pctStr}`; +} + +// Returns an array of row objects, one per representative target, each +// either a comparison row or a status row (missing from PR/baseline). +function diffSizeReports(prReport, baselineReport) { + return REPRESENTATIVE_TARGETS.map((target) => { + const pr = prReport[target]; + const base = baselineReport ? baselineReport[target] : undefined; + + if (!pr && !base) { + return { target, status: 'not-built' }; + } + if (!pr) { + return { target, status: 'missing-from-pr' }; + } + if (!base) { + return { target, status: 'no-baseline', flash: pr.flash, ram: pr.ram }; + } + + const flashDelta = pr.flash - base.flash; + const ramDelta = pr.ram - base.ram; + return { + target, + status: 'compared', + flash: pr.flash, + ram: pr.ram, + flashDelta, + ramDelta, + notable: Math.abs(flashDelta) >= NOISE_THRESHOLD_BYTES || Math.abs(ramDelta) >= NOISE_THRESHOLD_BYTES, + }; + }); +} + +// docLink: string URL to link, or null/undefined to omit the doc-link line. +function renderComment({ prReport, baselineReport, shortSha, docLink, marker }) { + const rows = diffSizeReports(prReport, baselineReport); + const lines = [marker, '**RAM / Flash usage vs. base branch** — commit `' + shortSha + '`', '']; + + if (!baselineReport) { + lines.push( + '> No size baseline is available yet for this PR\'s base branch ' + + '(first run after this feature shipped, or a new branch). ' + + 'This comment will show deltas once a baseline exists.', + '' + ); + } + + const anyComparable = rows.some((r) => r.status === 'compared' || r.status === 'no-baseline'); + if (anyComparable) { + lines.push('| Target | Flash Δ | RAM Δ |', '|---|---|---|'); + for (const row of rows) { + if (row.status === 'compared') { + const flashCell = formatDelta(row.flashDelta, row.flash - row.flashDelta); + const ramCell = formatDelta(row.ramDelta, row.ram - row.ramDelta); + const notableMark = row.notable ? ' ⚠️' : ''; + lines.push(`| ${row.target}${notableMark} | ${flashCell} | ${ramCell} |`); + } else if (row.status === 'no-baseline') { + lines.push(`| ${row.target} | ${row.flash} B (no baseline) | ${row.ram} B (no baseline) |`); + } else if (row.status === 'missing-from-pr') { + lines.push(`| ${row.target} | not built by this PR | not built by this PR |`); + } + // 'not-built': omit entirely, nothing meaningful to say + } + lines.push(''); + } else { + lines.push( + '_None of the representative targets (' + REPRESENTATIVE_TARGETS.join(', ') + ') ' + + 'were built by this PR — no size comparison to show._', + '' + ); + } + + if (docLink) { + lines.push(`See [RAM/flash optimization guide](${docLink}) for techniques to reduce usage.`, ''); + } + + return lines.join('\n').trimEnd() + '\n'; +} + +module.exports = { REPRESENTATIVE_TARGETS, NOISE_THRESHOLD_BYTES, diffSizeReports, renderComment, formatDelta }; diff --git a/.github/scripts/size-diff-comment.test.js b/.github/scripts/size-diff-comment.test.js new file mode 100644 index 00000000000..bf005ed012d --- /dev/null +++ b/.github/scripts/size-diff-comment.test.js @@ -0,0 +1,294 @@ +// Unit tests for size-diff-comment.js (RAM/Flash usage delta PR comment). +// +// Run with: node --test .github/scripts/size-diff-comment.test.js +// +// Pure-logic tests only — no filesystem/network access, matching the module +// under test. Uses Node's built-in test runner (node:test / node:assert), +// no dependencies. + +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { + REPRESENTATIVE_TARGETS, + NOISE_THRESHOLD_BYTES, + diffSizeReports, + renderComment, + formatDelta, +} = require('./size-diff-comment.js'); + +// Sanity on the fixed constants the rest of the tests assume. +test('REPRESENTATIVE_TARGETS is the expected 4 targets, NOISE_THRESHOLD_BYTES is 32', () => { + assert.deepEqual(REPRESENTATIVE_TARGETS, ['MATEKF405', 'MATEKF722', 'MATEKF765', 'MATEKH743']); + assert.equal(NOISE_THRESHOLD_BYTES, 32); +}); + +// --------------------------------------------------------------------------- +// formatDelta +// --------------------------------------------------------------------------- + +test('formatDelta: positive delta gets a + sign and correct percentage', () => { + const result = formatDelta(100, 10000); + assert.equal(result, '+100 B (+1.00%)'); +}); + +test('formatDelta: negative delta gets no extra sign (native minus) and correct percentage', () => { + const result = formatDelta(-100, 10000); + assert.equal(result, '-100 B (-1.00%)'); + // Make sure there's no double-minus like "--100" or "(-+1.00%)". + assert.ok(!result.includes('--')); +}); + +test('formatDelta: zero delta does not show a + sign', () => { + const result = formatDelta(0, 10000); + assert.ok(!result.includes('+'), `expected no "+" in zero-delta output, got: ${result}`); + assert.equal(result, '±0 B (±0.00%)'); +}); + +test('formatDelta: baseBytes of 0 omits the percentage entirely (no divide-by-zero artifact)', () => { + const result = formatDelta(50, 0); + assert.equal(result, '+50 B'); + assert.ok(!result.includes('NaN')); + assert.ok(!result.includes('Infinity')); +}); + +// --------------------------------------------------------------------------- +// diffSizeReports +// --------------------------------------------------------------------------- + +test('diffSizeReports: target present in both PR and baseline with a real delta -> compared, correct deltas', () => { + const pr = { MATEKF405: { flash: 500100, ram: 60000 } }; + const base = { MATEKF405: { flash: 500000, ram: 60200 } }; + const rows = diffSizeReports(pr, base); + const row = rows.find((r) => r.target === 'MATEKF405'); + + assert.equal(row.status, 'compared'); + assert.equal(row.flash, 500100); + assert.equal(row.ram, 60000); + assert.equal(row.flashDelta, 100); + assert.equal(row.ramDelta, -200); +}); + +test('diffSizeReports: notable is gated at exactly NOISE_THRESHOLD_BYTES (32 notable, 31 not)', () => { + const baseSizes = { flash: 100000, ram: 50000 }; + + const prAt32 = { MATEKF405: { flash: baseSizes.flash + 32, ram: baseSizes.ram } }; + const prAt31 = { MATEKF405: { flash: baseSizes.flash + 31, ram: baseSizes.ram } }; + const base = { MATEKF405: baseSizes }; + + const rowAt32 = diffSizeReports(prAt32, base).find((r) => r.target === 'MATEKF405'); + const rowAt31 = diffSizeReports(prAt31, base).find((r) => r.target === 'MATEKF405'); + + assert.equal(rowAt32.flashDelta, 32); + assert.equal(rowAt32.notable, true, 'delta of exactly 32 (the threshold) should be notable'); + + assert.equal(rowAt31.flashDelta, 31); + assert.equal(rowAt31.notable, false, 'delta of 31 (below the threshold) should not be notable'); +}); + +test('diffSizeReports: notable also triggers from ramDelta alone, and honors negative deltas via Math.abs', () => { + const base = { MATEKF405: { flash: 100000, ram: 50000 } }; + const pr = { MATEKF405: { flash: 100000, ram: 50000 - 40 } }; // flash unchanged, ram shrank by 40 + const row = diffSizeReports(pr, base).find((r) => r.target === 'MATEKF405'); + + assert.equal(row.flashDelta, 0); + assert.equal(row.ramDelta, -40); + assert.equal(row.notable, true, 'a -40 ram delta exceeds the 32-byte threshold in magnitude'); +}); + +test('diffSizeReports: target missing from PR report but present in baseline -> missing-from-pr', () => { + const pr = {}; // MATEKF405 not built this run + const base = { MATEKF405: { flash: 100000, ram: 50000 } }; + const row = diffSizeReports(pr, base).find((r) => r.target === 'MATEKF405'); + + assert.equal(row.status, 'missing-from-pr'); + assert.equal(row.flash, undefined); + assert.equal(row.ram, undefined); +}); + +test('diffSizeReports: target missing from both PR and baseline -> not-built', () => { + const pr = {}; + const base = {}; + const row = diffSizeReports(pr, base).find((r) => r.target === 'MATEKF405'); + + assert.equal(row.status, 'not-built'); +}); + +test('diffSizeReports: baselineReport undefined but target present in PR -> no-baseline', () => { + const pr = { MATEKF405: { flash: 100000, ram: 50000 } }; + const row = diffSizeReports(pr, undefined).find((r) => r.target === 'MATEKF405'); + + assert.equal(row.status, 'no-baseline'); + assert.equal(row.flash, 100000); + assert.equal(row.ram, 50000); +}); + +test('diffSizeReports: baselineReport null (not just undefined) behaves the same as undefined', () => { + const pr = { MATEKF405: { flash: 100000, ram: 50000 } }; + const row = diffSizeReports(pr, null).find((r) => r.target === 'MATEKF405'); + + assert.equal(row.status, 'no-baseline'); +}); + +test('diffSizeReports: returns exactly one row per representative target, in order', () => { + const rows = diffSizeReports({}, {}); + assert.equal(rows.length, REPRESENTATIVE_TARGETS.length); + assert.deepEqual(rows.map((r) => r.target), REPRESENTATIVE_TARGETS); +}); + +// --------------------------------------------------------------------------- +// renderComment +// --------------------------------------------------------------------------- + +function fullReport(sizes) { + // Helper: build a { target: {flash, ram} } report for all 4 representative + // targets from a flat { target: [flash, ram] } shorthand. + const report = {}; + for (const [target, [flash, ram]] of Object.entries(sizes)) { + report[target] = { flash, ram }; + } + return report; +} + +test('renderComment: baseline present, all 4 targets compared, mix of notable/non-notable', () => { + const marker = ''; + const baselineReport = fullReport({ + MATEKF405: [500000, 60000], + MATEKF722: [510000, 61000], + MATEKF765: [520000, 62000], + MATEKH743: [530000, 63000], + }); + const prReport = fullReport({ + MATEKF405: [500100, 60000], // +100 flash -> notable + MATEKF722: [510010, 61000], // +10 flash -> not notable + MATEKF765: [519960, 62000], // -40 flash -> notable + MATEKH743: [530000, 63000], // no change -> not notable + }); + + const body = renderComment({ prReport, baselineReport, shortSha: 'abc1234', docLink: null, marker }); + + // Marker present (and first line, as GitHub comment-identification markers + // are conventionally expected to be). + assert.ok(body.includes(marker)); + assert.equal(body.split('\n')[0], marker); + + // Markdown table present. + assert.ok(body.includes('| Target | Flash Δ | RAM Δ |')); + assert.ok(body.includes('|---|---|---|')); + + // Notable rows flagged with the warning emoji, non-notable rows are not. + const lines = body.split('\n'); + const matekf405Line = lines.find((l) => l.startsWith('| MATEKF405')); + const matekf722Line = lines.find((l) => l.startsWith('| MATEKF722')); + const matekf765Line = lines.find((l) => l.startsWith('| MATEKF765')); + const matekh743Line = lines.find((l) => l.startsWith('| MATEKH743')); + + assert.ok(matekf405Line.includes('⚠️'), 'MATEKF405 (+100 flash) should be flagged notable'); + assert.ok(!matekf722Line.includes('⚠️'), 'MATEKF722 (+10 flash) should NOT be flagged notable'); + assert.ok(matekf765Line.includes('⚠️'), 'MATEKF765 (-40 flash) should be flagged notable'); + assert.ok(!matekh743Line.includes('⚠️'), 'MATEKH743 (no change) should NOT be flagged notable'); + + // No "no baseline available" note when a baseline was supplied. + assert.ok(!body.includes('No size baseline is available yet')); +}); + +test('renderComment: baselineReport falsy -> includes the "no baseline available yet" note', () => { + const prReport = fullReport({ + MATEKF405: [500000, 60000], + MATEKF722: [510000, 61000], + MATEKF765: [520000, 62000], + MATEKH743: [530000, 63000], + }); + + const body = renderComment({ + prReport, + baselineReport: undefined, + shortSha: 'def5678', + docLink: null, + marker: '', + }); + + assert.ok(body.includes('No size baseline is available yet')); +}); + +test('renderComment: docLink set -> link line present', () => { + const prReport = fullReport({ MATEKF405: [500000, 60000] }); + const baselineReport = fullReport({ MATEKF405: [499000, 60000] }); + + const body = renderComment({ + prReport, + baselineReport, + shortSha: 'abc1234', + docLink: 'https://example.com/optimization-guide', + marker: '', + }); + + assert.ok(body.includes('[RAM/flash optimization guide](https://example.com/optimization-guide)')); +}); + +test('renderComment: docLink omitted -> link line absent entirely', () => { + const prReport = fullReport({ MATEKF405: [500000, 60000] }); + const baselineReport = fullReport({ MATEKF405: [499000, 60000] }); + + const body = renderComment({ + prReport, + baselineReport, + shortSha: 'abc1234', + docLink: null, + marker: '', + }); + + assert.ok(!body.includes('optimization guide')); + assert.ok(!body.includes('See [')); +}); + +test('renderComment: docLink undefined (key omitted from options) -> link line absent entirely', () => { + const prReport = fullReport({ MATEKF405: [500000, 60000] }); + const baselineReport = fullReport({ MATEKF405: [499000, 60000] }); + + const body = renderComment({ + prReport, + baselineReport, + shortSha: 'abc1234', + marker: '', + }); + + assert.ok(!body.includes('optimization guide')); +}); + +test('renderComment: none of the 4 representative targets built -> fallback message, not an empty/broken table', () => { + // Neither PR nor baseline built any representative target -> all rows 'not-built'. + const body = renderComment({ + prReport: {}, + baselineReport: {}, + shortSha: 'abc1234', + docLink: null, + marker: '', + }); + + assert.ok( + body.includes('None of the representative targets') && body.includes('were built by this PR'), + `expected fallback message, got:\n${body}` + ); + assert.ok( + REPRESENTATIVE_TARGETS.every((t) => body.includes(t)), + 'fallback message should still name all 4 representative targets' + ); + // No markdown table header should be emitted since there's nothing to show. + assert.ok(!body.includes('| Target | Flash Δ | RAM Δ |')); +}); + +test('renderComment: result always ends with exactly one trailing newline', () => { + const body = renderComment({ + prReport: fullReport({ MATEKF405: [500000, 60000] }), + baselineReport: fullReport({ MATEKF405: [499000, 60000] }), + shortSha: 'abc1234', + docLink: null, + marker: '', + }); + + assert.ok(body.endsWith('\n')); + assert.ok(!body.endsWith('\n\n')); +}); diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 967080a66fb..07205b1ecc8 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -48,6 +48,33 @@ This directory contains automated CI/CD workflows for the INAV project. ### Pull Request Helpers +#### `ci-size-report.yml` - RAM/Flash Usage Delta PR Comment +**Triggers:** `workflow_run` after "Build firmware" (`ci.yml`) completes +**Purpose:** Posts/updates a PR comment showing flash and RAM usage delta vs. +the PR's base branch, for 4 representative targets (one per MCU family: +MATEKF405, MATEKF722, MATEKF765, MATEKH743). Surfaces RAM/flash regressions +and creep before they become an overflow, on every PR. + +**How it works:** +1. `ci.yml` extracts a small per-target size report (`arm-none-eabi-size` + on each built `.elf`) right after each build and uploads it as an + artifact — no second build anywhere in this flow. +2. On pushes to a branch, `ci-size-report.yml` persists that size report as + a release asset (`size-baseline-`) in the companion + `iNavFlight/pr-test-builds` repo — the "known good" baseline for that + branch, overwritten on every push. +3. On PR builds, it fetches the PR's base branch's persisted baseline (no + rebuild), diffs it against the PR's own size report, and posts/updates a + comment (marker ``). + +**Script:** `.github/scripts/extract-size-report.sh` (size extraction), +`.github/scripts/merge-size-reports.sh` (merges per-shard reports), +`.github/scripts/size-diff-comment.js` (pure diff + markdown rendering, +unit tested in `.github/scripts/size-diff-comment.test.js`) + +**Uses the same `PR_BUILDS_TOKEN` secret and `workflow_run` trigger pattern +as `pr-test-builds.yml`** (secrets available even for fork PRs). + #### `pr-branch-suggestion.yml` - Branch Targeting Suggestion **Triggers:** PRs targeting master branch **Purpose:** Suggests using maintenance-9.x or maintenance-10.x instead diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml new file mode 100644 index 00000000000..e25cb092916 --- /dev/null +++ b/.github/workflows/ci-size-report.yml @@ -0,0 +1,193 @@ +name: CI Size Report + +# Runs after "Build firmware" completes. Uses workflow_run (rather than +# pull_request/push directly) so that secrets are available even for PRs +# from forks — same reasoning as pr-test-builds.yml. +# +# Two jobs: +# - publish-baseline: on branch pushes (maintenance-*), persists the size +# report as a release asset in the companion iNavFlight/pr-test-builds +# repo, so PR runs never need to rebuild the base branch to get a +# comparison point. +# - pr-comment: on PR builds, fetches that persisted baseline, diffs the +# 4 representative targets, and posts/updates a PR comment. +# +# Requires the same repository secret PR_BUILDS_TOKEN as pr-test-builds.yml +# (Contents: write access to iNavFlight/pr-test-builds). +on: + workflow_run: + workflows: ["Build firmware"] + types: [completed] + +jobs: + publish-baseline: + runs-on: ubuntu-latest + if: > + github.event.workflow_run.event == 'push' && + github.event.workflow_run.conclusion == 'success' + concurrency: + group: size-baseline-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + permissions: + actions: read # to download artifacts from the triggering workflow run + steps: + - name: Download size report + uses: actions/download-artifact@v4 + with: + name: size-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Download branch name + uses: actions/download-artifact@v4 + with: + name: branch-name + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Publish baseline + env: + GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} + run: | + BRANCH=$(cat branch.txt) + TAG="size-baseline-${BRANCH}" + gh release delete "$TAG" --repo iNavFlight/pr-test-builds --cleanup-tag --yes 2>/dev/null || true + gh release create "$TAG" size-report.json \ + --repo iNavFlight/pr-test-builds \ + --prerelease \ + --title "Size baseline: ${BRANCH}" \ + --notes "Latest per-target flash/RAM size report for ${BRANCH}. Auto-updated on every push. Not for human consumption." + + pr-comment: + runs-on: ubuntu-latest + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + concurrency: + group: pr-size-report-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + permissions: + actions: read + issues: write + pull-requests: write + contents: read + steps: + # Checks out this workflow's own ref (the default branch — workflow_run + # always runs the workflow file from the default branch), NOT the PR's + # head. We only need our own trusted .github/scripts/ here; the PR's + # untrusted code is never checked out in this privileged context. + - uses: actions/checkout@v4 + + - name: Download PR number + uses: actions/download-artifact@v4 + with: + name: pr-number + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Download base ref + uses: actions/download-artifact@v4 + with: + name: base-ref + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Download PR size report + uses: actions/download-artifact@v4 + with: + name: size-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Read PR number and base ref + id: pr + run: | + PR_NUM=$(tr -dc '0-9' < pr_number.txt) + if [ -z "$PR_NUM" ]; then + echo "::error::Invalid PR number in artifact" + exit 1 + fi + echo "number=${PR_NUM}" >> $GITHUB_OUTPUT + echo "base_ref=$(cat base_ref.txt)" >> $GITHUB_OUTPUT + echo "short_sha=$(echo '${{ github.event.workflow_run.head_sha }}' | cut -c1-7)" >> $GITHUB_OUTPUT + + - name: Fetch base branch baseline + id: baseline + env: + GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} + run: | + TAG="size-baseline-${{ steps.pr.outputs.base_ref }}" + mkdir -p baseline + if gh release download "$TAG" --repo iNavFlight/pr-test-builds --pattern size-report.json --dir baseline 2>/dev/null; then + echo "found=true" >> $GITHUB_OUTPUT + else + echo "found=false" >> $GITHUB_OUTPUT + fi + + - name: Check for doc on PR head commit + id: doc + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + HEAD_SHA="${{ github.event.workflow_run.head_sha }}" + HEAD_REPO="${{ github.event.workflow_run.head_repository.full_name }}" + if gh api "repos/${HEAD_REPO}/contents/docs/development/ram-and-flash-optimization.md?ref=${HEAD_SHA}" >/dev/null 2>&1; then + echo "link=https://github.com/${HEAD_REPO}/blob/${HEAD_SHA}/docs/development/ram-and-flash-optimization.md" >> $GITHUB_OUTPUT + else + echo "link=" >> $GITHUB_OUTPUT + fi + + - name: Post or update PR comment + uses: actions/github-script@v7 + env: + PR_NUMBER: ${{ steps.pr.outputs.number }} + SHORT_SHA: ${{ steps.pr.outputs.short_sha }} + BASELINE_FOUND: ${{ steps.baseline.outputs.found }} + DOC_LINK: ${{ steps.doc.outputs.link }} + with: + script: | + const fs = require('fs'); + const path = require('path'); + const { renderComment } = require( + path.join(process.env.GITHUB_WORKSPACE, '.github', 'scripts', 'size-diff-comment.js') + ); + + const prNumber = parseInt(process.env.PR_NUMBER, 10); + if (isNaN(prNumber)) throw new Error(`Invalid PR number: ${process.env.PR_NUMBER}`); + + const prReport = JSON.parse(fs.readFileSync('size-report.json', 'utf8')); + const baselineReport = process.env.BASELINE_FOUND === 'true' + ? JSON.parse(fs.readFileSync('baseline/size-report.json', 'utf8')) + : null; + + const marker = ''; + const body = renderComment({ + prReport, + baselineReport, + shortSha: process.env.SHORT_SHA, + docLink: process.env.DOC_LINK || null, + marker, + }); + + const comments = await github.paginate( + github.rest.issues.listComments, + { owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber } + ); + + const existing = comments.find(c => c.user.type === 'Bot' && c.body.includes(marker)); + + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body, + }); + } diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb48b235904..31064c8084a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,12 +121,20 @@ jobs: key: ${{ runner.os }}-downloads-${{ hashFiles('CMakeLists.txt') }}-${{ hashFiles('**/cmake/*')}} - name: Build targets (${{ matrix.id }}) run: mkdir -p build && cd build && cmake -DWARNINGS_AS_ERRORS=ON -DCI_JOB_INDEX=${{ matrix.id }} -DCI_JOB_COUNT=${{ strategy.job-total }} -DBUILD_SUFFIX=${{ env.BUILD_SUFFIX }} -DMAIN_COMPILE_OPTIONS=-pipe -G Ninja .. && ninja -j${{ env.NUM_CORES }} ci + - name: Extract size report + run: .github/scripts/extract-size-report.sh build build/size-report.json - name: Upload artifacts uses: actions/upload-artifact@v4 with: name: matrix-${{ env.BUILD_NAME }}.${{ matrix.id }} path: ./build/*.hex retention-days: 1 + - name: Upload size report + uses: actions/upload-artifact@v4 + with: + name: size-report-${{ matrix.id }} + path: ./build/size-report.json + retention-days: 1 build-single-target: needs: [detect] @@ -153,12 +161,20 @@ jobs: key: ${{ runner.os }}-downloads-${{ hashFiles('CMakeLists.txt') }}-${{ hashFiles('**/cmake/*')}} - name: Build targets (${{ needs.detect.outputs.target_names }}) run: mkdir -p build && cd build && cmake -DWARNINGS_AS_ERRORS=ON -DBUILD_SUFFIX=${{ env.BUILD_SUFFIX }} -DMAIN_COMPILE_OPTIONS=-pipe -G Ninja .. && ninja -j${{ env.NUM_CORES }} ${{ needs.detect.outputs.target_names }} + - name: Extract size report + run: .github/scripts/extract-size-report.sh build build/size-report.json - name: Upload artifacts uses: actions/upload-artifact@v4 with: name: matrix-${{ env.BUILD_NAME }}.single path: ./build/*.hex retention-days: 1 + - name: Upload size report + uses: actions/upload-artifact@v4 + with: + name: size-report-single + path: ./build/size-report.json + retention-days: 1 upload-artifacts: runs-on: ubuntu-latest @@ -212,6 +228,39 @@ jobs: name: pr-number path: pr_number.txt retention-days: 1 + - name: Save base ref + if: github.event_name == 'pull_request' + run: echo "${{ github.base_ref }}" > base_ref.txt + - name: Upload base ref + if: github.event_name == 'pull_request' + uses: actions/upload-artifact@v4 + with: + name: base-ref + path: base_ref.txt + retention-days: 1 + - name: Save branch name + if: github.event_name == 'push' + run: echo "${{ github.ref_name }}" > branch.txt + - name: Upload branch name + if: github.event_name == 'push' + uses: actions/upload-artifact@v4 + with: + name: branch-name + path: branch.txt + retention-days: 1 + - name: Download size reports + uses: actions/download-artifact@v4 + with: + pattern: size-report-* + path: size-reports + - name: Merge size reports + run: .github/scripts/merge-size-reports.sh size-report.json size-reports/*/size-report.json + - name: Upload merged size report + uses: actions/upload-artifact@v4 + with: + name: size-report + path: size-report.json + retention-days: 1 build-SITL-Linux-arm64: runs-on: ubuntu-22.04-arm From dc1dcb8b1b68f5913d36ee3b45795e127db78360 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 15:24:31 -0500 Subject: [PATCH 15/38] Fix GitHub Actions script-injection risk in size-report workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several steps spliced \${{ }} expressions directly into run: script text instead of passing them through env: — including one value (base_ref) that round-trips through an artifact from a less-trusted PR build before being used to construct a release tag. Route all of them through env: and validate base_ref against safe git-ref characters before use. --- .github/workflows/ci-size-report.yml | 34 ++++++++++++++++++++-------- .github/workflows/ci.yml | 8 +++++-- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index e25cb092916..380fd4acd8b 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -101,40 +101,54 @@ jobs: - name: Read PR number and base ref id: pr + env: + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} run: | PR_NUM=$(tr -dc '0-9' < pr_number.txt) if [ -z "$PR_NUM" ]; then echo "::error::Invalid PR number in artifact" exit 1 fi - echo "number=${PR_NUM}" >> $GITHUB_OUTPUT - echo "base_ref=$(cat base_ref.txt)" >> $GITHUB_OUTPUT - echo "short_sha=$(echo '${{ github.event.workflow_run.head_sha }}' | cut -c1-7)" >> $GITHUB_OUTPUT + + # base_ref.txt round-trips through an artifact produced by the + # (less-trusted) PR build, so validate it against safe git-ref + # characters before it's ever used to build a shell command or + # release tag downstream — never trust artifact content blindly. + BASE_REF=$(head -n1 base_ref.txt | tr -d '\r\n') + if ! [[ "$BASE_REF" =~ ^[A-Za-z0-9._/-]{1,100}$ ]]; then + echo "::error::Invalid base ref in artifact: $BASE_REF" + exit 1 + fi + + echo "number=${PR_NUM}" >> "$GITHUB_OUTPUT" + echo "base_ref=${BASE_REF}" >> "$GITHUB_OUTPUT" + echo "short_sha=${HEAD_SHA:0:7}" >> "$GITHUB_OUTPUT" - name: Fetch base branch baseline id: baseline env: GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} + BASE_REF: ${{ steps.pr.outputs.base_ref }} run: | - TAG="size-baseline-${{ steps.pr.outputs.base_ref }}" + TAG="size-baseline-${BASE_REF}" mkdir -p baseline if gh release download "$TAG" --repo iNavFlight/pr-test-builds --pattern size-report.json --dir baseline 2>/dev/null; then - echo "found=true" >> $GITHUB_OUTPUT + echo "found=true" >> "$GITHUB_OUTPUT" else - echo "found=false" >> $GITHUB_OUTPUT + echo "found=false" >> "$GITHUB_OUTPUT" fi - name: Check for doc on PR head commit id: doc env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }} run: | - HEAD_SHA="${{ github.event.workflow_run.head_sha }}" - HEAD_REPO="${{ github.event.workflow_run.head_repository.full_name }}" if gh api "repos/${HEAD_REPO}/contents/docs/development/ram-and-flash-optimization.md?ref=${HEAD_SHA}" >/dev/null 2>&1; then - echo "link=https://github.com/${HEAD_REPO}/blob/${HEAD_SHA}/docs/development/ram-and-flash-optimization.md" >> $GITHUB_OUTPUT + echo "link=https://github.com/${HEAD_REPO}/blob/${HEAD_SHA}/docs/development/ram-and-flash-optimization.md" >> "$GITHUB_OUTPUT" else - echo "link=" >> $GITHUB_OUTPUT + echo "link=" >> "$GITHUB_OUTPUT" fi - name: Post or update PR comment diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31064c8084a..615a55b9e7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -230,7 +230,9 @@ jobs: retention-days: 1 - name: Save base ref if: github.event_name == 'pull_request' - run: echo "${{ github.base_ref }}" > base_ref.txt + env: + BASE_REF: ${{ github.base_ref }} + run: echo "$BASE_REF" > base_ref.txt - name: Upload base ref if: github.event_name == 'pull_request' uses: actions/upload-artifact@v4 @@ -240,7 +242,9 @@ jobs: retention-days: 1 - name: Save branch name if: github.event_name == 'push' - run: echo "${{ github.ref_name }}" > branch.txt + env: + REF_NAME: ${{ github.ref_name }} + run: echo "$REF_NAME" > branch.txt - name: Upload branch name if: github.event_name == 'push' uses: actions/upload-artifact@v4 From bb4fa3f4663d07045afc5d044e6676efd776c09a Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 15:51:59 -0500 Subject: [PATCH 16/38] Address code review: target-coverage wording, JSON escaping, test hygiene - Clarify that the 4 representative targets are size tiers, not one-per- silicon-family (two are STM32F7, none are AT32); flag the AT32 gap in code comments/README rather than silently changing the manager-approved target list. - Build size-report.json via jq instead of manual string concatenation, so target names are JSON-escaped rather than assumed safe. - Carry baseFlash/baseRam through diffSizeReports instead of reconstructing them by subtracting the delta back out in renderComment. - Drop a test that only re-asserted a literal copy of a source constant (no behavior exercised); derive the threshold-boundary test from NOISE_THRESHOLD_BYTES instead of hardcoding 32/31. - Note the accepted tradeoff that this (non-gating) script runs on the PR's own checkout, so its numbers are informational only. --- .github/scripts/extract-size-report.sh | 24 ++++++++++++----------- .github/scripts/size-diff-comment.js | 11 +++++++++-- .github/scripts/size-diff-comment.test.js | 24 +++++++++-------------- .github/workflows/README.md | 8 +++++--- .github/workflows/ci-size-report.yml | 11 +++++++---- 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/.github/scripts/extract-size-report.sh b/.github/scripts/extract-size-report.sh index 87f810154d1..0cb9a5ea6c7 100755 --- a/.github/scripts/extract-size-report.sh +++ b/.github/scripts/extract-size-report.sh @@ -7,6 +7,11 @@ # # flash = .text + .data (what's programmed into flash) # ram = .data + .bss (what's reserved in RAM at runtime) +# +# Runs inside the (unprivileged) build job on the PR's own checkout, so a +# PR could in principle modify this script to misreport its own numbers. +# Accepted tradeoff: this feature is informational/non-gating, and a real +# overflow still fails the link step regardless of what this script says. set -euo pipefail @@ -24,7 +29,7 @@ if [ "${#ELFS[@]}" -eq 0 ]; then exit 0 fi -ENTRIES=() +JQ_ARGS=() for elf in "${ELFS[@]}"; do target=$(basename "$elf" .elf) @@ -34,16 +39,13 @@ for elf in "${ELFS[@]}"; do flash=$((text + data)) ram=$((data + bss)) - ENTRIES+=("\"$target\":{\"flash\":$flash,\"ram\":$ram}") + JQ_ARGS+=(--argjson "entry_${#JQ_ARGS[@]}" "{\"target\":\"${target}\",\"flash\":${flash},\"ram\":${ram}}") done -{ - printf '{' - printf '%s' "${ENTRIES[0]}" - for entry in "${ENTRIES[@]:1}"; do - printf ',%s' "$entry" - done - printf '}' -} > "$OUTPUT_JSON" +# Build via jq rather than manual string concatenation, so the target name +# (an .elf basename, not otherwise validated) is JSON-escaped properly +# instead of relying on it never containing a special character. +jq -n "${JQ_ARGS[@]}" 'reduce $ARGS.named[] as $e ({}; .[$e.target] = {flash: $e.flash, ram: $e.ram})' \ + > "$OUTPUT_JSON" -echo "Wrote size report for ${#ENTRIES[@]} target(s) to $OUTPUT_JSON" +echo "Wrote size report for ${#ELFS[@]} target(s) to $OUTPUT_JSON" diff --git a/.github/scripts/size-diff-comment.js b/.github/scripts/size-diff-comment.js index 0d66311cf13..0ec79554ecf 100644 --- a/.github/scripts/size-diff-comment.js +++ b/.github/scripts/size-diff-comment.js @@ -8,6 +8,11 @@ 'use strict'; +// 4 representative targets spanning flash/RAM size tiers (manager-approved +// set, 2026-08-17). Note MATEKF722 and MATEKF765 are both STM32F7 parts — +// "one per family" in the loose sense of distinct flash/RAM budgets, not +// one per silicon line. No AT32 target is covered; flagged back to the +// manager as a possible coverage gap, not decided unilaterally here. const REPRESENTATIVE_TARGETS = ['MATEKF405', 'MATEKF722', 'MATEKF765', 'MATEKH743']; // Below this magnitude a delta is noise (rounding/toolchain jitter), not a @@ -45,6 +50,8 @@ function diffSizeReports(prReport, baselineReport) { status: 'compared', flash: pr.flash, ram: pr.ram, + baseFlash: base.flash, + baseRam: base.ram, flashDelta, ramDelta, notable: Math.abs(flashDelta) >= NOISE_THRESHOLD_BYTES || Math.abs(ramDelta) >= NOISE_THRESHOLD_BYTES, @@ -71,8 +78,8 @@ function renderComment({ prReport, baselineReport, shortSha, docLink, marker }) lines.push('| Target | Flash Δ | RAM Δ |', '|---|---|---|'); for (const row of rows) { if (row.status === 'compared') { - const flashCell = formatDelta(row.flashDelta, row.flash - row.flashDelta); - const ramCell = formatDelta(row.ramDelta, row.ram - row.ramDelta); + const flashCell = formatDelta(row.flashDelta, row.baseFlash); + const ramCell = formatDelta(row.ramDelta, row.baseRam); const notableMark = row.notable ? ' ⚠️' : ''; lines.push(`| ${row.target}${notableMark} | ${flashCell} | ${ramCell} |`); } else if (row.status === 'no-baseline') { diff --git a/.github/scripts/size-diff-comment.test.js b/.github/scripts/size-diff-comment.test.js index bf005ed012d..d235e0132d6 100644 --- a/.github/scripts/size-diff-comment.test.js +++ b/.github/scripts/size-diff-comment.test.js @@ -19,12 +19,6 @@ const { formatDelta, } = require('./size-diff-comment.js'); -// Sanity on the fixed constants the rest of the tests assume. -test('REPRESENTATIVE_TARGETS is the expected 4 targets, NOISE_THRESHOLD_BYTES is 32', () => { - assert.deepEqual(REPRESENTATIVE_TARGETS, ['MATEKF405', 'MATEKF722', 'MATEKF765', 'MATEKH743']); - assert.equal(NOISE_THRESHOLD_BYTES, 32); -}); - // --------------------------------------------------------------------------- // formatDelta // --------------------------------------------------------------------------- @@ -71,21 +65,21 @@ test('diffSizeReports: target present in both PR and baseline with a real delta assert.equal(row.ramDelta, -200); }); -test('diffSizeReports: notable is gated at exactly NOISE_THRESHOLD_BYTES (32 notable, 31 not)', () => { +test('diffSizeReports: notable is gated at exactly NOISE_THRESHOLD_BYTES', () => { const baseSizes = { flash: 100000, ram: 50000 }; - const prAt32 = { MATEKF405: { flash: baseSizes.flash + 32, ram: baseSizes.ram } }; - const prAt31 = { MATEKF405: { flash: baseSizes.flash + 31, ram: baseSizes.ram } }; + const atThreshold = { MATEKF405: { flash: baseSizes.flash + NOISE_THRESHOLD_BYTES, ram: baseSizes.ram } }; + const belowThreshold = { MATEKF405: { flash: baseSizes.flash + NOISE_THRESHOLD_BYTES - 1, ram: baseSizes.ram } }; const base = { MATEKF405: baseSizes }; - const rowAt32 = diffSizeReports(prAt32, base).find((r) => r.target === 'MATEKF405'); - const rowAt31 = diffSizeReports(prAt31, base).find((r) => r.target === 'MATEKF405'); + const rowAtThreshold = diffSizeReports(atThreshold, base).find((r) => r.target === 'MATEKF405'); + const rowBelowThreshold = diffSizeReports(belowThreshold, base).find((r) => r.target === 'MATEKF405'); - assert.equal(rowAt32.flashDelta, 32); - assert.equal(rowAt32.notable, true, 'delta of exactly 32 (the threshold) should be notable'); + assert.equal(rowAtThreshold.flashDelta, NOISE_THRESHOLD_BYTES); + assert.equal(rowAtThreshold.notable, true, 'a delta of exactly NOISE_THRESHOLD_BYTES should be notable'); - assert.equal(rowAt31.flashDelta, 31); - assert.equal(rowAt31.notable, false, 'delta of 31 (below the threshold) should not be notable'); + assert.equal(rowBelowThreshold.flashDelta, NOISE_THRESHOLD_BYTES - 1); + assert.equal(rowBelowThreshold.notable, false, 'a delta one byte below NOISE_THRESHOLD_BYTES should not be notable'); }); test('diffSizeReports: notable also triggers from ramDelta alone, and honors negative deltas via Math.abs', () => { diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 07205b1ecc8..c4cd07979e0 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -51,9 +51,11 @@ This directory contains automated CI/CD workflows for the INAV project. #### `ci-size-report.yml` - RAM/Flash Usage Delta PR Comment **Triggers:** `workflow_run` after "Build firmware" (`ci.yml`) completes **Purpose:** Posts/updates a PR comment showing flash and RAM usage delta vs. -the PR's base branch, for 4 representative targets (one per MCU family: -MATEKF405, MATEKF722, MATEKF765, MATEKH743). Surfaces RAM/flash regressions -and creep before they become an overflow, on every PR. +the PR's base branch, for 4 representative targets spanning flash/RAM size +tiers (MATEKF405, MATEKF722, MATEKF765, MATEKH743 — note MATEKF722 and +MATEKF765 are both STM32F7 parts; no AT32 target is currently covered). +Surfaces RAM/flash regressions and creep before they become an overflow, on +every PR. **How it works:** 1. `ci.yml` extracts a small per-target size report (`arm-none-eabi-size` diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index 380fd4acd8b..fdf25724268 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -5,10 +5,13 @@ name: CI Size Report # from forks — same reasoning as pr-test-builds.yml. # # Two jobs: -# - publish-baseline: on branch pushes (maintenance-*), persists the size -# report as a release asset in the companion iNavFlight/pr-test-builds -# repo, so PR runs never need to rebuild the base branch to get a -# comparison point. +# - publish-baseline: on any branch push that triggers ci.yml (in practice, +# almost always maintenance-9.x/maintenance-10.x — see ci.yml's own push +# trigger for the exact filter), persists the size report as a release +# asset in the companion iNavFlight/pr-test-builds repo, so PR runs never +# need to rebuild the base branch to get a comparison point. Stale +# baselines for since-deleted branches aren't cleaned up automatically +# (same known limitation pr-test-builds has for old PR releases). # - pr-comment: on PR builds, fetches that persisted baseline, diffs the # 4 representative targets, and posts/updates a PR comment. # From b09680f88959ba6ea4b01fefb354d718f9eb7d35 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 16:04:10 -0500 Subject: [PATCH 17/38] Fix baseline-publish race flagged by Qodo review on PR #11790 Delete-then-recreate left a window where the release didn't exist at all, which a concurrent PR's baseline-fetch could hit and misreport as "no baseline available yet" for a baseline that actually existed. Create the release once, then only ever replace the asset in place on later pushes. Also add a short retry on the fetch side, since an individual asset replace still involves a brief delete-then-upload under the hood. --- .github/workflows/ci-size-report.yml | 42 ++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index fdf25724268..403461ac9be 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -54,12 +54,21 @@ jobs: run: | BRANCH=$(cat branch.txt) TAG="size-baseline-${BRANCH}" - gh release delete "$TAG" --repo iNavFlight/pr-test-builds --cleanup-tag --yes 2>/dev/null || true - gh release create "$TAG" size-report.json \ - --repo iNavFlight/pr-test-builds \ - --prerelease \ - --title "Size baseline: ${BRANCH}" \ - --notes "Latest per-target flash/RAM size report for ${BRANCH}. Auto-updated on every push. Not for human consumption." + # Never delete+recreate the release: that leaves a window where + # the release doesn't exist at all, which a concurrent PR's + # "Fetch base branch baseline" step could hit and misreport as + # "no baseline available yet". Create it once, then only ever + # replace the asset in place (--clobber) on later pushes — the + # release/tag itself stays continuously resolvable. + if gh release view "$TAG" --repo iNavFlight/pr-test-builds >/dev/null 2>&1; then + gh release upload "$TAG" size-report.json --repo iNavFlight/pr-test-builds --clobber + else + gh release create "$TAG" size-report.json \ + --repo iNavFlight/pr-test-builds \ + --prerelease \ + --title "Size baseline: ${BRANCH}" \ + --notes "Latest per-target flash/RAM size report for ${BRANCH}. Auto-updated on every push. Not for human consumption." + fi pr-comment: runs-on: ubuntu-latest @@ -135,11 +144,22 @@ jobs: run: | TAG="size-baseline-${BASE_REF}" mkdir -p baseline - if gh release download "$TAG" --repo iNavFlight/pr-test-builds --pattern size-report.json --dir baseline 2>/dev/null; then - echo "found=true" >> "$GITHUB_OUTPUT" - else - echo "found=false" >> "$GITHUB_OUTPUT" - fi + + # publish-baseline replaces the asset in place (--clobber) rather + # than deleting/recreating the release, but an individual asset + # replace still briefly deletes-then-uploads under the hood. A + # few short retries absorb that narrow window instead of a + # concurrent run misreporting "no baseline available yet" for a + # baseline that actually exists. + FOUND=false + for attempt in 1 2 3; do + if gh release download "$TAG" --repo iNavFlight/pr-test-builds --pattern size-report.json --dir baseline 2>/dev/null; then + FOUND=true + break + fi + sleep 3 + done + echo "found=${FOUND}" >> "$GITHUB_OUTPUT" - name: Check for doc on PR head commit id: doc From 4029d9848335e56426d97671b45479f155ff2d66 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 18:08:56 -0500 Subject: [PATCH 18/38] Fix arm-none-eabi-size not found on real CI runners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmake/arm-none-eabi-checks.cmake downloads its own toolchain into tools/ and adds it to PATH, but only inside that cmake process via set(ENV{PATH} ...) — that doesn't persist to a later CI step's shell. extract-size-report.sh defaulted to bare `arm-none-eabi-size` on PATH, which happened to work in local dev sandboxes with a system copy installed but fails on actual GitHub Actions runners, which have only the project's downloaded toolchain. Fall back to the same tools/ location CMake uses when nothing is found on PATH. --- .github/scripts/extract-size-report.sh | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/scripts/extract-size-report.sh b/.github/scripts/extract-size-report.sh index 0cb9a5ea6c7..24170ac952b 100755 --- a/.github/scripts/extract-size-report.sh +++ b/.github/scripts/extract-size-report.sh @@ -17,7 +17,24 @@ set -euo pipefail BUILD_DIR=${1:?usage: extract-size-report.sh [size-tool]} OUTPUT_JSON=${2:?usage: extract-size-report.sh [size-tool]} -SIZE_TOOL=${3:-arm-none-eabi-size} +SIZE_TOOL=${3:-} + +if [ -z "$SIZE_TOOL" ]; then + if command -v arm-none-eabi-size >/dev/null 2>&1; then + SIZE_TOOL=arm-none-eabi-size + else + # cmake/arm-none-eabi-checks.cmake downloads its own toolchain into + # tools/ and adds it to PATH — but only inside that cmake process + # via set(ENV{PATH} ...), which doesn't persist to a later CI step's + # shell. Fall back to the same location CMake would have used, + # relative to the repo root (this script must be run from there). + SIZE_TOOL=$(compgen -G 'tools/arm-gnu-toolchain-*/bin/arm-none-eabi-size' 2>/dev/null | head -n1 || true) + if [ -z "$SIZE_TOOL" ]; then + echo "::error::arm-none-eabi-size not found on PATH or under tools/arm-gnu-toolchain-*/bin/" >&2 + exit 1 + fi + fi +fi # CMake's RUNTIME_OUTPUT_DIRECTORY puts built executables under # /bin/ (see cmake/main.cmake), not directly — search From af9a4b5e64284e1653977a2dfb0588c01c173f8b Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Mon, 17 Aug 2026 22:06:08 -0500 Subject: [PATCH 19/38] Point baseline publishing at Build pre-release, not the dead ci.yml push trigger ci.yml's own push trigger is a no-op (a branches: list with only a negative pattern matches nothing per GitHub's docs; confirmed zero push-triggered runs exist in this repo's history). nightly-build.yml ("Build pre-release") is push-triggered correctly and already invokes ci.yml's jobs via workflow_call to build nightly releases -- listen to it instead, at no extra build cost. Add maintenance-10.x and release/9.1 to its branch list so baselines exist for those bases too. Also fix ci.yml's "Save branch name" step: it was gated on github.event_name == 'push', but that reports as 'workflow_call' when invoked from nightly-build.yml, not 'push' -- so branch.txt would never have been produced on the path this now depends on. --- .github/workflows/README.md | 38 +++++++++++++++++++------- .github/workflows/ci-size-report.yml | 40 ++++++++++++++++++---------- .github/workflows/ci.yml | 8 ++++-- .github/workflows/nightly-build.yml | 2 ++ 4 files changed, 63 insertions(+), 25 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index c4cd07979e0..d8f486386c1 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -7,13 +7,24 @@ This directory contains automated CI/CD workflows for the INAV project. ### Build and Test #### `ci.yml` - Build Firmware -**Triggers:** Pull requests, pushes to maintenance branches +**Triggers:** Pull requests (`workflow_call` too, used by `nightly-build.yml`). +Also declares an `on: push:` trigger, but it's currently a no-op — a +`branches:` list containing only a negative pattern (`'!maintenance-8.x.x'`) +matches nothing per GitHub's own docs, confirmed empirically (zero +push-triggered runs of this workflow exist in this repo's history). Direct +push builds happen via `nightly-build.yml` instead (see below). **Purpose:** Compiles INAV firmware for all targets to verify builds succeed **Matrix:** 15 parallel build jobs for faster CI -#### `nightly-build.yml` - Nightly Builds -**Triggers:** Scheduled nightly -**Purpose:** Creates nightly development builds for testing +#### `nightly-build.yml` - "Build pre-release" +**Triggers:** `push` to `master`, `maintenance-8.x.x`, `maintenance-9.x`, +`maintenance-10.x`, `release/9.1` — **not** a schedule, despite the +filename. Invokes `ci.yml`'s jobs via `workflow_call`, then publishes a +prerelease to the companion `iNavFlight/inav-nightly` repo. +**Purpose:** Creates nightly development builds for testing, and is the +actual per-push validation + baseline-generation point for +`ci-size-report.yml` (see below) since `ci.yml`'s own push trigger doesn't +fire. ### Documentation @@ -49,7 +60,8 @@ This directory contains automated CI/CD workflows for the INAV project. ### Pull Request Helpers #### `ci-size-report.yml` - RAM/Flash Usage Delta PR Comment -**Triggers:** `workflow_run` after "Build firmware" (`ci.yml`) completes +**Triggers:** `workflow_run` after "Build firmware" (`ci.yml`, PR builds) or +"Build pre-release" (`nightly-build.yml`, branch-push builds) completes **Purpose:** Posts/updates a PR comment showing flash and RAM usage delta vs. the PR's base branch, for 4 representative targets spanning flash/RAM size tiers (MATEKF405, MATEKF722, MATEKF765, MATEKH743 — note MATEKF722 and @@ -61,10 +73,18 @@ every PR. 1. `ci.yml` extracts a small per-target size report (`arm-none-eabi-size` on each built `.elf`) right after each build and uploads it as an artifact — no second build anywhere in this flow. -2. On pushes to a branch, `ci-size-report.yml` persists that size report as - a release asset (`size-baseline-`) in the companion - `iNavFlight/pr-test-builds` repo — the "known good" baseline for that - branch, overwritten on every push. +2. On pushes to `master`/`maintenance-9.x`/`maintenance-10.x`/`release/9.1`, + `nightly-build.yml` ("Build pre-release") invokes `ci.yml` via + `workflow_call` as part of building nightly releases — this already + produces the size report above at no extra build cost. When that + completes, `ci-size-report.yml` persists it as a release asset + (`size-baseline-`) in the companion `iNavFlight/pr-test-builds` + repo — the "known good" baseline for that branch, overwritten on every + push. (`ci.yml`'s *own* `on: push:` trigger is broken — a `branches:` + list containing only a negative pattern matches nothing per GitHub's + docs — so this deliberately listens to `nightly-build.yml` instead of + trying to fix that separately; verified empirically that `ci.yml` alone + has zero push-triggered runs in this repo's history.) 3. On PR builds, it fetches the PR's base branch's persisted baseline (no rebuild), diffs it against the PR's own size report, and posts/updates a comment (marker ``). diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index 403461ac9be..2af5c76d1f1 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -1,31 +1,42 @@ name: CI Size Report -# Runs after "Build firmware" completes. Uses workflow_run (rather than -# pull_request/push directly) so that secrets are available even for PRs -# from forks — same reasoning as pr-test-builds.yml. +# Uses workflow_run (rather than pull_request/push directly) so that +# secrets are available even for PRs from forks — same reasoning as +# pr-test-builds.yml. # -# Two jobs: -# - publish-baseline: on any branch push that triggers ci.yml (in practice, -# almost always maintenance-9.x/maintenance-10.x — see ci.yml's own push -# trigger for the exact filter), persists the size report as a release -# asset in the companion iNavFlight/pr-test-builds repo, so PR runs never -# need to rebuild the base branch to get a comparison point. Stale -# baselines for since-deleted branches aren't cleaned up automatically -# (same known limitation pr-test-builds has for old PR releases). -# - pr-comment: on PR builds, fetches that persisted baseline, diffs the -# 4 representative targets, and posts/updates a PR comment. +# Listens to two different upstream workflows, for two different jobs: +# - publish-baseline listens for "Build pre-release" (nightly-build.yml), +# NOT "Build firmware" directly. ci.yml's own `on: push:` trigger is +# broken (a branches: list with only a negative pattern matches +# nothing — confirmed empirically, zero push-triggered "Build firmware" +# runs exist). "Build pre-release" is push-triggered correctly and +# already invokes ci.yml's jobs via `uses:` (workflow_call) as part of +# building nightly releases — piggybacking here costs nothing extra, +# no second build. Persists the size report as a release asset in the +# companion iNavFlight/pr-test-builds repo, so PR runs never need to +# rebuild the base branch to get a comparison point. Only fires for +# branches nightly-build.yml's own push trigger covers (currently +# master, maintenance-9.x, maintenance-10.x, release/9.1 — see that +# file). Stale baselines for since-deleted branches aren't cleaned up +# automatically (same known limitation pr-test-builds has for old PR +# releases). +# - pr-comment listens for "Build firmware" (ci.yml) directly — PR builds +# genuinely do trigger it via `pull_request`, that part isn't broken. +# Fetches the persisted baseline, diffs the 4 representative targets, +# and posts/updates a PR comment. # # Requires the same repository secret PR_BUILDS_TOKEN as pr-test-builds.yml # (Contents: write access to iNavFlight/pr-test-builds). on: workflow_run: - workflows: ["Build firmware"] + workflows: ["Build firmware", "Build pre-release"] types: [completed] jobs: publish-baseline: runs-on: ubuntu-latest if: > + github.event.workflow_run.name == 'Build pre-release' && github.event.workflow_run.event == 'push' && github.event.workflow_run.conclusion == 'success' concurrency: @@ -73,6 +84,7 @@ jobs: pr-comment: runs-on: ubuntu-latest if: > + github.event.workflow_run.name == 'Build firmware' && github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' concurrency: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 615a55b9e7b..6232c17b431 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -241,12 +241,16 @@ jobs: path: base_ref.txt retention-days: 1 - name: Save branch name - if: github.event_name == 'push' + # Covers both a direct push and being invoked via workflow_call + # (e.g. from nightly-build.yml) — github.event_name reports + # 'workflow_call' in the latter case, not 'push', even though + # github.ref_name still correctly reflects the branch either way. + if: github.event_name != 'pull_request' env: REF_NAME: ${{ github.ref_name }} run: echo "$REF_NAME" > branch.txt - name: Upload branch name - if: github.event_name == 'push' + if: github.event_name != 'pull_request' uses: actions/upload-artifact@v4 with: name: branch-name diff --git a/.github/workflows/nightly-build.yml b/.github/workflows/nightly-build.yml index 51dd16650e5..9744fb85537 100644 --- a/.github/workflows/nightly-build.yml +++ b/.github/workflows/nightly-build.yml @@ -8,6 +8,8 @@ on: - master - maintenance-8.x.x - maintenance-9.x + - maintenance-10.x + - release/9.1 paths: - 'src/**' - '.github/**' From 61e93d27fe406b11652803b4caad5b08a0c53323 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 19 Aug 2026 19:34:54 -0500 Subject: [PATCH 20/38] Register MPU6000 and ICM42605 gyro/acc on AOCODARCF722AIO target.h defines USE_IMU_MPU6000, USE_IMU_MPU6500, and USE_IMU_ICM42605 (shared CS pin PB2, SPI1), but target.c only registered DEVHW_MPU6500 with BUSDEV_REGISTER_SPI_TAG. Units populated with MPU6000 or ICM42605 instead of MPU6500 silently failed gyro detection at boot. Add the missing registrations at the same tag, matching the pattern used on MAMBAH743 for multiple chip options sharing one physical footprint. --- src/main/target/AOCODARCF722AIO/target.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/target/AOCODARCF722AIO/target.c b/src/main/target/AOCODARCF722AIO/target.c index 038b4121b48..84ccbfd6708 100644 --- a/src/main/target/AOCODARCF722AIO/target.c +++ b/src/main/target/AOCODARCF722AIO/target.c @@ -25,6 +25,8 @@ #include "drivers/sensor.h" BUSDEV_REGISTER_SPI_TAG(busdev_mpu6500, DEVHW_MPU6500, MPU6500_SPI_BUS, MPU6500_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_MPU6500_ALIGN); +BUSDEV_REGISTER_SPI_TAG(busdev_mpu6000, DEVHW_MPU6000, MPU6000_SPI_BUS, MPU6000_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_MPU6000_ALIGN); +BUSDEV_REGISTER_SPI_TAG(busdev_icm42605, DEVHW_ICM42605, ICM42605_SPI_BUS, ICM42605_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_ICM42605_ALIGN); timerHardware_t timerHardware[] = { // DEF_TIM(TIM9, CH2, PA3, TIM_USE_PPM, 0, 0), // PPM From 6bfcc9b5966807d5c864b9260351dc04565a7ebf Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 19 Aug 2026 23:49:54 -0500 Subject: [PATCH 21/38] Remove orphaned servo_autotrim_iterm_threshold field Field had no settings.yaml entry, no reset initializer, and no code references anywhere in the tree. --- src/main/flight/servos.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/flight/servos.h b/src/main/flight/servos.h index 3f8ebf1b4e8..07a337a6d56 100644 --- a/src/main/flight/servos.h +++ b/src/main/flight/servos.h @@ -178,7 +178,6 @@ typedef struct servoConfig_s { uint8_t servo_protocol; // See servoProtocolType_e uint8_t tri_unarmed_servo; // send tail servo correction pulses even when unarmed uint8_t servo_autotrim_rotation_limit; // Max rotation for servo midpoints to be updated - uint8_t servo_autotrim_iterm_threshold; // How much of the Iterm is carried over to the servo midpoints on each update } servoConfig_t; PG_DECLARE(servoConfig_t, servoConfig); From fa7a86fa66e9985c8f98115f779aeb8c9f16e35e Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Thu, 20 Aug 2026 10:16:45 -0500 Subject: [PATCH 22/38] Don't gate baseline publish on Build pre-release's overall conclusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Release job (nightly upload to iNavFlight/inav-nightly) can fail for reasons unrelated to the build itself and drags the whole run's conclusion to failure even when the build succeeded and produced the size-report/branch-name artifacts. Confirmed live: the 2026-08-20 push to release/9.1 had every build job succeed but Release fail with "Bad credentials" (NIGHTLY_TOKEN), so publish-baseline never ran and no baseline has ever been published — which is why PR #11800 still shows "No size baseline is available yet". Check the specific build/upload-artifacts job's conclusion instead of the aggregate run conclusion. --- .github/workflows/ci-size-report.yml | 39 +++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index 2af5c76d1f1..9f8122bff77 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -37,15 +37,46 @@ jobs: runs-on: ubuntu-latest if: > github.event.workflow_run.name == 'Build pre-release' && - github.event.workflow_run.event == 'push' && - github.event.workflow_run.conclusion == 'success' + github.event.workflow_run.event == 'push' concurrency: group: size-baseline-${{ github.event.workflow_run.head_branch }} cancel-in-progress: true permissions: - actions: read # to download artifacts from the triggering workflow run + actions: read # to download artifacts and query job conclusions from the triggering workflow run steps: + # Don't gate on github.event.workflow_run.conclusion: "Build + # pre-release" also runs a separate Release job (nightly upload to + # iNavFlight/inav-nightly) that can fail for reasons unrelated to the + # build itself (e.g. an expired NIGHTLY_TOKEN) and drags the whole + # run's conclusion to failure even when the build succeeded and + # produced the artifacts we actually need. Check the specific job + # that produces them instead. + - name: Check build job succeeded + id: check + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RUN_ID: ${{ github.event.workflow_run.id }} + run: | + CONCLUSION=$(gh api "repos/${{ github.repository }}/actions/runs/${RUN_ID}/jobs" --paginate \ + --jq '.jobs[] | select(.name == "build / upload-artifacts") | .conclusion') + if [ -z "$CONCLUSION" ]; then + # Job not found at all usually means the caller/callee job names + # changed (e.g. nightly-build.yml's "build" job or ci.yml's + # "upload-artifacts" job got renamed) — that's a workflow + # structure mismatch, not an expected build failure, and is + # exactly the kind of thing that silently broke baseline + # publishing before. Warn louder than a plain failed build. + echo "::warning::build / upload-artifacts job not found in run ${RUN_ID} — job name may have changed, skipping baseline publish" + echo "proceed=false" >> "$GITHUB_OUTPUT" + elif [ "$CONCLUSION" != "success" ]; then + echo "::notice::build / upload-artifacts did not succeed (conclusion: ${CONCLUSION}), skipping baseline publish" + echo "proceed=false" >> "$GITHUB_OUTPUT" + else + echo "proceed=true" >> "$GITHUB_OUTPUT" + fi + - name: Download size report + if: steps.check.outputs.proceed == 'true' uses: actions/download-artifact@v4 with: name: size-report @@ -53,6 +84,7 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} - name: Download branch name + if: steps.check.outputs.proceed == 'true' uses: actions/download-artifact@v4 with: name: branch-name @@ -60,6 +92,7 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} - name: Publish baseline + if: steps.check.outputs.proceed == 'true' env: GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} run: | From 89f935bfb1ae5ed02508b75c9f6b899b28574957 Mon Sep 17 00:00:00 2001 From: Sensei Date: Sat, 22 Aug 2026 18:43:08 -0500 Subject: [PATCH 23/38] ORBITH743v2: add CAN pins --- src/main/target/ORBITH743v2/target.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/target/ORBITH743v2/target.h b/src/main/target/ORBITH743v2/target.h index 97d31ebb54c..62e50926793 100644 --- a/src/main/target/ORBITH743v2/target.h +++ b/src/main/target/ORBITH743v2/target.h @@ -100,6 +100,11 @@ #define UART8_TX_PIN PE1 #define UART8_RX_PIN PE0 +#define USE_DRONECAN +#define CAN1_RX PD0 +#define CAN1_TX PD1 +#define CAN1_STANDBY PD3 + #define SERIAL_PORT_COUNT 9 //VCP, UART1, UART2, UART3, UART4, UART5, UART6, UART7, UART8 #define DEFAULT_RX_TYPE RX_TYPE_SERIAL From 38c6cf4eee57b64747c1677b4fa8b736c61fb3ce Mon Sep 17 00:00:00 2001 From: orbittechnologyx <163323001+orbittechnologyx@users.noreply.github.com> Date: Sun, 23 Aug 2026 09:46:01 +0300 Subject: [PATCH 24/38] Update CAN1 pin definitions in target.h --- src/main/target/ORBITH743v2/target.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/target/ORBITH743v2/target.h b/src/main/target/ORBITH743v2/target.h index 62e50926793..a1619dbb695 100644 --- a/src/main/target/ORBITH743v2/target.h +++ b/src/main/target/ORBITH743v2/target.h @@ -101,9 +101,9 @@ #define UART8_RX_PIN PE0 #define USE_DRONECAN -#define CAN1_RX PD0 -#define CAN1_TX PD1 -#define CAN1_STANDBY PD3 +#define CAN1_RX PB8 +#define CAN1_TX PB9 +#define CAN1_STANDBY PA4 #define SERIAL_PORT_COUNT 9 //VCP, UART1, UART2, UART3, UART4, UART5, UART6, UART7, UART8 From c4ba4ad37c90d2489715f0cc52c8ad81cfc1260f Mon Sep 17 00:00:00 2001 From: orbittechnologyx Date: Sun, 23 Aug 2026 10:47:57 +0300 Subject: [PATCH 25/38] Added configurations for support LSM6DS320x --- src/main/target/ORBITH743v2/target.c | 1 + src/main/target/ORBITH743v2/target.h | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/main/target/ORBITH743v2/target.c b/src/main/target/ORBITH743v2/target.c index aaa8776c74e..1f99c809da7 100644 --- a/src/main/target/ORBITH743v2/target.c +++ b/src/main/target/ORBITH743v2/target.c @@ -26,6 +26,7 @@ #include "drivers/pinio.h" #include "drivers/sensor.h" +BUSDEV_REGISTER_SPI_TAG(busdev_lsm6dxx, DEVHW_LSM6D, LSM6DXX_SPI_BUS, LSM6DXX_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_LSM6DXX_ALIGN); BUSDEV_REGISTER_SPI_TAG(busdev_icm42688_1, DEVHW_ICM42605, ICM42688_SPI_BUS_1, ICM42688_CS_PIN_1, NONE, 0, DEVFLAGS_NONE, IMU_ICM42688_ALIGN_1); BUSDEV_REGISTER_SPI_TAG(busdev_icm40609, DEVHW_ICM42605, ICM40609_SPI_BUS, ICM40609_CS_PIN, NONE, 0, DEVFLAGS_NONE, IMU_ICM40609_ALIGN); diff --git a/src/main/target/ORBITH743v2/target.h b/src/main/target/ORBITH743v2/target.h index a1619dbb695..89df27ed6c8 100644 --- a/src/main/target/ORBITH743v2/target.h +++ b/src/main/target/ORBITH743v2/target.h @@ -128,6 +128,12 @@ #define ICM40609_SPI_BUS BUS_SPI3 #define ICM40609_EXTI_PIN PD2 +#define USE_IMU_LSM6DXX +#define IMU_LSM6DXX_ALIGN CW270_DEG +#define LSM6DXX_SPI_BUS BUS_SPI1 +#define LSM6DXX_CS_PIN PE11 +#define LSM6DXX_EXTI_PIN PE10 + // *************** OSD ***************************** #define USE_MAX7456 From 64c6a3a9e572dd1068b24f4dbd9c610e14fd64cd Mon Sep 17 00:00:00 2001 From: Sensei Date: Sun, 23 Aug 2026 23:54:16 -0500 Subject: [PATCH 26/38] size-diff-comment.js Update noise threshold for delta calculations Increased the noise threshold from 32 to 256 bytes to filter out minor changes. --- .github/scripts/size-diff-comment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/size-diff-comment.js b/.github/scripts/size-diff-comment.js index 0ec79554ecf..ee8c7fcd279 100644 --- a/.github/scripts/size-diff-comment.js +++ b/.github/scripts/size-diff-comment.js @@ -17,7 +17,7 @@ const REPRESENTATIVE_TARGETS = ['MATEKF405', 'MATEKF722', 'MATEKF765', 'MATEKH74 // Below this magnitude a delta is noise (rounding/toolchain jitter), not a // real change worth calling out. -const NOISE_THRESHOLD_BYTES = 32; +const NOISE_THRESHOLD_BYTES = 256; function formatDelta(deltaBytes, baseBytes) { const sign = deltaBytes > 0 ? '+' : deltaBytes < 0 ? '' : '±'; From e330d4a242b12d35d55bef3f4d5bd5430cf756eb Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 26 Aug 2026 00:01:41 -0500 Subject: [PATCH 27/38] Fix off-by-one in ICM42605 WHO_AM_I retry loop do/while with a decrementing post-condition ran 6 WHO_AM_I attempts instead of 5, adding an extra 150ms delay on the chip-absent boot autodetect path. Replaced with a plain for loop, matching the fix already applied to accgyro_icm40609d.c (66247db440), which was modeled on this file. --- src/main/drivers/accgyro/accgyro_icm42605.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/drivers/accgyro/accgyro_icm42605.c b/src/main/drivers/accgyro/accgyro_icm42605.c index b6cd029f9e2..a0538aa3206 100644 --- a/src/main/drivers/accgyro/accgyro_icm42605.c +++ b/src/main/drivers/accgyro/accgyro_icm42605.c @@ -288,13 +288,12 @@ static void icm42605AccAndGyroInit(gyroDev_t *gyro) static bool icm42605DeviceDetect(busDevice_t * dev) { uint8_t tmp; - uint8_t attemptsRemaining = 5; busSetSpeed(dev, BUS_SPEED_INITIALIZATION); busWrite(dev, ICM42605_RA_PWR_MGMT0, 0x00); - do { + for (int attempt = 0; attempt < 5; attempt++) { delay(150); busRead(dev, MPU_RA_WHO_AM_I, &tmp); @@ -315,7 +314,7 @@ static bool icm42605DeviceDetect(busDevice_t * dev) // Retry detection break; } - } while (attemptsRemaining--); + } return false; } From 63483529ca19658246d0d7060b7dbe926c040dd7 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 26 Aug 2026 10:02:07 -0500 Subject: [PATCH 28/38] docs: expand RAM/flash optimization guide, rewrite as rule-first reference Replace the PR #11718 seed stub with the full strategy guide. Each technique is stated as a rule with a short example rather than a per-PR anecdote. Reconciles the two previously-contradictory sections on static-in-header linkage: the rule is to be wary of `static` in a header (private copy per including TU, prefer extern + one .c definition), with the exception that pure, stateless static-inline functions are safe while the compiler inlines or discards them. --- .../development/ram-and-flash-optimization.md | 301 ++++++++++++++---- 1 file changed, 241 insertions(+), 60 deletions(-) diff --git a/docs/development/ram-and-flash-optimization.md b/docs/development/ram-and-flash-optimization.md index f13eae8bef3..173b08b65d8 100644 --- a/docs/development/ram-and-flash-optimization.md +++ b/docs/development/ram-and-flash-optimization.md @@ -1,65 +1,246 @@ # RAM and Flash Optimization -> **Status: seed draft.** This is a stub started while fixing a RAM overflow -> on PR #11718, so the technique wouldn't get lost before the full guide is -> written. It is not yet linked from `Development.md` and is not a complete -> guide — see the tracked project for the rest of this doc's scope. - -Static RAM headroom is a binding constraint on several boards (128KB-RAM -F4 targets in particular). Treat RAM and flash reduction as an active goal -when writing or reviewing code that adds static buffers, not just something -to check for overflow after the fact. - -## Don't size a buffer for a different subsystem's worst case - -If a buffer is reused across features, check what its size actually derives -from before assuming a new consumer needs the same size. On PR #11718 (the -MSP-over-MAVLink tunnel), a reply buffer was sized off `MSP_PORT_OUTBUF_SIZE` -(4112 bytes under `USE_FLASHFS`) simply because that was the constant already -in scope for MSP replies. But that size exists to let `MSP_DATAFLASH_READ` -return a full 4096-byte flash page in one shot for fast blackbox downloads -over USB/serial — the tunnel's own use case (MSP replies fragmented into -128-byte MAVLink `TUNNEL` payload chunks regardless of source buffer size) -never benefited from that size at all. Giving the tunnel its own, -independently-sized constant (512 bytes, matching the size every -non-FLASHFS board already uses for ordinary MSP replies) cut ~3.6KB with no -functional loss — data that used to arrive in one large read now arrives in -more, smaller reads, which costs nothing extra when it's already being -fragmented for the wire. - -**Before reusing an existing size constant for a new buffer, ask what -specifically drove that number** — it may be tied to an unrelated worst -case that doesn't apply to the new use. - -## When shrinking a shared buffer, audit what else writes into it - -A buffer's "extra" size can be silently load-bearing for a bug elsewhere, -not just wasted space. While sizing the tunnel buffer above, auditing every -MSP command reachable through it surfaced `serializeDataflashReadReply()` -computing its available-space clamp *before* writing a 4-byte address -header, then writing `address + clamped_data` — an unconditional 4-byte -overflow whenever a request is large enough to hit the clamp. On the -original 4112-byte buffer this was practically unreachable (16 bytes of -incidental headroom absorbed it for a standard 4096-byte request); shrinking -to 512 bytes with no equivalent headroom made it trivially reachable by any -normal-sized request. - -**When shrinking a buffer other code paths also write into, check whether -any of those paths were relying on the buffer being larger than their own -worst-case write** — not just whether the new size covers the new -consumer's own worst case. Fix the root cause (the clamp math, here) rather -than padding the buffer back up to paper over it. - -## Survey actual message/data sizes before assuming a buffer needs to be large - -"How big does this actually need to be" is often answerable by grep, not -guesswork — but match the audit's scope to everything that can actually -reach the buffer. A first pass on the tunnel buffer above checked only MSP2 -command handlers and found 432 bytes as the largest reply; widening to MSP1 -(legacy) handlers — reachable through the same buffer — found 512 bytes -exactly (`MSP_LED_STRIP_CONFIG`, no bounds check, no margin). Scoping an -audit to "the obviously relevant" category of code instead of everything -that can reach the buffer produces a confident-looking wrong answer. +> **Status: draft.** Started from PR #11718; not yet linked from +> `Development.md` and not a complete guide. + +Static RAM headroom is a binding constraint on several boards (128KB-RAM F4 +targets in particular). Treat RAM and flash reduction as an active goal when +writing or reviewing code that adds static buffers or new functions, not +just something to check for overflow after the fact. + +## Buffer sizing + +### Don't reuse another subsystem's size constant + +An existing size constant exists for a specific consumer; reusing it for a +different buffer usually over-allocates. Before reusing a constant, check +what specifically drove that number. + +Example: the MSP-over-MAVLink tunnel reply buffer was sized off +`MSP_PORT_OUTBUF_SIZE` (4112 B), which exists for `MSP_DATAFLASH_READ` +(returning a full flash page in one reply). The tunnel fragments replies +into 128-byte chunks regardless of source buffer size, so it never +benefited; its own 512-byte constant cut ~3.6 KB with no functional loss. + +### Audit every writer before shrinking a shared buffer + +Shrinking a shared buffer can expose code that relied on it being larger +than its own worst case. Check every code path into the buffer, not just +the new consumer. + +Example: shrinking the tunnel buffer (4112 B → 512 B) exposed +`serializeDataflashReadReply()`, which computed its space clamp *before* +writing a 4-byte address header — an unconditional 4-byte overflow once the +buffer had no spare headroom. + +### Measure real message sizes before sizing a buffer + +Audit everything that can reach the buffer, not just the obvious category. +A narrow audit produces a confident-looking wrong answer. + +Example: a first pass on the tunnel buffer checked only MSP2 handlers +(432 B largest reply); MSP1 legacy handlers reachable through the same +buffer needed 512 B (`MSP_LED_STRIP_CONFIG`, unbounded). + +### Chunk a large buffer into a circular buffer when the peripheral drains continuously + +If the peripheral consumes the buffer continuously, a circular buffer +refilled from a DMA half/full-transfer interrupt replaces one large static +array. Refill must come from a real hardware DMA interrupt — a +task-scheduler tick leaves gaps that read as a protocol reset. + +Example: the WS2812 driver buffered all 128 possible LEDs in one static +array (6,230 B) for a single DMA burst; a circular buffer holding 2 groups +of 4 LEDs (384 B), refilled from the DMA interrupt as each group finishes, +cut it >16x with no protocol change. + +### A minimum-duration constraint doesn't need buffer space + +A "hold low for at least ~50-60 us" requirement is a minimum, not a +window. Minimums are usually free — idle state already exceeds them — +while maximums aren't. Check which one you have before reserving buffer +space or adding a blocking wait. + +Example: the LED driver's 42-element reset/latch preamble became a +`micros()` compare (the line already idles low longer than the minimum); +the idle-tail became a direct write to the timer's preload-buffered +compare register. + +## Bounding work + +### Bound work to what's actually configured + +Process only the configured count when a runtime count is available, even +if the buffer stays sized for the worst case. This cuts CPU work and DMA +transfer proportionally, at zero cost to full-length users. + +Example: the LED driver processed all 128 possible slots every update; +threading `ledCounts.count` into the fill loop and DMA transfer length cut +work for short strips. + +## Sending data + +### Send components separately instead of concatenating + +Concatenating buffers costs the sum of their sizes. If the consumer is +sequential, send the components in order and skip the intermediate buffer. + +Example: a protocol of header + large body + trailer saved 4 KB of RAM by +sending the three components in order over serial instead of assembling a +"message" buffer. + +## Functions, linkage, and headers + +### Don't default to inlining large or multi-call-site functions + +Inlining creates one copy of the function body per call site. Let the +compiler decide — it's usually better at this than manual `inline` hints — +and reach for `NOINLINE` on large functions with several call sites that +don't need to be fast-path. + +### Be wary of `static` in a header + +`static` in a header (with or without `inline`) creates a private copy in +every `.c` file that includes it — of the function *and* of any local +`static` array/struct/table inside it. The linker cannot merge +internal-linkage symbols. `inline` is not the cause; `static` is. When you +don't need internal linkage, prefer `extern`: declare the function/object +in the header, define it exactly once in a matching `.c` file. Vendored +libraries often ship a documented switch for this — check before +hand-patching generated files, which get wiped on regeneration. + +Example: `mavlink_get_msg_entry()` in +`lib/main/MAVLink/mavlink_helpers.h` declares its local +`mavlink_message_crcs[]` table (337 entries, 4,044 B on the storm32 +dialect) with `static` linkage via the `MAVLINK_HELPER` macro. Two firmware +TUs call the function, so the binary carried two copies — 8,088 B on +BLUEBERRYF405. A grep for `"static inline"` missed it because the macro +hides the `static` keyword from the call site. Defining +`MAVLINK_SEPARATE_HELPERS` plus one compiled `mavlink_helpers.c` removed +the duplication. + +Use the audit tool in +`claude/developer/scripts/static-linkage-audit/` (inav-claude tooling +repo) to scan for static-in-header symbols and estimate duplication cost. + +### `static` in a header is okay only for pure, stateless functions + +The exception to the rule above: a `static inline` function with no static +state — no file-scope statics, no local `static` arrays or tables — has +nothing to duplicate, so it costs no RAM. It is not literally free, +though: inline still multiplies copies when there are multiple callers. +Every call site the compiler inlines embeds its own copy of the body in +flash, and if the compiler cannot inline the function and more than one TU +calls it, each TU gets an out-of-line copy that the linker cannot merge. +Keep such functions small and few-call-site, or fall back to the `extern` +pattern above. + +Example: PR #11553's four `*_logic.h` files (~1,700 lines, VTOL +auto-transition) are 100% `static inline` with no top-level static state — +each inlines into its one or two callers or gets discarded, and each has a +unit test that exercises the logic without linking the surrounding +subsystem. + +## State machines + +### Extend an enum-indexed dispatch table — don't grow an if/else-if chain + +New states belong in an existing table/switch as entries, not as branches +bolted onto the dispatch site. A parallel if/else-if chain costs more +flash and adds a second place to keep state in sync. + +Example: PR #11553 added navigation FSM states as +`[NAV_STATE_...] = { ... }` entries in `navFSM[NAV_STATE_COUNT]`, and its +mixer-profile transition FSM uses a dense `switch` the compiler can turn +into a jump table. + +## Duplication: state vs code + +### Distinguish necessary state duplication from avoidable code duplication + +A large duplicated struct may be necessary state — a second copy evolving +in parallel across time — while near-identical duplicated functions are +usually avoidable code, one parameterization away from being unified. +Trace which pattern a duplication finding is before accepting it as an +inherent cost. + +Example (necessary state): PR #11553's +`autoTransitionTargetPidState[FLIGHT_DYNAMICS_INDEX_COUNT]` is a full +second copy of `pidState_t` (3,072 B on MATEKF405, ~95% of the PR's net +RAM+CCM growth) — a bumpless handoff needs the *target* controller's +filter/integrator state evolving continuously in parallel, not a single +output value computed once. + +Example (avoidable code): the ~10-11 near-line-for-line duplicate +functions in `pid.c` (`applyAutoTransitionTargetDBoost` vs `applyDBoost`, ++6,632 B) exist only because the live PID functions read tuning values +from file-scope globals instead of taking a `pidProfile_t*` — the new +functions already take the profile explicitly, so the duplication is a +refactor away from being unified. + +## Measuring size + +### Sum every RAM-like region when reconciling against CI + +CI's single "RAM Δ" figure on F4/F7/H7 targets can be `RAM` + `CCM`/`DTCM` +summed together, not the `RAM:` linker region alone. Check which regions +the figure spans before suspecting the build or the measurement. + +Example: on PR #11553/MATEKF405, `RAM:` alone grew only 608 B against a +reported CI delta of +3,740 B — the gap was CCM growing another 3,128 B +and CI counting both regions as one number. + +### In an LTO-merged symbol, check every function folded into it + +LTO merges several unrelated functions into one symbol, so an +`nm --size-sort` diff attributes their combined growth to whichever name +survives — usually the caller, not the function that actually grew. Don't +attribute a merged symbol's growth to the most-recently-reviewed nearby +change; check every function folded into the blob. + +Example: a +5,168 B growth in `programmingFrameworkUpdateTask` was +initially guessed to be `logic_condition.c`'s new operands; that symbol +actually folds `programmingPidUpdateTask`, `outputProfileUpdateTask`, and +`logicConditionUpdateTask`, and the growth was entirely +`outputProfileUpdateTask()` in `mixer_profile.c`. + +### Isolate one function's real cost with NOINLINE at both commits + +To measure one function's true standalone cost inside an LTO-merged blob, +add `NOINLINE` to just that function in both the base and HEAD builds so it +gets its own `nm` entry at both points. **Caveat:** `NOINLINE` is +`__attribute__((noinline))` only under `USE_ITCM_RAM` (F7/H7 targets) — +it's a no-op on F4, so run this kind of isolation on an F7/H7 target or +the "before" and "after" builds will look identical for the wrong reason. + +Example: `outputProfileUpdateTask()` measured 148 B → 4,708 B once pulled +out of the merged symbol. + +## Hardware width pitfalls + +### Verify register-width assumptions on real hardware + +A DMA transfer's element width must match the target register width. This +class of bug is invisible to the compiler and to code review — confirm on +real hardware with a scope. + +Example: the LED strip DMA buffer used `uint8_t` elements (the theoretical +minimum for a duty value); TIM3/TIM4's CCR is a 16-bit register, so the +byte-sized DMA writes only updated its low byte and the output pin stuck +high. `uint16_t`, matching the register width, fixed it. + +### Use the codebase's DMA-safe width type, not a narrower derivation + +When the codebase defines a type for "the width DMA needs to write this +register safely," use it instead of deriving a narrower one from a single +datasheet — a width verified on one board doesn't prove it on every board, +because different MCU families route through different code branches. + +Example: `timerDMASafeType_t` is `uint32_t` on every F4/F7/H7/AT32 +`timer_def_*.h`, because DMA writes to a timer CCR must be word-width +across the whole family. A `uint16_t` fix compiled cleanly on H7 but +routed through a previously-unreached branch containing an unrelated +copy-paste bug that silently downgraded the write to a single byte. ## Related From b73a02bc510ed0bb7732e4341d50c8216b05fbf2 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Fri, 28 Aug 2026 19:29:34 -0500 Subject: [PATCH 29/38] Compare PR size deltas against the merge-base commit, not the branch tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The size-diff workflow stored one baseline per branch, clobbered on every push, so PRs were compared against the base branch's LATEST tip. When the base advances after a PR forks, the delta includes unrelated changes (PR #11785 showed a stale +9,788 B RAM figure that actually belonged to #11438; the PR's real marginal RAM was ~120 B). Publish per-commit baselines alongside the branch-tip pointer: - publish-baseline now stores size-baseline- (primary) in addition to size-baseline- (backward-compat latest-tip pointer), and prunes per-commit baselines to the newest 50 per branch (plus a global cap) so iNavFlight/pr-test-builds can't grow unbounded. Per-commit releases carry a machine-readable branch marker in their notes so pruning can group them. Resolve the PR's true base commit on the comparison side: - pr-comment computes the merge-base of PR head and base ref via the compare API (verified to resolve fork-PR head SHAs), fetches the exact per-commit baseline, and falls back to the nearest ancestor commit that has one by walking the base branch's first-parent chain. The branch-tip baseline is deliberately NOT a fallback — comparing against it is the stale-delta bug this fixes. All new SHA/ref inputs are regex-validated before use in shell commands or release tags. - The comment header now names the baseline commit used ("vs. base commit `abcd123`"), with a note when a nearest-available baseline was used; the graceful "no size baseline available" path is unchanged. Also fixes two pre-existing size-diff-comment tests whose notable-delta expectations predated NOISE_THRESHOLD_BYTES being raised to 256. --- .github/scripts/fetch-size-baseline.sh | 109 ++++++++++++++++++ .github/scripts/publish-size-baseline.sh | 130 ++++++++++++++++++++++ .github/scripts/size-diff-comment.js | 17 ++- .github/scripts/size-diff-comment.test.js | 81 ++++++++++++-- .github/workflows/README.md | 35 +++--- .github/workflows/ci-size-report.yml | 111 +++++++++++------- 6 files changed, 417 insertions(+), 66 deletions(-) create mode 100644 .github/scripts/fetch-size-baseline.sh create mode 100644 .github/scripts/publish-size-baseline.sh diff --git a/.github/scripts/fetch-size-baseline.sh b/.github/scripts/fetch-size-baseline.sh new file mode 100644 index 00000000000..9a6ad4b095f --- /dev/null +++ b/.github/scripts/fetch-size-baseline.sh @@ -0,0 +1,109 @@ +#!/bin/bash +# +# Fetch the size baseline for a PR's TRUE base commit — the merge-base of +# the PR head and base ref — rather than the base branch's latest tip, so +# the size-diff delta doesn't include unrelated changes merged to the base +# after the PR forked (see the +9,788 B stale-RAM regression on PR #11785). +# +# Strategy, in order: +# 1. Exact: size-baseline- in the companion builds repo. +# 2. Nearest ancestor: walk first-parents of the merge-base (older commits +# on the base branch) up to MAX_WALK steps, using the first per-commit +# baseline tag found. Nightly baselines exist only for commits that were +# actually pushed to the branch, so the walk finds the closest nightly +# build at-or-before the fork point. +# 3. Otherwise report found=false (caller emits the graceful +# "no size baseline available" comment path). +# +# The base branch's LATEST-tip baseline is deliberately NOT used as a +# fallback: comparing against it is exactly the stale-delta bug this fixes. +# +# Usage: fetch-size-baseline.sh +# the firmware repo, e.g. iNavFlight/inav +# companion repo holding the baselines, e.g. iNavFlight/pr-test-builds +# PR base branch, e.g. maintenance-10.x (validated) +# full 40-hex SHA of the PR's merge-base commit (validated) +# directory to write size-report.json into +# +# Env: GH_TOKEN with read access to both repos (secrets.PR_BUILDS_TOKEN in +# the pr-comment job has it; the companion repo is private). +# +# Prints key=value lines to stdout for the workflow to append to +# $GITHUB_OUTPUT: found, baseline_commit (full SHA), baseline_commit_short, +# baseline_exact (true when the merge-base itself had a stored baseline). + +set -euo pipefail + +MAIN_REPO=${1:?usage: fetch-size-baseline.sh } +BUILDS_REPO=${2:?usage: fetch-size-baseline.sh } +BASE_REF=${3:?usage: fetch-size-baseline.sh } +MERGE_BASE_SHA=${4:?usage: fetch-size-baseline.sh } +OUT_DIR=${5:?usage: fetch-size-baseline.sh } + +MAX_WALK=30 + +if ! [[ "$BASE_REF" =~ ^[A-Za-z0-9._/-]{1,100}$ ]]; then + echo "::error::Invalid base ref: $BASE_REF" >&2 + exit 1 +fi +if ! [[ "$MERGE_BASE_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::Invalid merge-base SHA: $MERGE_BASE_SHA" >&2 + exit 1 +fi + +mkdir -p "$OUT_DIR" + +# Set of per-commit baseline SHAs currently stored in the builds repo. +# One paginated call; tags are built by publish-size-baseline.sh and match +# the strict 40-hex pattern by construction. @tsv keeps the output raw +# (gh api has no -r/--raw flag; string filters would JSON-quote values). +list_baseline_shas() { + gh api "repos/${BUILDS_REPO}/releases?per_page=100" --paginate \ + --jq '.[] | select(.tag_name | test("^size-baseline-[0-9a-f]{40}$")) | + [.tag_name | sub("^size-baseline-"; "")] | @tsv' +} + +# publish-size-baseline replaces the asset in place (--clobber), which still +# briefly deletes-then-uploads under the hood; a few short retries absorb +# that window instead of misreporting "no baseline available". +download_baseline() { # $1 = 40-hex sha + local tag="size-baseline-${1}" + for attempt in 1 2 3; do + if gh release download "$tag" --repo "$BUILDS_REPO" \ + --pattern size-report.json --dir "$OUT_DIR" 2>/dev/null; then + return 0 + fi + sleep 3 + done + return 1 +} + +emit_found() { # $1 = 40-hex sha, $2 = true|false (exact) + echo "found=true" + echo "baseline_commit=${1}" + echo "baseline_commit_short=${1:0:7}" + echo "baseline_exact=${2}" +} + +BASELINE_SHAS=$(list_baseline_shas) || true + +if grep -qx "$MERGE_BASE_SHA" <<< "$BASELINE_SHAS" && download_baseline "$MERGE_BASE_SHA"; then + emit_found "$MERGE_BASE_SHA" true + exit 0 +fi + +# Nearest-ancestor walk along the base branch's first-parent chain. +sha="$MERGE_BASE_SHA" +for _ in $(seq 1 "$MAX_WALK"); do + parent=$(gh api "repos/${MAIN_REPO}/commits/${sha}" --jq '.parents[0].sha // empty' 2>/dev/null || true) + if [ -z "$parent" ] || ! [[ "$parent" =~ ^[0-9a-f]{40}$ ]]; then + break + fi + sha="$parent" + if grep -qx "$sha" <<< "$BASELINE_SHAS" && download_baseline "$sha"; then + emit_found "$sha" false + exit 0 + fi +done + +echo "found=false" diff --git a/.github/scripts/publish-size-baseline.sh b/.github/scripts/publish-size-baseline.sh new file mode 100644 index 00000000000..b409e1759cd --- /dev/null +++ b/.github/scripts/publish-size-baseline.sh @@ -0,0 +1,130 @@ +#!/bin/bash +# +# Publish the nightly size report as release assets in the companion +# pr-test-builds repo, keyed by BRANCH (latest-tip pointer, kept for +# backward compatibility) AND by COMMIT SHA (primary — lets PR size-diff +# comparisons use the PR's exact base commit instead of the branch tip), +# then prune old per-commit baselines so the companion repo doesn't grow +# unbounded. +# +# Usage: publish-size-baseline.sh [--dry-run] +# companion repo, e.g. iNavFlight/pr-test-builds +# branch the nightly was pushed to (e.g. maintenance-10.x) +# full 40-hex SHA of the pushed commit (workflow_run.head_sha) +# merged size-report.json artifact +# --dry-run print what would be published/pruned without touching GitHub +# +# Env: GH_TOKEN with write access to (Contents: write). +# +# Per-commit baseline releases carry a machine-readable `branch: ` +# first line in their notes so pruning can group them per branch. Tags are +# `size-baseline-<40-hex-sha>`; branch-tip tags are `size-baseline-`. +# +# Pruning policy: keep the newest KEEP_PER_BRANCH per-commit baselines per +# branch, plus a GLOBAL_CAP safety net for orphaned baselines (deleted +# branches, notes parse failures) so the repo can never grow unbounded. + +set -euo pipefail + +BUILDS_REPO=${1:?usage: publish-size-baseline.sh [--dry-run]} +BRANCH=${2:?usage: publish-size-baseline.sh [--dry-run]} +COMMIT_SHA=${3:?usage: publish-size-baseline.sh [--dry-run]} +REPORT_JSON=${4:?usage: publish-size-baseline.sh [--dry-run]} +DRY_RUN=${5:-} + +KEEP_PER_BRANCH=50 +GLOBAL_CAP=300 + +# --- Validate inputs: never trust artifact content or context values before +# they are used to build shell commands or release tags. +if ! [[ "$BRANCH" =~ ^[A-Za-z0-9._/-]{1,100}$ ]]; then + echo "::error::Invalid branch name in artifact: $BRANCH" >&2 + exit 1 +fi +if ! [[ "$COMMIT_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::Invalid commit SHA: $COMMIT_SHA" >&2 + exit 1 +fi +if [ ! -f "$REPORT_JSON" ]; then + echo "::error::Size report not found: $REPORT_JSON" >&2 + exit 1 +fi + +# publish one baseline release: create once, then only ever replace the +# asset in place (--clobber) so the release/tag stays continuously +# resolvable for concurrent PR comparisons. +publish_asset() { + local tag="$1" title="$2" notes="$3" + if gh release view "$tag" --repo "$BUILDS_REPO" >/dev/null 2>&1; then + gh release upload "$tag" "$REPORT_JSON" --repo "$BUILDS_REPO" --clobber + else + gh release create "$tag" "$REPORT_JSON" --repo "$BUILDS_REPO" --prerelease \ + --title "$title" --notes "$notes" + fi +} + +BRANCH_TAG="size-baseline-${BRANCH}" +COMMIT_TAG="size-baseline-${COMMIT_SHA}" + +if [ "$DRY_RUN" = "--dry-run" ]; then + echo "[dry-run] would publish ${BRANCH_TAG} and ${COMMIT_TAG} in ${BUILDS_REPO}" +else + publish_asset "$BRANCH_TAG" "Size baseline: ${BRANCH}" \ + "Latest per-target flash/RAM size report for ${BRANCH}. Auto-updated on every push. Not for human consumption." + publish_asset "$COMMIT_TAG" "Size baseline: ${COMMIT_SHA}" \ + "branch: ${BRANCH} +Per-commit per-target flash/RAM size report for ${COMMIT_SHA}. Consumed by PR size-diff comparisons; pruned to the newest ${KEEP_PER_BRANCH} per branch." +fi + +# --------------------------------------------------------------------------- +# Prune per-commit baselines +# --------------------------------------------------------------------------- +# Emit \t\t for every per-commit baseline release, +# newest first (GitHub release listing is newest-first). Branch comes from +# the notes' `branch: ` first line; '?' when unparseable. @tsv keeps +# the output raw (gh api has no -r/--raw flag; string filters would +# JSON-quote values). +list_per_commit_baselines() { + gh api "repos/${BUILDS_REPO}/releases?per_page=100" --paginate \ + --jq '.[] | select(.tag_name | test("^size-baseline-[0-9a-f]{40}$")) | + [.created_at, .tag_name, + ((.body // "") | capture("^branch: (?[A-Za-z0-9._/-]+)$") | .b // "?")] | @tsv' +} + +prune() { + local tmp + tmp=$(mktemp) + trap 'rm -f "$tmp"' RETURN + + # Pass 1 (NR==FNR): per-branch keep-set — newest KEEP_PER_BRANCH tags per + # branch. Pass 2: apply the global cap to the kept set in input order. + # Prints the tags to DELETE (kept per-branch but cut by the cap, or never + # in the keep-set at all). The API lists newest-first but pagination can + # interleave pages, so re-sort descending by created_at for a guaranteed + # newest-first input to the keep-selection. + list_per_commit_baselines | sort -r > "$tmp" + + awk -F '\t' -v keep="$KEEP_PER_BRANCH" -v cap="$GLOBAL_CAP" ' + NR == FNR { + if (count[$3] < keep) { count[$3]++; keepTag[$2] = 1 } + next + } + { + if (keepTag[$2]) { + if (kept < cap) { kept++; keepFinal[$2] = 1 } + } + } + END { + for (t in keepTag) if (!keepFinal[t]) print t + } + ' "$tmp" "$tmp" | while read -r tag; do + if [ "$DRY_RUN" = "--dry-run" ]; then + echo "[dry-run] would prune ${tag}" + else + gh release delete "$tag" --repo "$BUILDS_REPO" --yes --cleanup-tag \ + || echo "::warning::failed to prune ${tag}" >&2 + fi + done +} + +prune diff --git a/.github/scripts/size-diff-comment.js b/.github/scripts/size-diff-comment.js index ee8c7fcd279..e3b074f2d6a 100644 --- a/.github/scripts/size-diff-comment.js +++ b/.github/scripts/size-diff-comment.js @@ -60,17 +60,28 @@ function diffSizeReports(prReport, baselineReport) { } // docLink: string URL to link, or null/undefined to omit the doc-link line. -function renderComment({ prReport, baselineReport, shortSha, docLink, marker }) { +// baselineCommit: short SHA of the baseline commit the delta was computed +// against, or null/undefined to fall back to the generic "base branch" +// wording. baselineIsNearest: true when the exact base commit had no +// stored baseline and a nearest-ancestor baseline was used instead. +function renderComment({ prReport, baselineReport, shortSha, baselineCommit, baselineIsNearest, docLink, marker }) { const rows = diffSizeReports(prReport, baselineReport); - const lines = [marker, '**RAM / Flash usage vs. base branch** — commit `' + shortSha + '`', '']; + const vs = baselineCommit ? `vs. base commit \`${baselineCommit}\`` : 'vs. base branch'; + const lines = [marker, `**RAM / Flash usage ${vs}** — commit \`${shortSha}\``, '']; if (!baselineReport) { lines.push( - '> No size baseline is available yet for this PR\'s base branch ' + + '> No size baseline is available yet for this PR\'s base commit ' + '(first run after this feature shipped, or a new branch). ' + 'This comment will show deltas once a baseline exists.', '' ); + } else if (baselineIsNearest) { + lines.push( + '> Using the nearest available size baseline — the PR\'s exact base ' + + 'commit has no stored baseline yet.', + '' + ); } const anyComparable = rows.some((r) => r.status === 'compared' || r.status === 'no-baseline'); diff --git a/.github/scripts/size-diff-comment.test.js b/.github/scripts/size-diff-comment.test.js index d235e0132d6..ba00fa3ec55 100644 --- a/.github/scripts/size-diff-comment.test.js +++ b/.github/scripts/size-diff-comment.test.js @@ -84,12 +84,12 @@ test('diffSizeReports: notable is gated at exactly NOISE_THRESHOLD_BYTES', () => test('diffSizeReports: notable also triggers from ramDelta alone, and honors negative deltas via Math.abs', () => { const base = { MATEKF405: { flash: 100000, ram: 50000 } }; - const pr = { MATEKF405: { flash: 100000, ram: 50000 - 40 } }; // flash unchanged, ram shrank by 40 + const pr = { MATEKF405: { flash: 100000, ram: 50000 - 300 } }; // flash unchanged, ram shrank by 300 const row = diffSizeReports(pr, base).find((r) => r.target === 'MATEKF405'); assert.equal(row.flashDelta, 0); - assert.equal(row.ramDelta, -40); - assert.equal(row.notable, true, 'a -40 ram delta exceeds the 32-byte threshold in magnitude'); + assert.equal(row.ramDelta, -300); + assert.equal(row.notable, true, 'a -300 ram delta exceeds NOISE_THRESHOLD_BYTES in magnitude'); }); test('diffSizeReports: target missing from PR report but present in baseline -> missing-from-pr', () => { @@ -155,9 +155,9 @@ test('renderComment: baseline present, all 4 targets compared, mix of notable/no MATEKH743: [530000, 63000], }); const prReport = fullReport({ - MATEKF405: [500100, 60000], // +100 flash -> notable + MATEKF405: [500300, 60000], // +300 flash -> notable MATEKF722: [510010, 61000], // +10 flash -> not notable - MATEKF765: [519960, 62000], // -40 flash -> notable + MATEKF765: [519700, 62000], // -300 flash -> notable MATEKH743: [530000, 63000], // no change -> not notable }); @@ -179,9 +179,9 @@ test('renderComment: baseline present, all 4 targets compared, mix of notable/no const matekf765Line = lines.find((l) => l.startsWith('| MATEKF765')); const matekh743Line = lines.find((l) => l.startsWith('| MATEKH743')); - assert.ok(matekf405Line.includes('⚠️'), 'MATEKF405 (+100 flash) should be flagged notable'); + assert.ok(matekf405Line.includes('⚠️'), 'MATEKF405 (+300 flash) should be flagged notable'); assert.ok(!matekf722Line.includes('⚠️'), 'MATEKF722 (+10 flash) should NOT be flagged notable'); - assert.ok(matekf765Line.includes('⚠️'), 'MATEKF765 (-40 flash) should be flagged notable'); + assert.ok(matekf765Line.includes('⚠️'), 'MATEKF765 (-300 flash) should be flagged notable'); assert.ok(!matekh743Line.includes('⚠️'), 'MATEKH743 (no change) should NOT be flagged notable'); // No "no baseline available" note when a baseline was supplied. @@ -286,3 +286,70 @@ test('renderComment: result always ends with exactly one trailing newline', () = assert.ok(body.endsWith('\n')); assert.ok(!body.endsWith('\n\n')); }); + +// --------------------------------------------------------------------------- +// renderComment: baseline-commit header +// --------------------------------------------------------------------------- + +test('renderComment: baselineCommit supplied -> header names the baseline commit', () => { + const body = renderComment({ + prReport: fullReport({ MATEKF405: [500000, 60000] }), + baselineReport: fullReport({ MATEKF405: [499000, 60000] }), + shortSha: 'abc1234', + baselineCommit: '9e932ba', + baselineIsNearest: false, + docLink: null, + marker: '', + }); + + assert.ok(body.includes('vs. base commit `9e932ba`'), 'header should name the baseline commit'); + assert.ok(body.includes('commit `abc1234`'), 'header should still name the PR head commit'); + assert.ok(!body.includes('vs. base branch'), 'exact baseline should not use the generic "base branch" wording'); + assert.ok(!body.includes('nearest available size baseline'), 'exact baseline should not show the fallback note'); + assert.ok(!body.includes('No size baseline is available yet'), 'baseline present means no "no baseline" note'); +}); + +test('renderComment: baselineCommit + baselineIsNearest -> fallback note shown', () => { + const body = renderComment({ + prReport: fullReport({ MATEKF405: [500000, 60000] }), + baselineReport: fullReport({ MATEKF405: [499000, 60000] }), + shortSha: 'abc1234', + baselineCommit: '7f133b3', + baselineIsNearest: true, + docLink: null, + marker: '', + }); + + assert.ok(body.includes('vs. base commit `7f133b3`'), 'header should name the nearest baseline commit'); + assert.ok(body.includes('nearest available size baseline'), 'fallback note should explain the nearest-baseline choice'); +}); + +test('renderComment: baselineCommit omitted -> generic "vs. base branch" wording kept', () => { + const body = renderComment({ + prReport: fullReport({ MATEKF405: [500000, 60000] }), + baselineReport: fullReport({ MATEKF405: [499000, 60000] }), + shortSha: 'abc1234', + docLink: null, + marker: '', + }); + + assert.ok(body.includes('vs. base branch'), 'legacy callers without baselineCommit keep the old wording'); + assert.ok(!body.includes('base commit `'), 'no baseline commit rendered when none supplied'); +}); + +test('renderComment: baselineCommit supplied but no baseline report -> graceful note still wins', () => { + // Defensive: the workflow only sets baselineCommit when a baseline was + // found, so this combination should not occur; if it ever does, the + // "no baseline available" note must still be present (never silently + // claim a comparison happened). + const body = renderComment({ + prReport: fullReport({ MATEKF405: [500000, 60000] }), + baselineReport: null, + shortSha: 'abc1234', + baselineCommit: '9e932ba', + docLink: null, + marker: '', + }); + + assert.ok(body.includes('No size baseline is available yet')); +}); diff --git a/.github/workflows/README.md b/.github/workflows/README.md index d8f486386c1..2d6253d920d 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -77,20 +77,29 @@ every PR. `nightly-build.yml` ("Build pre-release") invokes `ci.yml` via `workflow_call` as part of building nightly releases — this already produces the size report above at no extra build cost. When that - completes, `ci-size-report.yml` persists it as a release asset - (`size-baseline-`) in the companion `iNavFlight/pr-test-builds` - repo — the "known good" baseline for that branch, overwritten on every - push. (`ci.yml`'s *own* `on: push:` trigger is broken — a `branches:` - list containing only a negative pattern matches nothing per GitHub's - docs — so this deliberately listens to `nightly-build.yml` instead of - trying to fix that separately; verified empirically that `ci.yml` alone - has zero push-triggered runs in this repo's history.) -3. On PR builds, it fetches the PR's base branch's persisted baseline (no - rebuild), diffs it against the PR's own size report, and posts/updates a - comment (marker ``). - -**Script:** `.github/scripts/extract-size-report.sh` (size extraction), + completes, `ci-size-report.yml` persists it as TWO release assets in the + companion `iNavFlight/pr-test-builds` repo: `size-baseline-` + (latest-tip pointer, kept for backward compatibility) and + `size-baseline-` (primary — lets PR comparisons key off the + exact base commit). Per-commit baselines are pruned to the newest 50 per + branch (plus a global cap) so the companion repo doesn't grow unbounded. + (`ci.yml`'s *own* `on: push:` trigger is broken — a `branches:` list + containing only a negative pattern matches nothing per GitHub's docs — + so this deliberately listens to `nightly-build.yml` instead of trying to + fix that separately; verified empirically that `ci.yml` alone has zero + push-triggered runs in this repo's history.) +3. On PR builds, it computes the PR's TRUE base commit — the merge-base of + the PR head and base ref via the compare API — fetches the per-commit + baseline for that exact commit, falling back to the nearest ancestor + commit that has one (never the branch tip, which would include unrelated + changes merged after the PR forked), diffs it against the PR's own size + report, and posts/updates a comment (marker ``) + naming the baseline commit that was used. + +**Scripts:** `.github/scripts/extract-size-report.sh` (size extraction), `.github/scripts/merge-size-reports.sh` (merges per-shard reports), +`.github/scripts/publish-size-baseline.sh` (per-commit publish + pruning), +`.github/scripts/fetch-size-baseline.sh` (merge-base baseline resolution), `.github/scripts/size-diff-comment.js` (pure diff + markdown rendering, unit tested in `.github/scripts/size-diff-comment.test.js`) diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index 9f8122bff77..f18e67bdd22 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -12,18 +12,22 @@ name: CI Size Report # runs exist). "Build pre-release" is push-triggered correctly and # already invokes ci.yml's jobs via `uses:` (workflow_call) as part of # building nightly releases — piggybacking here costs nothing extra, -# no second build. Persists the size report as a release asset in the +# no second build. Persists the size report as release assets in the # companion iNavFlight/pr-test-builds repo, so PR runs never need to -# rebuild the base branch to get a comparison point. Only fires for -# branches nightly-build.yml's own push trigger covers (currently -# master, maintenance-9.x, maintenance-10.x, release/9.1 — see that -# file). Stale baselines for since-deleted branches aren't cleaned up -# automatically (same known limitation pr-test-builds has for old PR -# releases). +# rebuild the base branch to get a comparison point. Two assets are +# written per push: size-baseline- (latest-tip pointer, kept +# for backward compatibility) and size-baseline- (primary — +# PR comparisons key off the exact base commit). Per-commit baselines +# are pruned to the newest 50 per branch (plus a global cap) so the +# companion repo doesn't grow unbounded; baselines for since-deleted +# branches aren't otherwise cleaned up (same known limitation +# pr-test-builds has for old PR releases). # - pr-comment listens for "Build firmware" (ci.yml) directly — PR builds # genuinely do trigger it via `pull_request`, that part isn't broken. -# Fetches the persisted baseline, diffs the 4 representative targets, -# and posts/updates a PR comment. +# Resolves the PR's TRUE base commit (merge-base of head and base ref), +# fetches the per-commit baseline for it (falling back to the nearest +# ancestor commit that has one), diffs the 4 representative targets, +# and posts/updates a PR comment showing which baseline was used. # # Requires the same repository secret PR_BUILDS_TOKEN as pr-test-builds.yml # (Contents: write access to iNavFlight/pr-test-builds). @@ -95,24 +99,22 @@ jobs: if: steps.check.outputs.proceed == 'true' env: GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} + COMMIT_SHA: ${{ github.event.workflow_run.head_sha }} run: | BRANCH=$(cat branch.txt) - TAG="size-baseline-${BRANCH}" - # Never delete+recreate the release: that leaves a window where - # the release doesn't exist at all, which a concurrent PR's - # "Fetch base branch baseline" step could hit and misreport as - # "no baseline available yet". Create it once, then only ever - # replace the asset in place (--clobber) on later pushes — the - # release/tag itself stays continuously resolvable. - if gh release view "$TAG" --repo iNavFlight/pr-test-builds >/dev/null 2>&1; then - gh release upload "$TAG" size-report.json --repo iNavFlight/pr-test-builds --clobber - else - gh release create "$TAG" size-report.json \ - --repo iNavFlight/pr-test-builds \ - --prerelease \ - --title "Size baseline: ${BRANCH}" \ - --notes "Latest per-target flash/RAM size report for ${BRANCH}. Auto-updated on every push. Not for human consumption." - fi + # Publishes size-baseline- (latest-tip pointer) and + # size-baseline- (primary), then prunes old per-commit + # baselines. Never delete+recreate the release: that leaves a + # window where the release doesn't exist at all, which a concurrent + # PR's "Fetch base branch baseline" step could hit and misreport as + # "no baseline available yet". Assets are replaced in place + # (--clobber) on later pushes — the release/tag stays continuously + # resolvable. + bash .github/scripts/publish-size-baseline.sh \ + iNavFlight/pr-test-builds \ + "${BRANCH}" \ + "${COMMIT_SHA}" \ + size-report.json pr-comment: runs-on: ubuntu-latest @@ -181,30 +183,49 @@ jobs: echo "base_ref=${BASE_REF}" >> "$GITHUB_OUTPUT" echo "short_sha=${HEAD_SHA:0:7}" >> "$GITHUB_OUTPUT" - - name: Fetch base branch baseline + - name: Compute PR base commit (merge-base) + id: mergebase + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BASE_REF: ${{ steps.pr.outputs.base_ref }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + # The PR's TRUE base commit is the merge-base of the PR head and + # base ref — NOT the base branch's latest tip (comparing against + # the tip includes unrelated changes merged after the PR forked). + # The compare API resolves fork-PR head SHAs too (verified). + # base_ref was already regex-validated in the step above; the + # returned SHA is validated here before any use. + MERGE_BASE=$(gh api "repos/${GITHUB_REPOSITORY}/compare/${BASE_REF}...${HEAD_SHA}" \ + --jq '.merge_base_commit.sha // empty' 2>/dev/null || true) + if [[ "$MERGE_BASE" =~ ^[0-9a-f]{40}$ ]]; then + echo "merge_base=${MERGE_BASE}" >> "$GITHUB_OUTPUT" + else + echo "::warning::could not determine merge-base for ${BASE_REF}...${HEAD_SHA}" + echo "merge_base=" >> "$GITHUB_OUTPUT" + fi + + - name: Fetch size baseline for PR base commit id: baseline env: GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} BASE_REF: ${{ steps.pr.outputs.base_ref }} + MERGE_BASE: ${{ steps.mergebase.outputs.merge_base }} run: | - TAG="size-baseline-${BASE_REF}" mkdir -p baseline - - # publish-baseline replaces the asset in place (--clobber) rather - # than deleting/recreating the release, but an individual asset - # replace still briefly deletes-then-uploads under the hood. A - # few short retries absorb that narrow window instead of a - # concurrent run misreporting "no baseline available yet" for a - # baseline that actually exists. - FOUND=false - for attempt in 1 2 3; do - if gh release download "$TAG" --repo iNavFlight/pr-test-builds --pattern size-report.json --dir baseline 2>/dev/null; then - FOUND=true - break - fi - sleep 3 - done - echo "found=${FOUND}" >> "$GITHUB_OUTPUT" + if [ -n "${MERGE_BASE}" ]; then + # Exact per-commit baseline for the merge-base first, then the + # nearest ancestor commit that has one; the branch-tip baseline + # is deliberately NOT a fallback (stale-delta bug). + bash .github/scripts/fetch-size-baseline.sh \ + "${GITHUB_REPOSITORY}" \ + iNavFlight/pr-test-builds \ + "${BASE_REF}" \ + "${MERGE_BASE}" \ + baseline >> "$GITHUB_OUTPUT" + else + echo "found=false" >> "$GITHUB_OUTPUT" + fi - name: Check for doc on PR head commit id: doc @@ -225,6 +246,8 @@ jobs: PR_NUMBER: ${{ steps.pr.outputs.number }} SHORT_SHA: ${{ steps.pr.outputs.short_sha }} BASELINE_FOUND: ${{ steps.baseline.outputs.found }} + BASELINE_COMMIT: ${{ steps.baseline.outputs.baseline_commit_short }} + BASELINE_EXACT: ${{ steps.baseline.outputs.baseline_exact }} DOC_LINK: ${{ steps.doc.outputs.link }} with: script: | @@ -247,6 +270,8 @@ jobs: prReport, baselineReport, shortSha: process.env.SHORT_SHA, + baselineCommit: process.env.BASELINE_COMMIT || null, + baselineIsNearest: !!process.env.BASELINE_COMMIT && process.env.BASELINE_EXACT !== 'true', docLink: process.env.DOC_LINK || null, marker, }); From dd3c3d0475cc756d7de6e3ca978277b7f64187d0 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Fri, 28 Aug 2026 19:31:42 -0500 Subject: [PATCH 30/38] Make per-commit baseline pruning failures non-fatal Pruning is housekeeping after the baseline publish; a prune failure must not fail the nightly publish job (the baseline itself already landed). Warn loudly instead. --- .github/scripts/publish-size-baseline.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/scripts/publish-size-baseline.sh b/.github/scripts/publish-size-baseline.sh index b409e1759cd..17dda75201b 100644 --- a/.github/scripts/publish-size-baseline.sh +++ b/.github/scripts/publish-size-baseline.sh @@ -127,4 +127,7 @@ prune() { done } -prune +# Pruning is housekeeping: a failure here must not fail the publish (the +# baseline itself already landed above), or the nightly would look broken +# for a cosmetic reason. Warn loudly instead. +prune || echo "::warning::per-commit baseline pruning failed (see stderr)" >&2 From 6a0fb56a7ea782dbe22a6c643b8a55ac921434f2 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Fri, 28 Aug 2026 19:42:15 -0500 Subject: [PATCH 31/38] Fix review findings: branch grouping in pruning, exact-baseline retry, header consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - publish-size-baseline: jq capture of the notes' `branch:` line needed the (?m) flag — without it, ^/$ anchored to the whole (multi-line) string, the capture never matched, and every per-commit baseline collapsed into the '?' bucket, so pruning treated all branches as one and deleted every baseline of the less-active branches (destructive data loss). Pruning now also tracks an explicit set of all tags instead of relying on awk's create-on-reference side effect. - fetch-size-baseline: the exact merge-base baseline is now retried as the first step of the ancestor walk, so a transient download failure on the exact commit degrades to a nearest-ancestor baseline instead of skipping the exact commit for that run. - size-diff-comment: only name the baseline commit in the header when a baseline report actually exists (no contradictory header), and reword the no-baseline note for accuracy during rollout (pre-existing PRs' merge-bases never advance; rebase refreshes them). --- .github/scripts/fetch-size-baseline.sh | 32 ++++++++++++----------- .github/scripts/publish-size-baseline.sh | 24 +++++++++-------- .github/scripts/size-diff-comment.js | 11 +++++--- .github/scripts/size-diff-comment.test.js | 8 +++--- 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/.github/scripts/fetch-size-baseline.sh b/.github/scripts/fetch-size-baseline.sh index 9a6ad4b095f..5802af4f117 100644 --- a/.github/scripts/fetch-size-baseline.sh +++ b/.github/scripts/fetch-size-baseline.sh @@ -55,12 +55,10 @@ mkdir -p "$OUT_DIR" # Set of per-commit baseline SHAs currently stored in the builds repo. # One paginated call; tags are built by publish-size-baseline.sh and match -# the strict 40-hex pattern by construction. @tsv keeps the output raw -# (gh api has no -r/--raw flag; string filters would JSON-quote values). +# the strict 40-hex pattern by construction. list_baseline_shas() { gh api "repos/${BUILDS_REPO}/releases?per_page=100" --paginate \ - --jq '.[] | select(.tag_name | test("^size-baseline-[0-9a-f]{40}$")) | - [.tag_name | sub("^size-baseline-"; "")] | @tsv' + --jq '.[].tag_name | select(test("^size-baseline-[0-9a-f]{40}$")) | sub("^size-baseline-"; "")' } # publish-size-baseline replaces the asset in place (--clobber), which still @@ -87,23 +85,27 @@ emit_found() { # $1 = 40-hex sha, $2 = true|false (exact) BASELINE_SHAS=$(list_baseline_shas) || true -if grep -qx "$MERGE_BASE_SHA" <<< "$BASELINE_SHAS" && download_baseline "$MERGE_BASE_SHA"; then - emit_found "$MERGE_BASE_SHA" true - exit 0 -fi - -# Nearest-ancestor walk along the base branch's first-parent chain. +# Exact merge-base first, then nearest ancestors along the base branch's +# first-parent chain. The loop STARTS at the merge-base itself so a +# transient download failure on the exact commit is retried here instead +# of silently degrading to a nearest-ancestor baseline. sha="$MERGE_BASE_SHA" -for _ in $(seq 1 "$MAX_WALK"); do +first=1 +for _ in $(seq 1 $((MAX_WALK + 1))); do + if grep -qx "$sha" <<< "$BASELINE_SHAS" && download_baseline "$sha"; then + if [ "$first" = 1 ]; then + emit_found "$sha" true + else + emit_found "$sha" false + fi + exit 0 + fi + first=0 parent=$(gh api "repos/${MAIN_REPO}/commits/${sha}" --jq '.parents[0].sha // empty' 2>/dev/null || true) if [ -z "$parent" ] || ! [[ "$parent" =~ ^[0-9a-f]{40}$ ]]; then break fi sha="$parent" - if grep -qx "$sha" <<< "$BASELINE_SHAS" && download_baseline "$sha"; then - emit_found "$sha" false - exit 0 - fi done echo "found=false" diff --git a/.github/scripts/publish-size-baseline.sh b/.github/scripts/publish-size-baseline.sh index 17dda75201b..2029190f7b4 100644 --- a/.github/scripts/publish-size-baseline.sh +++ b/.github/scripts/publish-size-baseline.sh @@ -81,14 +81,15 @@ fi # --------------------------------------------------------------------------- # Emit \t\t for every per-commit baseline release, # newest first (GitHub release listing is newest-first). Branch comes from -# the notes' `branch: ` first line; '?' when unparseable. @tsv keeps -# the output raw (gh api has no -r/--raw flag; string filters would -# JSON-quote values). +# the notes' `branch: ` FIRST LINE; the (?m) flag is required — the +# notes are multi-line, and without it ^/$ anchor to the whole string so +# the capture never matches and every baseline collapses into the '?' +# bucket, which would make pruning treat all branches as one. list_per_commit_baselines() { gh api "repos/${BUILDS_REPO}/releases?per_page=100" --paginate \ --jq '.[] | select(.tag_name | test("^size-baseline-[0-9a-f]{40}$")) | [.created_at, .tag_name, - ((.body // "") | capture("^branch: (?[A-Za-z0-9._/-]+)$") | .b // "?")] | @tsv' + ((.body // "") | capture("(?m)^branch: (?[A-Za-z0-9._/-]+)$") | .b // "?")] | @tsv' } prune() { @@ -96,16 +97,17 @@ prune() { tmp=$(mktemp) trap 'rm -f "$tmp"' RETURN - # Pass 1 (NR==FNR): per-branch keep-set — newest KEEP_PER_BRANCH tags per - # branch. Pass 2: apply the global cap to the kept set in input order. - # Prints the tags to DELETE (kept per-branch but cut by the cap, or never - # in the keep-set at all). The API lists newest-first but pagination can - # interleave pages, so re-sort descending by created_at for a guaranteed - # newest-first input to the keep-selection. + # Pass 1 (NR==FNR): record every tag in `all`, and the per-branch + # keep-set — newest KEEP_PER_BRANCH tags per branch — in `keepTag`. + # Pass 2: apply the global cap to the kept set in input order. + # END: delete every tag in `all` that did not survive to keepFinal. + # (Explicit `all` set so pruning never depends on awk's create-on- + # reference semantics of reading keepTag[$2] in pass 2.) list_per_commit_baselines | sort -r > "$tmp" awk -F '\t' -v keep="$KEEP_PER_BRANCH" -v cap="$GLOBAL_CAP" ' NR == FNR { + all[$2] = 1 if (count[$3] < keep) { count[$3]++; keepTag[$2] = 1 } next } @@ -115,7 +117,7 @@ prune() { } } END { - for (t in keepTag) if (!keepFinal[t]) print t + for (t in all) if (!keepFinal[t]) print t } ' "$tmp" "$tmp" | while read -r tag; do if [ "$DRY_RUN" = "--dry-run" ]; then diff --git a/.github/scripts/size-diff-comment.js b/.github/scripts/size-diff-comment.js index e3b074f2d6a..278036aa970 100644 --- a/.github/scripts/size-diff-comment.js +++ b/.github/scripts/size-diff-comment.js @@ -66,14 +66,19 @@ function diffSizeReports(prReport, baselineReport) { // stored baseline and a nearest-ancestor baseline was used instead. function renderComment({ prReport, baselineReport, shortSha, baselineCommit, baselineIsNearest, docLink, marker }) { const rows = diffSizeReports(prReport, baselineReport); - const vs = baselineCommit ? `vs. base commit \`${baselineCommit}\`` : 'vs. base branch'; + // Only name the baseline commit when there is actually a baseline to + // compare against (the workflow only sets baselineCommit in that case, + // but the renderer must not emit a contradictory header otherwise). + const vs = (baselineReport && baselineCommit) + ? `vs. base commit \`${baselineCommit}\`` : 'vs. base branch'; const lines = [marker, `**RAM / Flash usage ${vs}** — commit \`${shortSha}\``, '']; if (!baselineReport) { lines.push( '> No size baseline is available yet for this PR\'s base commit ' + - '(first run after this feature shipped, or a new branch). ' + - 'This comment will show deltas once a baseline exists.', + '(no per-commit baseline has been published for it). This comment ' + + 'will show deltas once one exists — rebasing the PR refreshes its ' + + 'base commit.', '' ); } else if (baselineIsNearest) { diff --git a/.github/scripts/size-diff-comment.test.js b/.github/scripts/size-diff-comment.test.js index ba00fa3ec55..5b10836e47d 100644 --- a/.github/scripts/size-diff-comment.test.js +++ b/.github/scripts/size-diff-comment.test.js @@ -337,11 +337,11 @@ test('renderComment: baselineCommit omitted -> generic "vs. base branch" wording assert.ok(!body.includes('base commit `'), 'no baseline commit rendered when none supplied'); }); -test('renderComment: baselineCommit supplied but no baseline report -> graceful note still wins', () => { +test('renderComment: baselineCommit supplied but no baseline report -> graceful note wins, no contradictory header', () => { // Defensive: the workflow only sets baselineCommit when a baseline was // found, so this combination should not occur; if it ever does, the - // "no baseline available" note must still be present (never silently - // claim a comparison happened). + // header must NOT claim a base commit was compared (no "vs. base + // commit") and the "no baseline available" note must be present. const body = renderComment({ prReport: fullReport({ MATEKF405: [500000, 60000] }), baselineReport: null, @@ -352,4 +352,6 @@ test('renderComment: baselineCommit supplied but no baseline report -> graceful }); assert.ok(body.includes('No size baseline is available yet')); + assert.ok(!body.includes('vs. base commit `9e932ba`'), 'header must not name a baseline commit when no baseline exists'); + assert.ok(body.includes('vs. base branch'), 'header should fall back to the generic wording'); }); From 87bc1d0ac1c403678ff7fefc8d99e132f48e4262 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sat, 29 Aug 2026 17:56:19 -0500 Subject: [PATCH 32/38] Fix size-baseline publishing: check out repo before calling scripts The publish-baseline job in ci-size-report.yml called .github/scripts/publish-size-baseline.sh without an actions/checkout step, so the runner workspace was empty and the script invocation failed with exit 127. No per-commit baselines were ever published, and every PR's size-diff comment reported "no size baseline is available yet". Add the checkout as the job's first step, and add explicit reminders in the workflow, the two size-baseline scripts, and the workflow README that repo scripts must never be called from a job that has not checked out a known branch first. --- .github/scripts/fetch-size-baseline.sh | 8 ++++++++ .github/scripts/publish-size-baseline.sh | 7 +++++++ .github/workflows/README.md | 7 +++++++ .github/workflows/ci-size-report.yml | 8 ++++++++ 4 files changed, 30 insertions(+) diff --git a/.github/scripts/fetch-size-baseline.sh b/.github/scripts/fetch-size-baseline.sh index 5802af4f117..f2460f6996e 100644 --- a/.github/scripts/fetch-size-baseline.sh +++ b/.github/scripts/fetch-size-baseline.sh @@ -1,5 +1,13 @@ #!/bin/bash # +# WARNING: This script is invoked from a GitHub Actions job via +# `bash .github/scripts/...`. It MUST NEVER be called from a job that has +# not first checked out a known branch (e.g. `- uses: actions/checkout@v4` +# as the job's first step): the runner workspace is empty until then, so +# the script doesn't exist and the call fails with exit 127, silently +# reporting no baseline. Every job calling this script must check out +# first. +# # Fetch the size baseline for a PR's TRUE base commit — the merge-base of # the PR head and base ref — rather than the base branch's latest tip, so # the size-diff delta doesn't include unrelated changes merged to the base diff --git a/.github/scripts/publish-size-baseline.sh b/.github/scripts/publish-size-baseline.sh index 2029190f7b4..28154cbb4d7 100644 --- a/.github/scripts/publish-size-baseline.sh +++ b/.github/scripts/publish-size-baseline.sh @@ -1,5 +1,12 @@ #!/bin/bash # +# WARNING: This script is invoked from a GitHub Actions job via +# `bash .github/scripts/...`. It MUST NEVER be called from a job that has +# not first checked out a known branch (e.g. `- uses: actions/checkout@v4` +# as the job's first step): the runner workspace is empty until then, so +# the script doesn't exist and the call fails with exit 127, silently +# publishing nothing. Every job calling this script must check out first. +# # Publish the nightly size report as release assets in the companion # pr-test-builds repo, keyed by BRANCH (latest-tip pointer, kept for # backward compatibility) AND by COMMIT SHA (primary — lets PR size-diff diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 2d6253d920d..7e28f7cad01 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -103,6 +103,13 @@ every PR. `.github/scripts/size-diff-comment.js` (pure diff + markdown rendering, unit tested in `.github/scripts/size-diff-comment.test.js`) +**⚠️ Every job that calls a `.github/scripts/` file MUST first run +`- uses: actions/checkout@v4` as its first step.** The runner workspace is +empty until a checkout — `bash .github/scripts/...` then fails with exit +127 and the job silently does nothing, so a missing checkout looks like a +missing baseline/artifact instead of a broken job. Repo scripts MUST NEVER +be invoked from a job that has not checked out a known branch first. + **Uses the same `PR_BUILDS_TOKEN` secret and `workflow_run` trigger pattern as `pr-test-builds.yml`** (secrets available even for fork PRs). diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index f18e67bdd22..ee8c76d872d 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -48,6 +48,14 @@ jobs: permissions: actions: read # to download artifacts and query job conclusions from the triggering workflow run steps: + # REQUIRED FIRST STEP: check out a known branch before any step calls + # a script from .github/scripts/. Repo scripts MUST NEVER be invoked + # from a job that has not checked out first: the runner workspace is + # empty until a checkout, so `bash .github/scripts/...` fails with + # exit 127 and the job silently publishes nothing. workflow_run runs + # this file from the default branch, so checkout's default ref is the + # trusted one to use. + - uses: actions/checkout@v4 # Don't gate on github.event.workflow_run.conclusion: "Build # pre-release" also runs a separate Release job (nightly upload to # iNavFlight/inav-nightly) that can fail for reasons unrelated to the From 37bf13a75f5a606b35395295be83618e995e5324 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sat, 29 Aug 2026 18:00:39 -0500 Subject: [PATCH 33/38] Add contents: read to publish-baseline permissions The job now checks out the repo (to run .github/scripts from the workspace), so declare the contents: read that checkout needs, matching the sibling pr-comment job in the same file. --- .github/workflows/ci-size-report.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci-size-report.yml b/.github/workflows/ci-size-report.yml index ee8c76d872d..70bcde57bcd 100644 --- a/.github/workflows/ci-size-report.yml +++ b/.github/workflows/ci-size-report.yml @@ -47,6 +47,7 @@ jobs: cancel-in-progress: true permissions: actions: read # to download artifacts and query job conclusions from the triggering workflow run + contents: read # for the checkout step (scripts live in this repo) steps: # REQUIRED FIRST STEP: check out a known branch before any step calls # a script from .github/scripts/. Repo scripts MUST NEVER be invoked From 7ab25a4c97eced5e5a2e2aaa22ab68a1a3b4e547 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 30 Aug 2026 11:09:25 -0500 Subject: [PATCH 34/38] docs: add terrain-AGL RAM techniques and correct WS2812 buffer figures Rebuild of PR #11827's content change on a clean branch based off release/9.1, preserving exactly the branch's file delta (+45/-2): - Correct the WS2812 DMA buffer figures: the element type is timerDMASafeType_t (uint32_t) on every F4/F7/H7/AT32 family since 2019, so the full 3,115-element buffer is 12,460 B and the 2x4-LED circular design is 768 B, not the previously stated 6,230 B / 384 B. The >16x reduction ratio is unchanged. - Add three generalizable RAM techniques from the PR #11785 (terrain AGL hold) review: budget speculative cache reads against the shared cache's size (lookahead capped at (CACHE_SIZE - 3) * 540 m plus a time horizon); consume an existing cache in place, never a private copy; keep a feature's whole static state in one caller-owned struct (the entire hold state machine is ~100 B of static RAM). --- .../development/ram-and-flash-optimization.md | 47 ++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/docs/development/ram-and-flash-optimization.md b/docs/development/ram-and-flash-optimization.md index 173b08b65d8..e113ac20763 100644 --- a/docs/development/ram-and-flash-optimization.md +++ b/docs/development/ram-and-flash-optimization.md @@ -50,8 +50,8 @@ array. Refill must come from a real hardware DMA interrupt — a task-scheduler tick leaves gaps that read as a protocol reset. Example: the WS2812 driver buffered all 128 possible LEDs in one static -array (6,230 B) for a single DMA burst; a circular buffer holding 2 groups -of 4 LEDs (384 B), refilled from the DMA interrupt as each group finishes, +array (12,460 B) for a single DMA burst; a circular buffer holding 2 groups +of 4 LEDs (768 B), refilled from the DMA interrupt as each group finishes, cut it >16x with no protocol change. ### A minimum-duration constraint doesn't need buffer space @@ -78,6 +78,34 @@ Example: the LED driver processed all 128 possible slots every update; threading `ledCounts.count` into the fill loop and DMA transfer length cut work for short strips. +### Budget speculative reads against the shared cache they consume + +A feature that pre-reads a shared cache (tile cache, lookup tables) must +size its speculative distance against that cache's capacity, not against +what the feature would like to know. Otherwise the speculative consumer can +evict the block the cache's primary consumer needs next, turning the cache +into a thrash loop. Compute the budget from the cache size with margin for +the primary consumer, and additionally cap by a time horizon so the scan +scales with what the aircraft can actually reach. + +Example: PR #11785's terrain lookahead caps its scan at +`(TERRAIN_GRID_BLOCK_CACHE_SIZE - 3) * 540 m` per query cycle — the `-3` +reserves the current block plus margin — and then at `groundspeed * 35 s`. +On an 8-entry cache (F765/H743) that is a 2,700 m ceiling, with the time +horizon usually binding first. The scan can never evict the block the AGL +query reads. + +### Consume an existing cache in place; add no private copy + +When a feature needs data from a cache-backed source, read through the +existing cache and mark misses for loading instead of allocating its own +shadow buffer "for cleanliness". A miss is a `return not-ready`, not a +reason to copy. + +Example: PR #11785's `terrainNavGetHeightAtLocation()` reads the #11438 +terrain grid cache directly and calls `markGridBlockNeedRead()` on a miss; +the whole terrain_nav layer holds zero tile buffers of its own. + ## Sending data ### Send components separately instead of concatenating @@ -154,6 +182,21 @@ Example: PR #11553 added navigation FSM states as mixer-profile transition FSM uses a dense `switch` the compiler can turn into a jump table. +### Keep a feature's whole static state in ONE caller-owned struct + +When a feature's decision logic has many state fields, don't scatter them +as file-scope statics across the module — hold them in one struct that the +caller passes in/out. That makes the feature's RAM cost equal to one +visible `sizeof()`, keeps the core logic pure (unit-testable without +linking the subsystem), and makes reset = one memset-style function +instead of a dozen assignments. + +Example: PR #11785's terrain_nav_hold layer keeps its entire state machine +in `terrainNavHoldState_t` (64 B on MATEKF765, verified via nm) plus one +small output struct — the whole feature's static RAM is ~100 B, not +scattered buffers. `terrain_nav_hold_core.c` is pure logic with zero +file-scope state; `terrain_nav_hold.c` owns the single state struct. + ## Duplication: state vs code ### Distinguish necessary state duplication from avoidable code duplication From 2294bf51b0a0fd9fe8adb36e6c0762a3eabca4a7 Mon Sep 17 00:00:00 2001 From: Sensei Date: Sun, 30 Aug 2026 12:31:32 -0500 Subject: [PATCH 35/38] Update RAM and Flash Optimization documentation Clarified example regarding buffer size requirements for MSP handlers. --- docs/development/ram-and-flash-optimization.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/ram-and-flash-optimization.md b/docs/development/ram-and-flash-optimization.md index e113ac20763..ca5f291c95b 100644 --- a/docs/development/ram-and-flash-optimization.md +++ b/docs/development/ram-and-flash-optimization.md @@ -40,7 +40,7 @@ A narrow audit produces a confident-looking wrong answer. Example: a first pass on the tunnel buffer checked only MSP2 handlers (432 B largest reply); MSP1 legacy handlers reachable through the same -buffer needed 512 B (`MSP_LED_STRIP_CONFIG`, unbounded). +buffer needed 512 B. ### Chunk a large buffer into a circular buffer when the peripheral drains continuously From 6a6a0bdc257709e1489b100e72269234109b4741 Mon Sep 17 00:00:00 2001 From: RunnyCow Date: Mon, 21 Sep 2026 14:46:32 +0800 Subject: [PATCH 36/38] update config setting --- src/main/target/ACCTON_GFF4/config.c | 8 +- src/main/target/ACCTON_GFF4/target.c | 20 +++-- src/main/target/ACCTON_GFF4/target.h | 113 ++++++++++++++------------- 3 files changed, 79 insertions(+), 62 deletions(-) diff --git a/src/main/target/ACCTON_GFF4/config.c b/src/main/target/ACCTON_GFF4/config.c index 08906266f40..4c9b7ca49fa 100644 --- a/src/main/target/ACCTON_GFF4/config.c +++ b/src/main/target/ACCTON_GFF4/config.c @@ -19,11 +19,15 @@ #include "platform.h" -#include "drivers/serial.h" +#include "fc/fc_msp_box.h" + +#include "io/piniobox.h" #include "io/serial.h" void targetConfiguration(void) { - serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART4)].functionMask = FUNCTION_RX_SERIAL; + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART1)].functionMask = FUNCTION_ESCSERIAL; serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART3)].functionMask = FUNCTION_GPS; + serialConfigMutable()->portConfigs[findSerialPortIndexByIdentifier(SERIAL_PORT_USART4)].functionMask = FUNCTION_RX_SERIAL; + pinioBoxConfigMutable()->permanentId[0] = BOX_PERMANENT_ID_USER1; } diff --git a/src/main/target/ACCTON_GFF4/target.c b/src/main/target/ACCTON_GFF4/target.c index 0702b547365..e9075e2c376 100644 --- a/src/main/target/ACCTON_GFF4/target.c +++ b/src/main/target/ACCTON_GFF4/target.c @@ -18,16 +18,24 @@ #include #include "platform.h" + +#include "drivers/bus.h" #include "drivers/io.h" -#include "drivers/timer.h" #include "drivers/pwm_mapping.h" -#include "drivers/bus.h" +#include "drivers/timer.h" timerHardware_t timerHardware[] = { - DEF_TIM(TIM8, CH1, PC6, TIM_USE_OUTPUT_AUTO, 0, 1), // S1 - DMA2_S2_Ch7 - DEF_TIM(TIM8, CH2, PC7, TIM_USE_OUTPUT_AUTO, 0, 1), // S2 - DMA2_S3_Ch7 - DEF_TIM(TIM8, CH3, PC8, TIM_USE_OUTPUT_AUTO, 0, 1), // S3 - DMA2_S4_Ch7 - DEF_TIM(TIM8, CH4, PC9, TIM_USE_OUTPUT_AUTO, 0, 0), // S4 - DMA2_S7_Ch7 + DEF_TIM(TIM8, CH1, PC6, TIM_USE_OUTPUT_AUTO, 0, 1), // M1 DMA2_S2_Ch7 + DEF_TIM(TIM8, CH2, PC7, TIM_USE_OUTPUT_AUTO, 0, 1), // M2 DMA2_S3_Ch7 + DEF_TIM(TIM8, CH3, PC8, TIM_USE_OUTPUT_AUTO, 0, 1), // M3 DMA2_S4_Ch7 + DEF_TIM(TIM8, CH4, PC9, TIM_USE_OUTPUT_AUTO, 0, 0), // M4 DMA2_S7_Ch7 + + DEF_TIM(TIM3, CH1, PB4, TIM_USE_OUTPUT_AUTO, 0, 0), // M5 DMA1_S4_Ch5 + DEF_TIM(TIM3, CH2, PB5, TIM_USE_OUTPUT_AUTO, 0, 0), // M6 DMA1_S5_Ch5 + DEF_TIM(TIM3, CH3, PB0, TIM_USE_OUTPUT_AUTO, 0, 0), // M7 DMA1_S7_Ch5 + DEF_TIM(TIM3, CH4, PB1, TIM_USE_OUTPUT_AUTO, 0, 0), // M8 DMA1_S2_Ch5 + + DEF_TIM(TIM1, CH1, PA8, TIM_USE_LED, 0, 1), // LED strip DMA2_S1_Ch6 }; const int timerHardwareCount = sizeof(timerHardware) / sizeof(timerHardware[0]); diff --git a/src/main/target/ACCTON_GFF4/target.h b/src/main/target/ACCTON_GFF4/target.h index aa2d9b9d0f4..d9c69457f1d 100644 --- a/src/main/target/ACCTON_GFF4/target.h +++ b/src/main/target/ACCTON_GFF4/target.h @@ -23,41 +23,63 @@ #define USBD_PRODUCT_STRING "ACCT" // ******** Board LEDs ********************** -#define LED0 PB14 -#define LED0_INVERTED -#define LED1 PB3 -#define LED1_INVERTED +#define LED0 PB3 +#define LED1 PC4 // ******* Beeper *********** #define BEEPER PB15 #define BEEPER_INVERTED -// ******* GYRO and ACC ******** +// ******* SPI ******** +#define USE_SPI + +#define USE_SPI_DEVICE_1 +#define SPI1_NSS_PIN PA4 +#define SPI1_SCK_PIN PA5 +#define SPI1_MISO_PIN PA6 +#define SPI1_MOSI_PIN PA7 + +#define USE_SPI_DEVICE_2 +#define SPI2_NSS_PIN PB12 +#define SPI2_SCK_PIN PB13 +#define SPI2_MISO_PIN PC2 +#define SPI2_MOSI_PIN PC3 + +#define USE_SPI_DEVICE_3 +#define SPI3_NSS_PIN PA15 +#define SPI3_SCK_PIN PC10 +#define SPI3_MISO_PIN PC11 +#define SPI3_MOSI_PIN PC12 + +// ************ IMU ****************************** #define USE_IMU_ICM42605 #define IMU_ICM42605_ALIGN CW0_DEG #define ICM42605_SPI_BUS BUS_SPI1 #define ICM42605_CS_PIN PA4 +#define GYRO_1_EXTI_PIN PC15 + +// ************** OSD **************************** +#define USE_MAX7456 +#define MAX7456_SPI_BUS BUS_SPI2 +#define MAX7456_CS_PIN PB12 // *************** I2C *************************** #define USE_I2C -#define USE_I2C_DEVICE_1 +#define USE_I2C_DEVICE_1 #define I2C1_SCL PB6 #define I2C1_SDA PB7 -#define DEFAULT_I2C_BUS BUS_I2C1 -// *************** Baro ************************** #define USE_BARO -#define BARO_I2C_BUS DEFAULT_I2C_BUS +#define BARO_I2C_BUS BUS_I2C1 #define USE_BARO_DPS310 #define DPS310_I2C_ADDR (0x77) -// *********** Magnetometer / Compass ************* #define USE_MAG -#define MAG_I2C_BUS DEFAULT_I2C_BUS +#define MAG_I2C_BUS BUS_I2C1 #define USE_MAG_IST8310 -// ******* SERIAL ******** +// *************** Serial ports ******************* #define USE_VCP #define USE_UART1 @@ -76,45 +98,28 @@ #define UART4_TX_PIN PA0 #define UART4_RX_PIN PA1 -#define SERIAL_PORT_COUNT 5 - -// ******* SPI ******** -#define USE_SPI +#define USE_UART5 +#define UART5_RX_PIN PD2 +#define UART5_TX_PIN NONE -#define USE_SPI_DEVICE_1 -#define SPI1_NSS_PIN PA4 -#define SPI1_SCK_PIN PA5 -#define SPI1_MISO_PIN PA6 -#define SPI1_MOSI_PIN PA7 - -#define USE_SPI_DEVICE_2 -#define SPI2_NSS_PIN PB12 -#define SPI2_SCK_PIN PB13 -#define SPI2_MISO_PIN PC2 -#define SPI2_MOSI_PIN PC3 +#define SERIAL_PORT_COUNT 6 -#define USE_SPI_DEVICE_3 -#define SPI3_NSS_PIN PA15 -#define SPI3_SCK_PIN PC10 -#define SPI3_MISO_PIN PC11 -#define SPI3_MOSI_PIN PC12 +#define DEFAULT_RX_TYPE RX_TYPE_SERIAL +#define SERIALRX_UART SERIAL_PORT_USART4 +#define SERIALRX_PROVIDER SERIALRX_CRSF -// ******* ADC ******** +// *************** ADC **************************** #define USE_ADC -#define ADC_CHANNEL_1_PIN PC4 -#define ADC_CHANNEL_2_PIN PC5 +#define ADC_INSTANCE ADC1 +#define ADC_CHANNEL_1_PIN PC0 +#define ADC_CHANNEL_2_PIN PC1 #define VBAT_ADC_CHANNEL ADC_CHN_1 #define CURRENT_METER_ADC_CHANNEL ADC_CHN_2 -#define VBAT_SCALE_DEFAULT 841 -#define CURRENT_METER_SCALE 250 - -// ******* OSD ******** -#define USE_OSD -#define USE_MAX7456 -#define MAX7456_SPI_BUS BUS_SPI2 -#define MAX7456_CS_PIN PB12 +// *************** LED strip ********************** +#define USE_LED_STRIP +#define WS2811_PIN PA8 // ******* FLASH ******** #define USE_FLASHFS @@ -124,21 +129,21 @@ #define ENABLE_BLACKBOX_LOGGING_ON_SPIFLASH_BY_DEFAULT // ******* FEATURES ******** -#define DEFAULT_RX_TYPE RX_TYPE_SERIAL -#define SERIALRX_UART SERIAL_PORT_USART4 -#define SERIALRX_PROVIDER SERIALRX_CRSF - -#define DEFAULT_FEATURES (FEATURE_OSD | FEATURE_TELEMETRY | FEATURE_CURRENT_METER | FEATURE_VBAT | FEATURE_TX_PROF_SEL) +#define TARGET_IO_PORTA (0xffff & ~(BIT(13) | BIT(14))) +#define TARGET_IO_PORTB 0xffff +#define TARGET_IO_PORTC 0xffff +#define TARGET_IO_PORTD 0xffff -#define TARGET_IO_PORTA 0xffff -#define TARGET_IO_PORTB 0xffff -#define TARGET_IO_PORTC 0xffff -#define TARGET_IO_PORTD (BIT(2)) +#define MAX_PWM_OUTPUT_PORTS 8 -#define MAX_PWM_OUTPUT_PORTS 4 +#define DEFAULT_FEATURES (FEATURE_OSD | FEATURE_TELEMETRY | FEATURE_CURRENT_METER | FEATURE_VBAT | FEATURE_TX_PROF_SEL | FEATURE_BLACKBOX) // ESC-related features #define USE_DSHOT -#define USE_SERIALSHOT #define USE_ESC_SENSOR #define USE_SERIAL_4WAY_BLHELI_INTERFACE + +// *************** VTX ************ +#define USE_PINIO +#define USE_PINIOBOX +#define PINIO1_PIN PB2 // 9V_EN Switch From fd273c966dfb43fa03815ac85d6951097d9fcea3 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Thu, 15 Oct 2026 19:05:18 -0500 Subject: [PATCH 37/38] ACCTON_GFF4: name the ICM42605 interrupt pin the way the gyro driver reads it The board registers its ICM42605 through common_hardware.c, which takes the interrupt pin from ICM42605_EXTI_PIN. GYRO_1_EXTI_PIN is not read for this gyro, so the pin was ignored and the gyro had no interrupt line. --- src/main/target/ACCTON_GFF4/target.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/target/ACCTON_GFF4/target.h b/src/main/target/ACCTON_GFF4/target.h index d9c69457f1d..c0553891804 100644 --- a/src/main/target/ACCTON_GFF4/target.h +++ b/src/main/target/ACCTON_GFF4/target.h @@ -56,7 +56,7 @@ #define IMU_ICM42605_ALIGN CW0_DEG #define ICM42605_SPI_BUS BUS_SPI1 #define ICM42605_CS_PIN PA4 -#define GYRO_1_EXTI_PIN PC15 +#define ICM42605_EXTI_PIN PC15 // ************** OSD **************************** #define USE_MAX7456 From db7a93872bd86489f0e5910ee7aa24f1aba13496 Mon Sep 17 00:00:00 2001 From: RunnyCow Date: Tue, 20 Oct 2026 10:01:26 +0800 Subject: [PATCH 38/38] add cleanup script step with GH_TOKEN authentication --- .github/workflows/pr-test-builds.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/pr-test-builds.yml b/.github/workflows/pr-test-builds.yml index f969f40218c..64eccbdbc0c 100644 --- a/.github/workflows/pr-test-builds.yml +++ b/.github/workflows/pr-test-builds.yml @@ -149,3 +149,12 @@ jobs: body, }); } + + - name: Checkout code (for cleanup script) + uses: actions/checkout@v4 + + - name: Run cleanup script + env: + GH_TOKEN: ${{ secrets.PR_BUILDS_TOKEN }} + run: | + python3 .github/scripts/cleanup-old-pr-test-builds.py --older-than 14