Method for adding defaults using defines for resources. #12342

blckmn · 2023-02-09T23:57:25Z

As we will always be using the fullTimerHardware, we just need to configure the pin mapping.

This is done in the config.h as

#define TIMER_PIN_MAPPING       \
    TIMER_PIN_MAP(0, PA0, 1, 0) \
    TIMER_PIN_MAP(1, PA1, 1, 0)

Old timerHardware[] dependencies (that no longer exist as there are no target.c files - other than SITL - with them present) to be removed in another PR.

blckmn · 2023-02-10T02:17:20Z

AUTOMERGE: (FAIL)

github identifies PR as mergeable -> PASS
assigned to a milestone -> PASS
cooling off period lapsed -> PASS
commit count less or equal to three -> FAIL
Don't merge label NOT found -> PASS
at least one RN: label found -> PASS
Tested label found -> FAIL
assigned to an approver -> PASS
approver count at least three -> PASS

…figure the pin mapping. This is done in the config.h as #define TIMER_PIN_MAPPING \ TIMER_PIN_MAP(0, PA0, 1, 0) \ TIMER_PIN_MAP(0, PA1, 1, 0) timerHardware[] dependencies to be removed in another PR

haslinghuis · 2023-02-10T23:49:52Z

src/main/pg/timerio.c

@@ -29,33 +29,15 @@

 #include "timerio.h"

+#define TIMER_PIN_MAP(idx, pin, pos, dmaOpt)  \
+        { config[idx].ioTag = IO_TAG(pin); config[idx].index = pos; config[idx].dmaopt = dmaOpt; }
+
 PG_REGISTER_ARRAY_WITH_RESET_FN(timerIOConfig_t, MAX_TIMER_PINMAP_COUNT, timerIOConfig, PG_TIMER_IO_CONFIG, 0);


Do we need to raise version?

Not sure what you mean?

src/main/pg/timerio.c

src/main/fc/init.c

SteveCEvans

This is simpler than what we had.

src/main/pg/rx_pwm.c

src/main/pg/motor.c

github-actions · 2023-02-11T20:55:56Z

Do you want to test this code? Here you have an automated build:
Assets
WARNING: It may be unstable. Use only for testing! See: https://www.youtube.com/watch?v=I1uN9CN30gw for instructions for unified targets!

hydra · 2023-02-13T13:16:52Z

src/main/pg/motor.c

+/*
+  NOTE as we predominantly build for quads, the default motor pin defines is 4,
+  add more if a specific configuration ever requires it.
+*/


MAX_SUPPORTED_MOTORS is 8. This should support the same amount of motor pins.

Also, all SPRacing FC's always have 8 motor outputs.

hydra · 2023-02-13T13:47:28Z

#define TIMER_PIN_MAPPING       \
    TIMER_PIN_MAP(0, PA0, 1, 0) \
    TIMER_PIN_MAP(1, PA1, 1, 0)

Can you confirm that the above example is incorrect and that the 3rd argument is the index into the array of fullTimerHardware for the MCU +1, it's possible that I'm reading the new and old code in the diff incorrectly while trying to understand how to use this new macro.

e.g. Given the fullTimerHardware for the H7 is as follows:

const timerHardware_t fullTimerHardware[FULL_TIMER_CHANNEL_COUNT] = {
    DEF_TIM(TIM2, CH1, PA0, TIM_USE_ANY, 0, 0, 0),
    DEF_TIM(TIM2, CH2, PA1, TIM_USE_ANY, 0, 0, 0),
    ...
}

The above example should be corrected to:

#define TIMER_PIN_MAPPING       \
    TIMER_PIN_MAP(0, PA0, 1, 0) \
    TIMER_PIN_MAP(1, PA1, 2, 0)

which would correspond to:

0: Pin = PA0, Timer = TIM2, Channel = CH1, DMA Option = 0,
1: Pin = PA1, Timer = TIM2, Channel = CH1, DMA Option = 0,

yes/no?

IMHO:

In the TIMER_PIN_MAPPING macro pos is
a) functionally opaque.
b) badly named.
c) should probably be renamed to 'fullTimerIndexPosition' via another PR.

additionally,

the macro's use should be documented in the code better.
this PR didn't document mention any wiki page changes where a how-to should probably have been created.
a search for MOTOR1_PIN on the BF wiki or the unified targets repo does not turn up any results, is documentation going to be created for this, and the other similar macros, or does everyone have to trawl the code and PR/issues?

hydra · 2023-02-13T15:08:22Z

referencing both the example in the original post and my comment above, and these files:

https://github.com/betaflight/betaflight/blob/4.3-maintenance/src/main/target/SPRACINGH7EXTREME/target.h
https://github.com/betaflight/betaflight/blob/4.3-maintenance/src/main/target/SPRACINGH7EXTREME/target.c
https://github.com/betaflight/unified-targets/pull/758/files#diff-798f98249c0f7fb35f321785866c6a48138c47113ea3ffcb476e1b8c0cd5c568
https://github.com/spracing/betaflight-targets/blob/828b1339915f4ece7b58d0cf515aea3bbaf11b08/configs/default/SPRO-SPRACINGH7EXTREME.config

I tried this:

#define TIMER_PIN_MAPPING \
    TIMER_PIN_MAP(0, PA7, 6, 0) \
    TIMER_PIN_MAP(1, PB11, 33, 0) \
    TIMER_PIN_MAP(2, PB15, 46, 0) \
    TIMER_PIN_MAP(3, PE5, 67, 0) \
    TIMER_PIN_MAP(4, PE6, 68, 0) \
    TIMER_PIN_MAP(5, PA0, 12, 0) \
    TIMER_PIN_MAP(6, PA1, 13, 0) \
    TIMER_PIN_MAP(7, PA2, 14, 0) \
    TIMER_PIN_MAP(8, PA3, 15, 0) \
    TIMER_PIN_MAP(9, PB6, 41, 1) \
    TIMER_PIN_MAP(10, PB7, 42, 1) \
    TIMER_PIN_MAP(11, PC6, 51, 2) \
    TIMER_PIN_MAP(12, PC7, 52, 2) \
    TIMER_PIN_MAP(13, PD14, 57, 0) \
    TIMER_PIN_MAP(14, PD15, 58, 0) \
    TIMER_PIN_MAP(15, PA6, 16, 0) \
    TIMER_PIN_MAP(16, PA7, 17, 0) \
    TIMER_PIN_MAP(17, PB0, 18, 0) \
    TIMER_PIN_MAP(18, PB1, 19, 0)

Which resulted in this:

# timer
timer A07 AF1
# pin A07: TIM0 CH27 (AF1)
timer B11 AF1
# pin B11: TIM0 CH27 (AF1)
timer B15 AF1
# pin B15: TIM0 CH27 (AF1)
timer E05 AF1
# pin E05: TIM0 CH27 (AF1)
timer E06 AF1
# pin E06: TIM0 CH27 (AF1)
timer A00 AF1
# pin A00: TIM0 CH27 (AF1)
timer A01 AF1
# pin A01: TIM0 CH27 (AF1)
timer A02 AF1
# pin A02: TIM0 CH27 (AF1)
timer A03 AF1
# pin A03: TIM0 CH27 (AF1)
timer B06 AF1
# pin B06: TIM0 CH27 (AF1)
timer B07 AF1
# pin B07: TIM0 CH27 (AF1)
timer C06 AF1
# pin C06: TIM0 CH27 (AF1)
timer C07 AF1
# pin C07: TIM0 CH27 (AF1)
timer D14 AF1
# pin D14: TIM0 CH27 (AF1)
timer D15 AF1
# pin D15: TIM0 CH27 (AF1)
timer A06 AF1
# pin A06: TIM0 CH27 (AF1)
timer A07 AF1
# pin A07: TIM0 CH27 (AF1)
timer B00 AF1
# pin B00: TIM0 CH27 (AF1)
timer B01 AF1
# pin B01: TIM0 CH27 (AF1)

# timer show

Currently active Timers:
-----------------------
TIM1: FREE
TIM2: FREE
TIM3: FREE
TIM4: FREE
TIM5: FREE
TIM6: FREE
TIM7: FREE
TIM8: FREE
TIM12: FREE
TIM13: FREE
TIM14: FREE
TIM15: FREE
TIM16: FREE
TIM17: FREE

# get motor_pwm_protocol
motor_pwm_protocol = ONESHOT125
Allowed values: PWM, ONESHOT125, ONESHOT42, MULTISHOT, BRUSHED, DSHOT150, DSHOT300, DSHOT600, PROSHOT1000, DISABLED
Default value: DISABLED

Note that TIM0 doesn't exist, and channel 27 is erroneous.

Obviously, that resulted in total failure...

I also tried used the 1 based index for 'pos' based on the example in the OP, but the result was similar.

#define TIMER_PIN_MAPPING \
    TIMER_PIN_MAP(0, PA7, 1, 0) \
    TIMER_PIN_MAP(1, PB11, 2, 0) \
    TIMER_PIN_MAP(2, PB15, 3, 0) \
    TIMER_PIN_MAP(3, PE5, 4, 0) \
    TIMER_PIN_MAP(4, PE6, 5, 0) \
    TIMER_PIN_MAP(5, PA0, 6, 0) \
    TIMER_PIN_MAP(6, PA1, 7, 0) \
    TIMER_PIN_MAP(7, PA2, 8, 0) \
    TIMER_PIN_MAP(8, PA3, 9, 0) \
    TIMER_PIN_MAP(9, PB6, 10, 1) \
    TIMER_PIN_MAP(10, PB7, 11, 1) \
    TIMER_PIN_MAP(11, PC6, 12, 2) \
    TIMER_PIN_MAP(12, PC7, 13, 2) \
    TIMER_PIN_MAP(13, PD14, 14, 0) \
    TIMER_PIN_MAP(14, PD15, 15, 0) \
    TIMER_PIN_MAP(15, PA6, 16, 0) \
    TIMER_PIN_MAP(16, PA7, 17, 0) \
    TIMER_PIN_MAP(17, PB0, 18, 0) \
    TIMER_PIN_MAP(18, PB1, 19, 0)

# timer
timer A07 AF1
# pin A07: TIM1 CH1N (AF1)
timer B11 AF1
# pin B11: TIM0 CH27 (AF1)
timer B15 AF1
# pin B15: TIM0 CH27 (AF1)
timer E05 AF1
# pin E05: TIM0 CH27 (AF1)
timer E06 AF1
# pin E06: TIM0 CH27 (AF1)
timer A00 AF1
# pin A00: TIM0 CH27 (AF1)
timer A01 AF1
# pin A01: TIM0 CH27 (AF1)
timer A02 AF1
# pin A02: TIM0 CH27 (AF1)
timer A03 AF1
# pin A03: TIM0 CH27 (AF1)
timer B06 AF1
# pin B06: TIM0 CH27 (AF1)
timer B07 AF1
# pin B07: TIM0 CH27 (AF1)
timer C06 AF1
# pin C06: TIM0 CH27 (AF1)
timer C07 AF1
# pin C07: TIM0 CH27 (AF1)
timer D14 AF1
# pin D14: TIM0 CH27 (AF1)
timer D15 AF1
# pin D15: TIM0 CH27 (AF1)
timer A06 AF1
# pin A06: TIM0 CH27 (AF1)
timer A07 AF1
# pin A07: TIM0 CH27 (AF1)
timer B00 AF1
# pin B00: TIM0 CH27 (AF1)
timer B01 AF1
# pin B01: TIM0 CH27 (AF1)

Here's the config.h file, so the others can try it.

/*
 * This file is part of Betaflight.
 *
 * Betaflight is free software. You can redistribute this software
 * and/or modify this software 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.
 *
 * Betaflight 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 software.
 *
 * If not, see <http://www.gnu.org/licenses/>.
 */

/*
   This file has been auto generated from unified-targets repo.

   The auto generation is transitional only.
*/

#define FC_TARGET_MCU     STM32H750

#define BOARD_NAME        SPRACINGH7EXTREME
#define MANUFACTURER_ID   SPRO

#define TARGET_BOARD_IDENTIFIER "SP7E"
#define USBD_PRODUCT_STRING "SPRacingH7EXTREME"
#define EEPROM_SIZE 8192
#define USE_SPRACING_PERSISTENT_RTC_WORKAROUND
#define USE_BUTTONS
#define BUTTON_A_PIN            PE4
#define BUTTON_A_PIN_INVERTED
#define BUTTON_B_PIN            PE4
#define BUTTON_B_PIN_INVERTED
#define USE_QUADSPI
#define USE_QUADSPI_DEVICE_1
#define QUADSPI1_SCK_PIN PB2
#define QUADSPI1_BK1_IO0_PIN PD11
#define QUADSPI1_BK1_IO1_PIN PD12
#define QUADSPI1_BK1_IO2_PIN PE2
#define QUADSPI1_BK1_IO3_PIN PD13
#define QUADSPI1_BK1_CS_PIN PB10
#define QUADSPI1_BK2_IO0_PIN PE7
#define QUADSPI1_BK2_IO1_PIN PE8
#define QUADSPI1_BK2_IO2_PIN PE9
#define QUADSPI1_BK2_IO3_PIN PE10
#define QUADSPI1_BK2_CS_PIN NONE
#define QUADSPI1_MODE QUADSPI_MODE_BK1_ONLY
#define QUADSPI1_CS_FLAGS (QUADSPI_BK1_CS_HARDWARE | QUADSPI_BK2_CS_NONE | QUADSPI_CS_MODE_LINKED)
#define ENABLE_BLACKBOX_LOGGING_ON_SDCARD_BY_DEFAULT
#define FLASH_QUADSPI_INSTANCE    QUADSPI
#define CONFIG_IN_EXTERNAL_FLASH
#define SDCARD_DETECT_PIN PD10
#define SDCARD_DETECT_INVERTED
#define SDIO_DEVICE             SDIODEV_1
#define SDIO_USE_4BIT           1
#define SDIO_CK_PIN             PC12
#define SDIO_CMD_PIN            PD2
#define SDIO_D0_PIN             PC8
#define SDIO_D1_PIN             PC9
#define SDIO_D2_PIN             PC10
#define SDIO_D3_PIN             PC11
#define USE_SPI
#define USE_SPI_DEVICE_2
#define SPI2_SCK_PIN            PD3
#define SPI2_MISO_PIN           PC2
#define SPI2_MOSI_PIN           PC3
#define SPI2_NSS_PIN            PB12
#define USE_SPI_DEVICE_3
#define SPI3_SCK_PIN            PB3
#define SPI3_MISO_PIN           PB4
#define SPI3_MOSI_PIN           PD6
#define SPI3_NSS_PIN            PA15
#define USE_SPI_DEVICE_4
#define SPI4_SCK_PIN            PE12
#define SPI4_MISO_PIN           PE13
#define SPI4_MOSI_PIN           PE14
#define SPI4_NSS_PIN            PE11
#define USE_USB_ID
#define USE_I2C
#define USE_I2C_DEVICE_1
#define I2C1_SCL PB8
#define I2C1_SDA PB9
#define I2C_DEVICE (I2CDEV_1)
#define ENSURE_MPU_DATA_READY_IS_LOW
#define USE_PID_AUDIO
#define VTX_RTC6705_OPTIONAL
#define ADC1_DMA_OPT 8
#define ADC3_DMA_OPT 9
#define USE_ACC_SPI_MPU6500
#define USE_GYRO_SPI_MPU6500
#define USE_BARO_BMP388
#define USE_MAG_HMC5883
#define USE_MAG_QMC5883
#define USE_FLASH_W25N01G
#define USE_SDCARD
#define USE_CAMERA_CONTROL
#define USE_MAX7456

#define LED0_PIN                PE3
#define BEEPER_PIN              PD7

#define USE_UART1
#define UART1_RX_PIN            PB15
#define UART1_TX_PIN            PB14
#define USE_UART2
#define UART2_RX_PIN            NONE
#define UART2_TX_PIN            PD5   // TX pin is bidirectional.
#define USE_UART3
#define UART3_RX_PIN            PD9
#define UART3_TX_PIN            PD8
#define USE_UART4
#define UART4_RX_PIN            PD0
#define UART4_TX_PIN            PD1
#define USE_UART5
#define UART5_RX_PIN            PB5
#define UART5_TX_PIN            PB13
#define USE_UART6
#define UART6_RX_PIN            PC7 // aka M7
#define UART6_TX_PIN            PC6 // aka M8
#define USE_UART8
#define UART8_RX_PIN            PE0
#define UART8_TX_PIN            PE1

#define USE_SPI_GYRO

#define GYRO_1_EXTI_PIN         PD4
#define GYRO_1_CS_PIN           SPI3_NSS_PIN
#define GYRO_1_SPI_INSTANCE     SPI3
#define GYRO_1_ALIGN            CW180_DEG

#define GYRO_2_EXTI_PIN         PE15
#define GYRO_2_CS_PIN           SPI2_NSS_PIN
#define GYRO_2_SPI_INSTANCE     SPI2
#define GYRO_2_ALIGN            ALIGN_CUSTOM
#define GYRO_2_CUSTOM_ALIGN     SENSOR_ALIGNMENT(  0, 0, 225)

#define MAX7456_SPI_INSTANCE    SPI4
#define MAX7456_SPI_CS_PIN      SPI4_NSS_PIN

#define RSSI_ADC_PIN            PC4
#define VBAT_ADC_PIN            PC1
#define CURRENT_METER_ADC_PIN   PC0
#define EXTERNAL1_ADC_PIN       PC5

#define CURRENT_METER_SCALE_DEFAULT         225

#define DEFAULT_VOLTAGE_METER_SOURCE VOLTAGE_METER_ADC
#define DEFAULT_CURRENT_METER_SOURCE CURRENT_METER_ADC

#define ENABLE_BLACKBOX_LOGGING_ON_SDCARD_BY_DEFAULT

#define DEFAULT_RX_FEATURE      FEATURE_RX_SERIAL
#define DEFAULT_FEATURES        (FEATURE_TRANSPONDER | FEATURE_RSSI_ADC | FEATURE_TELEMETRY | FEATURE_OSD | FEATURE_LED_STRIP)

#define TIMER_PIN_MAPPING \
    TIMER_PIN_MAP(0, PA7, 6, 0) \
    TIMER_PIN_MAP(1, PB11, 33, 0) \
    TIMER_PIN_MAP(2, PB15, 46, 0) \
    TIMER_PIN_MAP(3, PE5, 67, 0) \
    TIMER_PIN_MAP(4, PE6, 68, 0) \
    TIMER_PIN_MAP(5, PA0, 12, 0) \
    TIMER_PIN_MAP(6, PA1, 13, 0) \
    TIMER_PIN_MAP(7, PA2, 14, 0) \
    TIMER_PIN_MAP(8, PA3, 15, 0) \
    TIMER_PIN_MAP(9, PB6, 41, 1) \
    TIMER_PIN_MAP(10, PB7, 42, 1) \
    TIMER_PIN_MAP(11, PC6, 51, 2) \
    TIMER_PIN_MAP(12, PC7, 52, 2) \
    TIMER_PIN_MAP(13, PD14, 57, 0) \
    TIMER_PIN_MAP(14, PD15, 58, 0) \
    TIMER_PIN_MAP(15, PA6, 16, 0) \
    TIMER_PIN_MAP(16, PA7, 17, 0) \
    TIMER_PIN_MAP(17, PB0, 18, 0) \
    TIMER_PIN_MAP(18, PB1, 19, 0)

#define LED_STRIP_PIN PA8
#define TRANSPONDER_PIN PB11
#define RX_PPM_PIN PB15
#define CAMERA_CONTROL_PIN PE5
#define MOTOR1_PIN PA0
#define MOTOR2_PIN PA1
#define MOTOR3_PIN PA2
#define MOTOR4_PIN PA3
#define MOTOR5_PIN PB6
#define MOTOR6_PIN PB7
#define MOTOR7_PIN PC6
#define MOTOR8_PIN PC7
#define MOTOR9_PIN PD14
#define MOTOR10_PIN PD15
#define SERVO1_PIN PA6
#define SERVO2_PIN PA7
#define SERVO3_PIN PB0
#define SERVO4_PIN PB1

Is there something I'm missing here?

How was this PR tested? @haslinghuis @blckmn for PR's like this, can you ensure that they are tested and that working examples are given that others are able to replicate /prior/ to merging them, as is normally the case with PRs.

ledvinap · 2023-02-14T14:48:47Z

src/main/drivers/camera_control.c

-    cameraControlConfig->ioTag = timerioTagGetByUsage(TIM_USE_CAMERA_CONTROL, 0);
+#ifdef CAMERA_CONTROL_PIN
+    cameraControlConfig->ioTag = IO_TAG(CAMERA_CONTROL_PIN);
+#else


It is possible to use (black) macro magic so that CAMERA_CONTROL_PIN will expand to tag if defined (including NONE), and to NONE when undefined. It may also check that TAG definition is valid.
It may make this code more readable ..

ping me if you think it is good idea ..

ping @blckmn ☝️

@ledvinap absolutely! You are the king of macros, so happy for you to show how it'd work.

…2342) * Method for adding defaults using defines for resources. * As we will always be using the fullTimerHardware, we just need to configure the pin mapping. This is done in the config.h as #define TIMER_PIN_MAPPING \ TIMER_PIN_MAP(0, PA0, 1, 0) \ TIMER_PIN_MAP(0, PA1, 1, 0) timerHardware[] dependencies to be removed in another PR * Adding missing pin definitions (removing dependency on timerHardware) * Typo * In case MOTOR1_PIN is not defined, but a motor is configured