Skip to content

Conversation

@sensei-hacker
Copy link
Member

@sensei-hacker sensei-hacker commented Apr 6, 2025

User description

Support for Brother Hobby H743, being handled as per the new hardware policy.

  • Samples received
  • Flash firmware
  • Calibrate
  • Orientation matches
  • Gyro working
  • Accel working
  • Baro working
  • Voltage correct
  • Current correct
  • Mag I2C Bus
  • Additional I2C2 Buses (Airspeed/other accessories)
  • UART1
  • UART2
  • UART3
  • UART4
  • UART6
  • UART7
  • UART8
  • Analog Camera working
  • Video Out working
  • OSD working
  • LEDs working
  • Buzzer working
  • Motor outputs
  • DShot support on m1-4
  • Servo outputs
  • Blackbox
  • PINIO1
  • PINIO2

PR Type

Other


Description

This description is generated by an AI tool. It may have inaccuracies

  • Add support for Brother Hobby H743 flight controller

  • Configure dual ICM42605 IMU sensors on SPI buses

  • Setup 8 UART ports and I2C peripherals

  • Enable OSD, blackbox, and LED strip features


Diagram Walkthrough

flowchart LR
  A["New Target"] --> B["Hardware Config"]
  B --> C["Dual IMU Setup"]
  B --> D["UART/I2C Config"]
  B --> E["OSD/Blackbox"]
  C --> F["ICM42605 SPI1/SPI4"]
  D --> G["8 Serial Ports"]
  E --> H["MAX7456 OSD"]
Loading

File Walkthrough

Relevant files
New target
target.h
Complete hardware configuration for H743 target                   

src/main/target/BROTHERHOBBYH743/target.h

  • Define hardware pins and peripherals for H743 board
  • Configure dual ICM42605 IMU sensors on SPI1/SPI4
  • Setup 8 UART ports, I2C buses, ADC channels
  • Enable OSD, blackbox, LED strip, and PINIO features
+187/-0 
target.c
Hardware device registration and timer setup                         

src/main/target/BROTHERHOBBYH743/target.c

  • Register dual ICM42605 IMU devices on SPI buses
  • Define timer hardware for 12 motor/servo outputs
  • Configure LED strip and beeper PWM timers
+53/-0   
config.c
Target-specific configuration settings                                     

src/main/target/BROTHERHOBBYH743/config.c

  • Configure PINIO box permanent IDs for user controls
  • Enable PWM mode for beeper configuration
+32/-0   
CMakeLists.txt
CMake build configuration                                                               

src/main/target/BROTHERHOBBYH743/CMakeLists.txt

  • Add CMake build target for STM32H743XI processor
+1/-0     

@sensei-hacker
Copy link
Member Author

sensei-hacker commented Apr 7, 2025

After correcting the i2c bus for the pitot and temperature sensors, the remaining issue is the camera inputs.
I see no video, only OSD over a black background, for both C1 and C2. I tested with USER1 and USER2 on and off.
I tested with two different cameras.

@sensei-hacker sensei-hacker marked this pull request as draft May 26, 2025 04:27
@sensei-hacker sensei-hacker added the New target This PR adds a new target label Jun 18, 2025
@sensei-hacker
Copy link
Member Author

Camera now working

@sensei-hacker sensei-hacker self-assigned this Aug 30, 2025
@sensei-hacker sensei-hacker marked this pull request as ready for review August 30, 2025 18:27
@sensei-hacker
Copy link
Member Author

/analyze

@qodo-code-review
Copy link
Contributor

The analyze command only supports the following languages: python, py, javascript, js, jsx, typescript, ts, tsx, kotlin, kt, kts, go, java, cpp, c++, cs, c#, csharp

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
⚡ Recommended focus areas for review

Duplicate Macro

USE_IMU_ICM42605 is defined twice for dual IMU; confirm the build system or headers tolerate repeated defines or guard with conditional checks to avoid redefinition conflicts across compilers.

#define USE_IMU_ICM42605

#define IMU_ICM42605_ALIGN      CW90_DEG
#define ICM42605_SPI_BUS        BUS_SPI1
#define ICM42605_CS_PIN         PC15

// *************** SPI4 IMU1 ICM42605 **************
#define USE_SPI_DEVICE_4
#define SPI4_SCK_PIN            PE12
#define SPI4_MISO_PIN           PE13
#define SPI4_MOSI_PIN           PE14

#define USE_IMU_ICM42605
Timer DMA Indexing

Timer hardware entries use mixed DMA indices including zeros and comments indicating DMA_NONE; verify the DMA indices and flags match actual DMA availability to avoid LED/beeper/pwm glitches or resource conflicts.

    DEF_TIM(TIM4, CH1, PD12, TIM_USE_OUTPUT_AUTO, 0, 6),   // S7
    DEF_TIM(TIM4, CH2, PD13, TIM_USE_OUTPUT_AUTO, 0, 7),   // S8
    DEF_TIM(TIM4, CH3, PD14, TIM_USE_OUTPUT_AUTO, 0, 0),   // S9
    DEF_TIM(TIM4, CH4, PD15, TIM_USE_OUTPUT_AUTO, 0, 0),   // S10 DMA_NONE

    DEF_TIM(TIM15, CH1, PE5, TIM_USE_OUTPUT_AUTO, 0, 0),   // S11
    DEF_TIM(TIM15, CH2, PE6, TIM_USE_OUTPUT_AUTO, 0, 0),   // S12 DMA_NONE

    DEF_TIM(TIM1, CH1, PA8, TIM_USE_LED, 0, 9),    // LED_2812
    DEF_TIM(TIM2, CH1, PA15, TIM_USE_BEEPER, 0, 0),  // BEEPER PWM
};
Config Defaults

Sets PINIO permanent IDs and enables beeper PWM by default; ensure these defaults align with user expectations and don’t conflict with existing box IDs or board beeper polarity/inversion.

void targetConfiguration(void)
{
    pinioBoxConfigMutable()->permanentId[0] = BOX_PERMANENT_ID_USER1;
    pinioBoxConfigMutable()->permanentId[1] = BOX_PERMANENT_ID_USER2;
    beeperConfigMutable()->pwmMode = true;

@sensei-hacker sensei-hacker merged commit 47e0527 into iNavFlight:master Aug 30, 2025
21 checks passed
@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Add IMU data-ready interrupts

Both ICM42605 registrations use NONE for the EXTI/data-ready line, which forces
polled sampling and can cause timing jitter, aliasing, and desynchronization
between the dual IMUs on H7. Expose and define the DRDY pins for each IMU in
target.h and pass the corresponding EXTI tags to BUSDEV_REGISTER_SPI_TAG so the
driver can run interrupt-driven sampling. This will materially improve gyro
timing, filter behavior, and overall stability, especially with dual sensors.

Examples:

src/main/target/BROTHERHOBBYH743/target.c [29-30]
BUSDEV_REGISTER_SPI_TAG(busdev_icm42605,   DEVHW_ICM42605,  ICM42605_SPI_BUS,   ICM42605_CS_PIN,   NONE,   0,  DEVFLAGS_NONE,  IMU_ICM42605_ALIGN);
BUSDEV_REGISTER_SPI_TAG(busdev_icm42605_2,   DEVHW_ICM42605,  ICM42605_SPI_BUS_2,   ICM42605_CS_PIN_2,   NONE,   0,  DEVFLAGS_NONE,  IMU_ICM42605_ALIGN_2);

Solution Walkthrough:

Before:

// src/main/target/BROTHERHOBBYH743/target.h
// ...
// No data-ready interrupt pins are defined for the IMUs.

// src/main/target/BROTHERHOBBYH743/target.c
// ...
// IMUs are registered without an interrupt pin, forcing polled mode.
BUSDEV_REGISTER_SPI_TAG(busdev_icm42605, ..., ICM42605_CS_PIN, NONE, ...);
BUSDEV_REGISTER_SPI_TAG(busdev_icm42605_2, ..., ICM42605_CS_PIN_2, NONE, ...);
// ...

After:

// src/main/target/BROTHERHOBBYH743/target.h
// ...
// Define data-ready interrupt pins for both IMUs.
#define ICM42605_INT_PIN        PC13 // Example pin
#define ICM42605_INT_PIN_2      PE10 // Example pin

// src/main/target/BROTHERHOBBYH743/target.c
// ...
// Register IMUs with interrupt pins to enable interrupt-driven sampling.
BUSDEV_REGISTER_SPI_TAG(busdev_icm42605, ..., ICM42605_CS_PIN, ICM42605_INT_PIN, ...);
BUSDEV_REGISTER_SPI_TAG(busdev_icm42605_2, ..., ICM42605_CS_PIN_2, ICM42605_INT_PIN_2, ...);
// ...
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical performance issue in the IMU configuration, where polling is used instead of interrupts, and proposes a correct fix that will significantly improve flight stability.

High
Possible issue
Fix DMA channel conflicts

The DMA channel assignments for S9 and S10 are both set to 0, which may cause
conflicts. Each timer output should have a unique DMA channel assignment to
prevent resource conflicts during PWM operations.

src/main/target/BROTHERHOBBYH743/target.c [43-44]

-DEF_TIM(TIM4, CH3, PD14, TIM_USE_OUTPUT_AUTO, 0, 0),   // S9
-DEF_TIM(TIM4, CH4, PD15, TIM_USE_OUTPUT_AUTO, 0, 0),   // S10 DMA_NONE
+DEF_TIM(TIM4, CH3, PD14, TIM_USE_OUTPUT_AUTO, 0, 8),   // S9
+DEF_TIM(TIM4, CH4, PD15, TIM_USE_OUTPUT_AUTO, 0, 9),   // S10 DMA_NONE
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that multiple timer outputs (S1, S9, S10, S11, S12, and BEEPER) are assigned to DMA channel index 0, which can cause resource conflicts and lead to incorrect PWM operation. This is a critical bug for a flight controller.

High
Resolve DMA channel duplication

Both S11 and S12 timer outputs have DMA channel 0 assigned, which creates a
resource conflict. Assign unique DMA channels to prevent timer interference and
ensure proper PWM functionality.

src/main/target/BROTHERHOBBYH743/target.c [46-47]

-DEF_TIM(TIM15, CH1, PE5, TIM_USE_OUTPUT_AUTO, 0, 0),   // S11
-DEF_TIM(TIM15, CH2, PE6, TIM_USE_OUTPUT_AUTO, 0, 0),   // S12 DMA_NONE
+DEF_TIM(TIM15, CH1, PE5, TIM_USE_OUTPUT_AUTO, 0, 10),   // S11
+DEF_TIM(TIM15, CH2, PE6, TIM_USE_OUTPUT_AUTO, 0, 11),   // S12 DMA_NONE
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that multiple timer outputs (S1, S9, S10, S11, S12, and BEEPER) are assigned to DMA channel index 0, which can cause resource conflicts and lead to incorrect PWM operation. This is a critical bug for a flight controller.

High
  • More

@MrD-RC MrD-RC added this to the 9.0 milestone Oct 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants