Skip to content

I2C async refactor - #12108

Closed
error414 wants to merge 20 commits into
iNavFlight:maintenance-10.xfrom
error414:error414/feature/i2c_refactor
Closed

error414 wants to merge 20 commits into
iNavFlight:maintenance-10.xfrom
error414:error414/feature/i2c_refactor

Conversation

@error414

@error414 error414 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

I created PR to track progress, if anybody would interested.

In Inav is I2C sync, it means that all i2c calls waits for response. For 16bit register it's approx 100us, it's event worse in OLED driver, there can be blocking for 10ms and more. It blocks PID loop and creating dshot frames.

image

Async call in this PR uses state machine, send request and in next cycle check response.

Probably I would need help with testing. Lot of HW for test,

I2C back-ends (per MCU)

  • STM32F4 (bus_i2c_stm32f40x.c, new IRQ engine ported from BF) – never ran on hardware, e.g. SPEEDYBEEF405V4. Highest priority
  • STM32H7 HAL (bus_i2c_hal.c) – H743 verified with baro + mag only
  • STM32F7 HAL (bus_i2c_hal.c)
  • AT32F43x (bus_i2c_at32f43x.c) – address phase still blocks ~70 µs
  • RP2350 (bus_i2c_rp2350.c, FIFO + IRQ engine)
  • Soft I2C (bus_i2c_soft.c, synchronous fallback)

Baro

  • DPS310 / SPL06 (H743) need check agains Detect the DPS310 on both of its I2C addresses #11906
  • BMP280 – forced mode, async start + 6 B read, new cadence up_delay + 1 ms
  • BMP388 – same
  • 2SMPB-02B – same
  • MS5611 / MS5607 – two-phase async, zero-value check
  • BMP085 – two-phase async
  • LPS25H – 6 B burst from STATUS_REG, up_delay 20 ms instead of 1 ms polling

Mag

  • the mag on the H743 test board
  • QMC5883L – status → data
  • QMC5883P – status → data
  • HMC5883L – single 6 B read (have no idea where to find HMC mag)
  • IST8308 – DRDY check from maintenance rewritten into async steps (merge-resolved)
  • IST8310 – same
  • AK8963 – 3 steps
  • AK8975 – 4 steps
  • LIS2MDL – status + data
  • LIS3MDL – status + data
  • MAG3110 – status + data
  • RM3100 – status + data
  • VCM5883 – single read
  • MLX90393 – single read

Pitot

  • MS4525 – 1 B start, two 4 B frames
  • MS5525 – async conversion + 3 B ADC read
  • DLVR-L10D – 4 B read

Rangefinder

  • VL53L0X – 8 start writes, one per call
  • VL53L1X – index / read / clear steps, cached interrupt polarity
  • SRF10
  • TeraRanger Evo (can test myself)
  • TOF10120
  • US42 – zero-length probe write was broken on H7 before

Other I2C

  • OLED SSD1306 – framebuffer rewrite: dashboard, CMS via displayport, page change, display unplugged at runtime
  • OLED SH1106 – same, plus the 2-column offset
  • LM75 – detection and protothread fix: with a chip and without one
  • DS2482 + DS18B20
  • PCF8574
  • INA226 – getters return cached values
  • IRLock
  • SD card over SPI – API change only (busIsBusy(dev, NULL)), regression check of logging

Cross-cutting scenarios

  • Several async devices on one bus (MATEKH743: OLED shares I2C1 with the mag; baro + temperature + pitot on I2C2)
  • 100 kHz and 400 kHz bus speed
  • CLI tasks: BARO / COMPASS max µs, no DSHOT gaps
  • Device unplugged at runtime, unplugged and plugged back (error back-off, HAL re-init)
  • EEPROM save during I2C traffic
  • debug_mode = I2C statistics
  • Flight test – nobody has flown this yet

@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Make periodic I2C transfers non-blocking across sensors and OLED

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add interrupt-driven I2C transfers so periodic device access no longer waits for wire completion.
• Move sensor reads and OLED updates into scheduler-driven steps while retaining blocking
 initialization paths.
• Document the new APIs and add I2C diagnostics; broader hardware validation remains outstanding.
Diagram

graph TD
  TASKS["Scheduler tasks"] --> SENSORS["Sensor state machines"] --> BUS["Bus start and poll API"] --> BACKENDS["MCU I2C backends"] --> DEVICES["I2C devices"]
  TASKS --> OLED["OLED frame pump"] --> BUS
  BUS --> FALLBACK["Blocking fallback"] --> DEVICES
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Queued bus transactions
  • ➕ Centralizes arbitration when several devices share one bus.
  • ➕ Can associate completion and errors with individual requests.
  • ➖ Requires queue ownership, buffer-lifetime, and callback policies.
  • ➖ Substantially increases scope and memory use.
2. DMA-backed transfers
  • ➕ Could further reduce interrupt overhead for long display writes.
  • ➖ Adds MCU-specific DMA configuration and buffer constraints.
  • ➖ Offers less benefit for short sensor transactions.

Recommendation: The start-and-poll approach is a reasonable first step for short, infrequent sensor transfers and preserves existing blocking callers. Prioritize shared-bus, timeout, unplug/replug, and MCU hardware testing before relying on it in flight; consider a queue if contention or per-device error attribution becomes material.

Files changed (76) +3330 / -1191

Enhancement (69) +3136 / -1184
debug.hAdd an I2C debug identifier +1/-0

Add an I2C debug identifier

• Introduces DEBUG_I2C for transfer diagnostics.

src/main/build/debug.h

ds2482.cStart bridge commands and status polls asynchronously +26/-8

Start bridge commands and status polls asynchronously

• Uses start-and-poll status reads and attempts non-blocking command writes, with blocking fallback when the bus is occupied.

src/main/drivers/1-wire/ds2482.c

barometer.hExtend barometer measurement hooks +11/-0

Extend barometer measurement hooks

• Adds optional asynchronous read hooks and a combined pressure/temperature read flag while retaining blocking-driver compatibility.

src/main/drivers/barometer/barometer.h

barometer_2smpb_02b.cSplit 2SMPB measurement start from data retrieval +14/-4

Split 2SMPB measurement start from data retrieval

• Starts forced conversions and combined pressure/temperature reads without waiting for completion.

src/main/drivers/barometer/barometer_2smpb_02b.c

barometer_bmp085.cStage BMP085 temperature and pressure reads +25/-17

Stage BMP085 temperature and pressure reads

• Starts conversion writes and ADC reads asynchronously, then parses the persistent receive buffer.

src/main/drivers/barometer/barometer_bmp085.c

barometer_bmp280.cMake BMP280 forced-mode sampling asynchronous +20/-36

Make BMP280 forced-mode sampling asynchronous

• Separates conversion start, combined data read, and parsing; removes unused temperature-phase hooks.

src/main/drivers/barometer/barometer_bmp280.c

barometer_bmp388.cStage BMP388 forced-mode measurements +24/-25

Stage BMP388 forced-mode measurements

• Adds asynchronous conversion and combined-result reads, retaining the SPI dummy-byte handling.

src/main/drivers/barometer/barometer_bmp388.c

barometer_dps310.cShare DPS310 sampling and detection with SPL06 +62/-23

Share DPS310 sampling and detection with SPL06

• Adds asynchronous readiness and measurement reads and probes SPL06 devices through the shared driver.

src/main/drivers/barometer/barometer_dps310.c

barometer_dps310.hExpose SPL06 detection in the DPS310 driver +1/-0

Expose SPL06 detection in the DPS310 driver

• Declares the shared driver's SPL06 detection entry point.

src/main/drivers/barometer/barometer_dps310.h

barometer_lps25h.cRead LPS25H status and measurements in one burst +25/-49

Read LPS25H status and measurements in one burst

• Replaces separate blocking polls with an asynchronous six-byte status, pressure, and temperature read.

src/main/drivers/barometer/barometer_lps25h.c

barometer_ms56xx.cStage MS56xx conversion and ADC reads +31/-8

Stage MS56xx conversion and ADC reads

• Uses asynchronous conversion writes and buffered ADC reads, rejecting zero-valued unfinished results.

src/main/drivers/barometer/barometer_ms56xx.c

bus.cAdd bus-level transfer start operations +89/-2

Add bus-level transfer start operations

• Dispatches asynchronous reads and writes by bus type and makes busy polling expose I2C errors; SPI operations still complete synchronously.

src/main/drivers/bus.c

bus.hDefine the asynchronous bus contract +20/-1

Define the asynchronous bus contract

• Declares start, busy, and error APIs plus results for multi-transfer sensor reads.

src/main/drivers/bus.h

bus_busdev_i2c.cForward bus-device starts to I2C backends +19/-6

Forward bus-device starts to I2C backends

• Preserves raw-register flags when starting transfers and forwards backend busy and error results.

src/main/drivers/bus_busdev_i2c.c

bus_i2c.hDeclare platform asynchronous I2C operations +8/-1

Declare platform asynchronous I2C operations

• Adds start functions, buffer-lifetime guidance, and STM32F4 interrupt metadata.

src/main/drivers/bus_i2c.h

bus_i2c_at32f43x.cAdd AT32 interrupt-driven transfer support +201/-32

Add AT32 interrupt-driven transfer support

• Uses the AT32 interrupt APIs for transfer data, polls completion and timeouts, and retains synchronous address-phase and blocking-call compatibility.

src/main/drivers/bus_i2c_at32f43x.c

bus_i2c_hal.cAdd interrupt-driven STM32F7/H7 HAL transfers +247/-9

Add interrupt-driven STM32F7/H7 HAL transfers

• Starts HAL interrupt transfers, records callback completion and errors, resets timed-out transfers, and coordinates blocking callers.

src/main/drivers/bus_i2c_hal.c

bus_i2c_rp2350.cAdd RP2350 FIFO and IRQ transfer engine +291/-7

Add RP2350 FIFO and IRQ transfer engine

• Queues I2C commands into the controller FIFO, handles read refills and completion interrupts, and waits before legacy blocking operations.

src/main/drivers/bus_i2c_rp2350.c

bus_i2c_soft.cProvide synchronous software-I2C fallback +26/-0

Provide synchronous software-I2C fallback

• Implements start calls through existing blocking transfers and reports immediate completion.

src/main/drivers/bus_i2c_soft.c

bus_i2c_stm32f40x.cReplace STM32F4 I2C engine with interrupt-driven transfers +441/-421

Replace STM32F4 I2C engine with interrupt-driven transfers

• Reworks event and error handling around asynchronous transfer state, polling, timeout recovery, blocking compatibility, and diagnostics.

src/main/drivers/bus_i2c_stm32f40x.c

compass.hAdd optional compass read-start hook +5/-1

Add optional compass read-start hook

• Lets a compass stage transfers before its existing read callback parses a completed sample.

src/main/drivers/compass/compass.h

compass_ak8963.cStage AK8963 status, data, and retrigger writes +42/-17

Stage AK8963 status, data, and retrigger writes

• Turns the single-measurement read into asynchronous steps and parses buffered data after completion.

src/main/drivers/compass/compass_ak8963.c

compass_ak8975.cStage AK8975 four-transfer samples +55/-13

Stage AK8975 four-transfer samples

• Separates status, axes, overflow status, and next-conversion trigger into non-blocking steps.

src/main/drivers/compass/compass_ak8975.c

compass_hmc5883l.cBuffer HMC5883L continuous-mode reads +15/-18

Buffer HMC5883L continuous-mode reads

• Starts one asynchronous six-byte read and parses its result later, preserving SPI register selection.

src/main/drivers/compass/compass_hmc5883l.c

compass_ist8308.cStage IST8308 readiness and axis reads +35/-13

Stage IST8308 readiness and axis reads

• Reads status before asynchronously fetching axes when a fresh sample is available.

src/main/drivers/compass/compass_ist8308.c

compass_ist8310.cStage IST8310 readiness and axis reads +35/-12

Stage IST8310 readiness and axis reads

• Splits data-ready polling and axis retrieval into non-blocking transfers.

src/main/drivers/compass/compass_ist8310.c

compass_lis2mdl.cStage LIS2MDL status and data reads +37/-9

Stage LIS2MDL status and data reads

• Reads the ready flag before fetching and parsing buffered axes.

src/main/drivers/compass/compass_lis2mdl.c

compass_lis3mdl.cStage LIS3MDL status and data reads +36/-10

Stage LIS3MDL status and data reads

• Uses asynchronous status and axis transfers, preserving the fresh-data check.

src/main/drivers/compass/compass_lis3mdl.c

compass_mag3110.cStage MAG3110 ready checks and samples +36/-9

Stage MAG3110 ready checks and samples

• Starts separate non-blocking status and axis reads and parses completed data.

src/main/drivers/compass/compass_mag3110.c

compass_mlx90393.cBuffer MLX90393 measurement replies +12/-6

Buffer MLX90393 measurement replies

• Moves the measurement command and response into an asynchronous read-start hook.

src/main/drivers/compass/compass_mlx90393.c

compass_qmc5883l.cStage QMC5883L status and axis transfers +40/-15

Stage QMC5883L status and axis transfers

• Checks the buffered data-ready status before starting a non-blocking axes read.

src/main/drivers/compass/compass_qmc5883l.c

compass_qmc5883p.cStage QMC5883P status and axis transfers +38/-12

Stage QMC5883P status and axis transfers

• Splits status and measurement access into asynchronous steps and parses only ready data.

src/main/drivers/compass/compass_qmc5883p.c

compass_rm3100.cStage RM3100 status and nine-byte measurement +41/-24

Stage RM3100 status and nine-byte measurement

• Starts status and axis transfers separately and parses buffered 24-bit values.

src/main/drivers/compass/compass_rm3100.c

compass_vcm5883.cBuffer VCM5883 continuous-mode samples +12/-10

Buffer VCM5883 continuous-mode samples

• Starts the six-byte axis read asynchronously and parses it after completion.

src/main/drivers/compass/compass_vcm5883.c

display_ug2864hsweg01.cBuffer OLED drawing and stream changed columns +207/-71

Buffer OLED drawing and stream changed columns

• Adds a shadow framebuffer, dirty-column tracking, and a chunked asynchronous pump with error backoff and SH1106 offset handling. Initialization and explicit flush remain blocking.

src/main/drivers/display_ug2864hsweg01.c

display_ug2864hsweg01.hExpose OLED pump and transfer status +8/-1

Expose OLED pump and transfer status

• Declares background pumping, busy and capacity queries, flush, and retry timing.

src/main/drivers/display_ug2864hsweg01.h

ina226.cCache asynchronously refreshed INA226 readings +46/-8

Cache asynchronously refreshed INA226 readings

• Alternates background bus- and shunt-voltage reads and serves completed cached values through the existing getters.

src/main/drivers/ina226.c

ina226.hStore INA226 background-read state +10/-0

Store INA226 background-read state

• Adds receive, pending-register, cached-value, and validity fields plus a polling entry point.

src/main/drivers/ina226.h

io_pcf8574.cStart PCF8574 writes without waiting +4/-1

Start PCF8574 writes without waiting

• Attempts a non-blocking output write and falls back to blocking access if it cannot start.

src/main/drivers/io_pcf8574.c

irlock.cScan IRLock frames across asynchronous reads +53/-17

Scan IRLock frames across asynchronous reads

• Reads sync bytes and frame payload over successive calls, returning a verified frame only after completion.

src/main/drivers/irlock.c

irlock.hDistinguish pending IRLock reads from complete frames +7/-1

Distinguish pending IRLock reads from complete frames

• Introduces read outcomes for no data, pending transfer, and checksum-verified frame.

src/main/drivers/irlock.h

pitotmeter.hAdd staged pitot read hook +7/-0

Add staged pitot read hook

• Defines an optional multi-step read start callback alongside the existing sample parser.

src/main/drivers/pitotmeter/pitotmeter.h

pitotmeter_dlvr_l10d.cBuffer DLVR pressure frames +11/-6

Buffer DLVR pressure frames

• Starts a non-blocking four-byte read before validating and parsing the pressure frame.

src/main/drivers/pitotmeter/pitotmeter_dlvr_l10d.c

pitotmeter_ms4525.cStage MS4525 request and paired frames +26/-13

Stage MS4525 request and paired frames

• Starts the measurement request and reads two frames over separate non-blocking steps.

src/main/drivers/pitotmeter/pitotmeter_ms4525.c

pitotmeter_ms5525.cStage MS5525 conversion and ADC reads +18/-13

Stage MS5525 conversion and ADC reads

• Starts conversion writes and three-byte ADC reads asynchronously, rejecting zero-valued results.

src/main/drivers/pitotmeter/pitotmeter_ms5525.c

rangefinder.hLet rangefinder updates request short retries +5/-1

Let rangefinder updates request short retries

• Changes the update callback to return a delay and defines the pending-transfer retry interval.

src/main/drivers/rangefinder/rangefinder.h

rangefinder_srf10.cStage SRF10 revision, range, and firing operations +63/-20

Stage SRF10 revision, range, and firing operations

• Spreads status checks, two range-byte reads, and the next ranging command across calls.

src/main/drivers/rangefinder/rangefinder_srf10.c

rangefinder_teraranger_evo.cStage TeraRanger read and retrigger +43/-15

Stage TeraRanger read and retrigger

• Uses asynchronous reads and triggers while retaining CRC and out-of-range checks.

src/main/drivers/rangefinder/rangefinder_teraranger_evo.c

rangefinder_tof10120_i2c.cBuffer TOF10120 distance reads +22/-7

Buffer TOF10120 distance reads

• Starts a two-byte distance read, revisits it after completion, and reports transfer errors.

src/main/drivers/rangefinder/rangefinder_tof10120_i2c.c

rangefinder_us42.cStage US42 reads and probe commands +49/-15

Stage US42 reads and probe commands

• Moves measurement retrieval and the zero-length probe write into non-blocking update steps.

src/main/drivers/rangefinder/rangefinder_us42.c

rangefinder_vl53l0x.cStage VL53L0X measurement sequence +106/-83

Stage VL53L0X measurement sequence

• Issues start-register writes one per call and asynchronously polls readiness, reads range, and clears the interrupt.

src/main/drivers/rangefinder/rangefinder_vl53l0x.c

rangefinder_vl53l1x.cStage VL53L1X indexed reads +107/-21

Stage VL53L1X indexed reads

• Separates 16-bit register-index writes from status and range reads, caches interrupt polarity at initialization, and clears interrupts asynchronously.

src/main/drivers/rangefinder/rangefinder_vl53l1x.c

sensor.hDefine temperature read-start callback +1/-0

Define temperature read-start callback

• Adds the function-pointer type used by asynchronous temperature drivers.

src/main/drivers/sensor.h

lm75.cStage LM75 reads while preserving detection +22/-10

Stage LM75 reads while preserving detection

• Reads temperature into device scratchpad asynchronously but keeps a blocking acknowledge check during detection.

src/main/drivers/temperature/lm75.c

temperature.hAdd optional temperature read-start hook +1/-0

Add optional temperature read-start hook

• Extends the device interface so temperature sampling can precede parsing with a transfer.

src/main/drivers/temperature/temperature.h

cli.cExpose I2C diagnostics in the CLI +2/-1

Expose I2C diagnostics in the CLI

• Adds I2C to debug mode names.

src/main/fc/cli.c

fc_tasks.cReschedule device tasks around pending transfers +27/-8

Reschedule device tasks around pending transfers

• Uses sensor-requested deadlines, feeds the barometer estimator only on new samples, and pumps dirty OLED data from the dashboard task.

src/main/fc/fc_tasks.c

displayport_oled.cReport OLED background-transfer state +2/-2

Report OLED background-transfer state

• Connects display-port busy and available-capacity queries to the buffered OLED driver.

src/main/io/displayport_oled.c

barometer.cDrive barometer measurements through staged phases +102/-33

Drive barometer measurements through staged phases

• Replaces the two-state loop with start, read, and parse phases, retries pending transfers, and signals when a fresh sample is ready. Routes SPL06 detection through the DPS310 driver.

src/main/sensors/barometer.c

barometer.hExpose barometer new-sample status +1/-1

Expose barometer new-sample status

• Changes baroUpdate to report whether the current call produced a sample.

src/main/sensors/barometer.h

battery.cPoll INA226 during battery and current updates +18/-0

Poll INA226 during battery and current updates

• Keeps cached monitor readings refreshed without performing synchronous reads in the value getters.

src/main/sensors/battery.c

compass.cDrive multi-step compass transfers +49/-6

Drive multi-step compass transfers

• Polls pending reads, advances driver steps, and returns short retry or regular-period scheduling delays.

src/main/sensors/compass.c

compass.hDefine compass update cadence and deadline +5/-1

Define compass update cadence and deadline

• Adds regular-period constants and changes compassUpdate to return its next-call delay.

src/main/sensors/compass.h

irlock.cReschedule IRLock while a frame is pending +13/-2

Reschedule IRLock while a frame is pending

• Updates frame freshness only after a complete frame and requests short retries during transfers.

src/main/sensors/irlock.c

irlock.hDefine IRLock retry and regular periods +6/-1

Define IRLock retry and regular periods

• Declares timing constants and the deadline-returning update function.

src/main/sensors/irlock.h

pitotmeter.cDrive staged pitot reads in the protothread +45/-2

Drive staged pitot reads in the protothread

• Polls multi-step transfers, parses completed samples, and requests short retries while retaining the regular update period.

src/main/sensors/pitotmeter.c

pitotmeter.hDefine pitot retry and update periods +6/-1

Define pitot retry and update periods

• Adds timing constants and changes pitotUpdate to return its next-call delay.

src/main/sensors/pitotmeter.h

rangefinder.cHonor rangefinder driver retry deadlines +5/-2

Honor rangefinder driver retry deadlines

• Uses a driver's requested short delay when present and otherwise retains the configured regular period.

src/main/sensors/rangefinder.c

temperature.cWait for LM75 completion in the temperature protothread +18/-3

Wait for LM75 completion in the temperature protothread

• Starts asynchronous LM75 reads, checks completion before parsing, and avoids retaining a stale configuration pointer across a protothread wait.

src/main/sensors/temperature.c

Refactor (3) +5 / -6
CMakeLists.txtRemove the separate SPL06 sources from the build +0/-2

Remove the separate SPL06 sources from the build

• Drops SPL06 source entries after its detection moves into the DPS310 driver.

src/main/CMakeLists.txt

rangefinder_virtual.cAdapt virtual rangefinder update return type +3/-2

Adapt virtual rangefinder update return type

• Returns the regular scheduling delay after calling the unchanged high-level update function.

src/main/drivers/rangefinder/rangefinder_virtual.c

sdcard_spi.cAdapt SD-card callers to the busy API +2/-2

Adapt SD-card callers to the busy API

• Passes a null error pointer when polling SPI completion.

src/main/drivers/sdcard/sdcard_spi.c

Documentation (3) +188 / -0
AGENTS.mdIndex the I2C development guide +1/-0

Index the I2C development guide

• Adds the non-blocking I2C guide to the development-document index.

AGENTS.md

Settings.mdDocument the I2C debug mode +1/-0

Document the I2C debug mode

• Lists I2C among available debug settings.

docs/Settings.md

non-blocking-i2c.mdExplain non-blocking I2C contracts and limitations +186/-0

Explain non-blocking I2C contracts and limitations

• Documents backend behavior, buffer ownership, sensor state machines, OLED pumping, debug fields, and remaining blocking paths.

docs/development/non-blocking-i2c.md

Other (1) +1 / -1
settings.yamlRegister the I2C debug setting +1/-1

Register the I2C debug setting

• Adds I2C to the debug mode values.

src/main/fc/settings.yaml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. I2C errors can stall later transfers ✓ Resolved
Description
i2cErrorHandler() marks the STM32F4 bus idle after arbitration loss or overrun without reliably
disabling its event interrupt. On the next request, i2cStartTransfer() sees that interrupt enabled
and skips generating START, leaving the request on stale peripheral state until it times out.
Code

src/main/drivers/bus_i2c_stm32f40x.c[R455-459]

+    // If AF, BERR or ARLO, abandon the current job
+    if (SR1Register & (I2C_SR1_BERR | I2C_SR1_ARLO | I2C_SR1_AF)) {
+        (void)I2Cx->SR2;                                                        // read second status register to clear ADDR if it is set (note that BTF will not be set after a NACK)
+        I2C_ITConfig(I2Cx, I2C_IT_BUF, DISABLE);                                // disable the RXNE/TXE interrupt - prevent the ISR tailchaining onto the ER (hopefully)
+        if (!(SR1Register & I2C_SR1_ARLO) && !(I2Cx->CR1 & I2C_CR1_STOP)) {     // if we dont have an ARLO error, ensure sending of a stop
Evidence
The handler excludes overrun from its cleanup branch and excludes arbitration loss from the branch
that disables event interrupts, yet clears busy for both. The start path generates START only when
event interrupts are disabled.

src/main/drivers/bus_i2c_stm32f40x.c[451-475]
src/main/drivers/bus_i2c_stm32f40x.c[321-332]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The STM32F4 error handler exposes an idle bus after arbitration-loss or overrun handling without restoring the interrupt and peripheral state expected by the next transfer.
## Fix Focus Areas
- src/main/drivers/bus_i2c_stm32f40x.c[451-475]
- src/main/drivers/bus_i2c_stm32f40x.c[321-332]
## Recommended Fix
Give every terminal error path appropriate interrupt cleanup and peripheral recovery before clearing `busy`. Ensure the next start generates START even when an error left event interrupts enabled.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Pitot retries make stale airspeed look new ✓ Resolved
Description
taskUpdatePitot() republishes the cached airspeed on every short retry even when pitotThread()
has yielded without parsing a new sample. While an async read remains pending and the previous value
is healthy, each retry advances the estimator's pitot timestamp, potentially at the one-millisecond
retry cadence.
Code

src/main/fc/fc_tasks.c[R229-232]

+    const uint32_t newDeadline = pitotUpdate();
+    if (newDeadline != 0) {
+        rescheduleTask(TASK_SELF, newDeadline);
+    }
Evidence
The new pending-transfer branch yields without recalculating airspeed and requests a short retry.
The scheduler still publishes on that retry, and publication unconditionally assigns the current
time as the sample's last update.

src/main/sensors/pitotmeter.c[270-298]
src/main/sensors/pitotmeter.c[429-433]
src/main/fc/fc_tasks.c[228-235]
src/main/navigation/navigation_pos_estimator.c[343-348]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Short async pitot retries update the estimator timestamp even though no new airspeed sample was parsed.
## Fix Focus Areas
- src/main/fc/fc_tasks.c[228-235]
- src/main/sensors/pitotmeter.c[270-298]
- src/main/sensors/pitotmeter.c[408-433]
## Recommended Fix
Have `pitotUpdate()` indicate when a new sample was parsed, and call `updatePositionEstimator_PitotTopic()` only then. Keep the short retry deadline for pending transfers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Shared-bus barometers run at gyro speed ✓ Resolved
Description
Deleting the SPL06 driver's busSetSpeed(..., BUS_SPEED_STANDARD) call leaves baroSPL06Detect()
without a replacement speed selection. On CORVON405V2 the SPL06 shares SPI2 with a BMI088 gyro that
selects BUS_SPEED_FAST, so barometer transactions can use that bus setting instead of the
previously selected standard speed.
Code

src/main/drivers/barometer/barometer_spl06.c[235]

-    busSetSpeed(baro->busDev, BUS_SPEED_STANDARD);
Evidence
The removed driver selected standard speed before probing, while the replacement detection path has
no speed call. The cited target puts the barometer and BMI088 on SPI2, and the gyro initialization
selects fast speed on that shared peripheral.

src/main/sensors/barometer.c[147-153]
src/main/drivers/barometer/barometer_dps310.c[356-374]
src/main/drivers/barometer/barometer_dps310.c[427-432]
src/main/target/CORVON405V2/target.h[46-56]
src/main/drivers/accgyro/accgyro_bmi088.c[85-103]
src/main/drivers/bus_busdev_spi.c[62-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The replacement SPL06 detection path omits the old driver's SPI speed selection, so a shared bus can retain the gyro's faster setting for barometer transactions.
## Fix Focus Areas
- src/main/drivers/barometer/barometer_spl06.c[235-235]
- src/main/drivers/barometer/barometer_dps310.c[356-374]
- src/main/drivers/barometer/barometer_dps310.c[427-432]
## Recommended Fix
Select `BUS_SPEED_STANDARD` for an SPL06 SPI device in the new detection path before its first register transaction. Preserve the intended speed when other devices change the shared SPI peripheral's setting.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Sensors on a shared bus get each other's results ✓ Resolved
Description
busIsBusy() returns a bus-wide error flag that i2cStartTransfer() clears for every new
transfer and i2cInit() wipes during timeout recovery, rather than preserving the result for the
device that initiated the transfer. If another device starts a transfer before a sensor or OLED
caller polls, or a blocking call triggers recovery, callers can treat a failed transfer as
successful and consume stale sensor data or discard a failed OLED chunk, while another device’s
error can cause a good sensor sample to be dropped.
Code

src/main/drivers/bus.c[R504-508]

+bool busIsBusy(const busDevice_t * dev, bool * error)
+{
+    if (error) {
+        *error = false;
+    }
Evidence
The F4 and HAL back-ends maintain completion errors in state shared by a controller and clear that
state when a new transfer starts; F4 failure recovery also resets the state through i2cInit(),
which can be reached from i2cWaitForIdle() during a blocking call. Sensor and OLED completion
paths poll busIsBusy(dev, &error) after their transfers, and separate tasks can start another
device’s transfer between completion and that poll, so the reported error need not belong to the
caller’s transfer.

src/main/drivers/bus_i2c_stm32f40x.c[316-324]
src/main/drivers/bus_i2c_stm32f40x.c[178-183]
src/main/drivers/bus_i2c_stm32f40x.c[275-277]
src/main/drivers/bus_i2c_stm32f40x.c[341-352]
src/main/drivers/bus_i2c_stm32f40x.c[357-381]
src/main/drivers/bus_i2c_hal.c[285-292]
src/main/sensors/barometer.c[319-327]
src/main/sensors/compass.c[472-500]
src/main/sensors/pitotmeter.c[270-284]
src/main/drivers/bus_i2c_hal.c[159-165]
src/main/drivers/bus_i2c_hal.c[280-293]
src/main/drivers/display_ug2864hsweg01.c[266-280]
src/main/fc/fc_tasks.c[189-216]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Non-blocking I2C callers use `busIsBusy(dev, &error)` to determine the outcome of their own transfers, but the back-ends expose a bus-wide error that another transfer or timeout recovery can clear before the caller polls it. Sensor reads and OLED writes can therefore be handled according to another transfer’s result.
## Fix Focus Areas
- src/main/drivers/bus_i2c_stm32f40x.c[316-324]
- src/main/drivers/bus_i2c_stm32f40x.c[357-381]
- src/main/drivers/bus_i2c_hal.c[285-293]
- src/main/drivers/bus.c[504-520]
- src/main/drivers/bus_busdev_i2c.c[74-77]
- src/main/sensors/barometer.c[319-327]
- src/main/drivers/display_ug2864hsweg01.c[266-280]
## Recommended Fix
Associate each transfer’s completion and error result with its owner (`busDevice_t*` or address) or a transfer token, and retain that result independently of later transfers—for example, in per-device `lastResult` and `pending` fields updated on completion or error. Make `i2cBusBusy(dev, &error)` report whether that device’s transfer remains pending and return its own result; mark the aborted owner’s transfer as failed during `i2cInit()` recovery rather than wiping its outcome. Apply the same contract to async callers and across the F4, HAL, AT32, and RP2350 I2C back-ends.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/main/drivers/bus_i2c_stm32f40x.c Outdated
Comment thread src/main/fc/fc_tasks.c Outdated
Comment thread src/main/drivers/barometer/barometer_spl06.c
Comment thread src/main/drivers/bus.c
@MrScothh

MrScothh commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

I tested this on two F722 boards with a DPS310 on I2C1, which covers the STM32F7 HAL line of your list: a Flywoo GOKU F722 Pro V2 (target FLYWOOF722PRO) and a Diatone Mamba F722 2022B (MAMBAF722_2022B), both flashed with 9a2da45.

A DPS310 read as an SPL06. On maintenance-10.x, baro_hardware = AUTO takes a DPS310 for an SPL06: both answer 0x10 at register 0x0D, and the SPL06 is probed first. The SPL06 driver then reads the DPS310's temperature about 27 °C high. With this PR, AUTO still names it SPL06, but the reading goes through the DPS310 code and the temperature is right. Blackbox baroTemperature, board at rest, room about 20 °C:

Board 10.x, AUTO (SPL06) 10.x, DPS310 This PR, AUTO
Flywoo F722 Pro V2 46.2 °C 19.6 °C 18.7 to 19.8 °C
Mamba F722 2022B 44.0 °C 17.0 °C 16.0 °C

BARO task (tasks, Mamba): 211 µs average and 239 µs max at 62 Hz on 10.x with AUTO, 124/320 µs with DPS310; with this PR 7 µs average and 25 µs max, at 980 Hz.

I2C on the F7 HAL path. The error count at boot is the same with and without this PR (50 to 60, from the probes) and does not grow: still 50 after 133 s and 88 s on the Flywoo. The baro altitude at rest updates at the normal rate with the same noise as on 10.x. Everything else works as on 10.x: gyro on SPI, MAX7456 OSD, blackbox on the W25Q128 flash, the M10 GPS on the Flywoo and the CRSF receiver on the Mamba.

Size. On F722 targets, which are close to full: FLYWOOF722PRO +5,508 B (to 99.28 %), MAMBAF722_2022B +5,888 B (to 99.68 %, about 1.6 KB left).

Not tested: a magnetometer or another device on the same I2C bus, and flight.

@error414

error414 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

cool, thanks you very much for testing. Seems that proof of concept works. About SPL06 and DPS310, these baros have identical address and registers, @Raffi1202 made some changes there in #11906 these changes are in this branch but was quite tricky to merge it. i will have to look at that closelly.

Errors during boot should be unchanged, detect and init is done by old way, sync. There would not be benefit to make it async. I would like to reduce complexity as low as possible.

I'm glad that SPI is untouched and other devices works.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

RAM / Flash usage vs. base commit e87050f — commit ce0af93

Using the nearest available size baseline — the PR's exact base commit has no stored baseline yet.

Target Flash Δ RAM Δ
MATEKF405 ⚠️ +3300 B (+0.46%) CCM: -4 B (-0.01%)
RAM: +1460 B (+1.40%)
MATEKF722 ⚠️ +5388 B (+1.12%) ITCM_RAM: +40 B (+0.32%)
RAM: +1424 B (+1.64%)
TCM: +4 B (+0.02%)
MATEKF765 ⚠️ +6680 B (+0.88%) DTCM_RAM: ±0 B (±0.00%)
SRAM1: +1456 B (+1.16%)
MATEKH743 ⚠️ +6376 B (+0.80%) D2_RAM: ±0 B (±0.00%)
DTCM_RAM: ±0 B (±0.00%)
ITCM_RAM: ±0 B (±0.00%)
RAM: +1544 B (+1.09%)

See RAM/flash optimization guide for techniques to reduce usage.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Test firmware build ready — commit ce0af93

Download firmware for PR #12108

251 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

@sensei-hacker

sensei-hacker commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

It looks like this and #12063 and covering the same issue?

It may be useful to cross-reference and see if each PR offers any different that may be useful. Having not read the code yet, my prediction would be that the two differ in some ways and a combination would be best.

@MrScothh

MrScothh commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

I answered the same question on #12063, comparing the two: #12063 (comment)

@error414

error414 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

will be solved in diferent PR

@error414 error414 closed this Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants