Skip to content

i2c, baro: read the barometer without waiting on the bus - #12063

Open
MrScothh wants to merge 5 commits into
iNavFlight:maintenance-10.xfrom
MrScothh:feature/baro-i2c-async
Open

MrScothh wants to merge 5 commits into
iNavFlight:maintenance-10.xfrom
MrScothh:feature/baro-i2c-async

Conversation

@MrScothh

@MrScothh MrScothh commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

The barometer task waits on the I2C bus for every read. On my H743 (DPS310 on I2C at 400 kHz) the BARO task runs 64 times a second, 113 us on average and 297 us at most, and nearly all of that is the CPU spinning while the status byte and the six measurement bytes go over the wire. Those 300 us land in the main loop wherever the scheduler happens to put the task.

This makes barometer reads start the transfer and come back for the result later, so the CPU doesn't wait on the bus.

The change

A small API next to the blocking one, busReadBufAsync() and busAsyncState() in drivers/bus.c. On an I2C bus that supports it the read only starts, and busAsyncState() says whether it is still on its way, done, or failed. On SPI, on software I2C and on RP2350 the read runs to the end inside busReadBufAsync(), as busReadBuf() does, and the state is reported done at once, so drivers need no special case.

How the transfer moves on, per family, reusing what each driver already has:

  • STM32F4 (bus_i2c_stm32f40x.c): the existing polled state machine, stepped from taskRunRealtimeCallbacks() instead of in a loop. Every wait of the I2Cv1 peripheral holds SCL low until software acts, so a late step only makes the transfer longer, and the one- and two-byte endings still happen within a single step.
  • STM32F7 and H7 (bus_i2c_hal.c): HAL_I2C_Mem_Read_IT(), completed by the I2C interrupt handlers that are already there. The main loop only looks at the handle's state.
  • AT32F43x (bus_i2c_at32f43x.c): the steps of i2c_memory_read() as a small polled state machine, each step taken when its flag is up instead of waited for.

One transfer per bus at a time. A blocking transfer on the same bus (compass, pitot, anything else) first lets an asynchronous one finish, so the two never collide, and a failed transfer, a NACK included, goes through the same recovery as a blocking one (error counter, i2cInit()). A transfer that stops moving for I2C_TIMEOUT is abandoned the same way; a main loop that was busy elsewhere for a while (a settings save, for example) doesn't count as the transfer being stuck.

Barometer: a driver that sets asyncRead returns false from get_up while its read is on its way, and baroUpdate() comes back 500 us later. Those extra runs don't feed the position estimator, so it gets the same updates as today. Converted: DPS310 / SPL07-003 (status, then the measurement) and BMP280 / BME280 (the measurement read; the one-byte write that starts a conversion stays blocking, so the conversion time is still counted from when it really starts). The other barometers are unchanged.

Testing

TBS Lucid H7 Wing (H743), DPS310 on I2C2 at 400 kHz, one minute each, same board and config:

maintenance-10.x this PR
BARO task, average 113 us 3 us
BARO task, max 297 us 6 us
SYSTEM task, max (where the polling is booked) 8 us 9 us
barometer samples per second 31 31
  • 3000 blocking reads of the DPS310 ID register on the same bus, one right after each barometer task run (so with a read of its own just started) and others from the realtime callbacks: all 3000 returned the right value, on both builds, with no I2C error.
  • A read from an address nobody answers: it ends as failed in 137 us (the NACK, not the timeout), the I2C error counter goes up by one as with a blocking read, and the barometer carries on at 31 samples per second.

Flash: +976 B on MATEKF405SE, +988 B on BLUEBERRYF435WING, +2,364 B on MATEKF722SE (98.65% to 99.13%), +3,504 B on MATEKH743. On F7 and H7 most of it is the HAL's interrupt driven read, which was not linked before.

Built for MATEKF405SE, MATEKF722SE, MATEKH743, TBS_LUCID_H7_WING, BLUEBERRYF435WING and SITL.

Not tested:

  • F4 and AT32F43x boards: only built. F7 has run since, on a Diatone Mamba F722 2022B (comment of 2026-10-07). The F4 and AT32 paths depend on the realtime callbacks running often enough; on a loaded CPU they still run after every gyro/PID run, so a read takes a few ms at most against a barometer period of 15-30 ms.
  • BME280 on I2C, and either sensor on SPI, where nothing should change. A BMP280 on I2C has run since, on a Holybro Kakute H7 Mini (comment of 2026-10-07).
  • A bus stuck low, or a sensor unplugged in flight.

If anyone has an F4, F7 or AT32 board with a DPS310 or BMP280 on I2C, the tasks output (the BARO line) and status (the I2C error count) before and after would tell whether it behaves the same there.

After review

  • A stuck read times out even when the barometer task looks at it less than once a millisecond: a long gap counts as 1 ms instead of restarting the timer.
  • F7 and H7 no longer step anything from the realtime callbacks: there the HAL interrupts carry a read to its end and the barometer task sees it through busAsyncState(), so the call only cost ITCM. After the merge of maintenance-10.x, LTO pulled it into scheduler() and KAKUTEF7HDV went 16 B over its ITCM. asyncfatfs: keep afatfs_poll out of the ITCM scheduler #12118 takes afatfs_poll() out of scheduler(); with it, KAKUTEF7HDV links this PR at 15 416 B. F4 and AT32 still step their reads from there.

Built on F4, F7, H7, AT32 and SITL. After the merge of maintenance-10.x, ITCM is at 16264 B on KAKUTEF7 and 16384 of 16384 B on KAKUTEF7HDV (maintenance-10.x: 16232 B on both). None of the growth is in code this PR touches: LTO lays out scheduler(), pidApplyMulticopterRateController() and gyroFilter() differently. Holybro Kakute H7 Mini with the BMP280, the same as before the change: BARO task 25 to 27 us on average, 72 us at most, altitude noise 9.5 cm, I2C error count unchanged. On the TBS Lucid H7 Wing after review: DPS310 at 31 samples/s, no I2C error after boot. A stuck read can't be caused on the bench.

Related PRs

  • Independent of my other open PRs.

busReadBufAsync() starts an I2C read and busAsyncState() tells when it has
ended, so the barometer task no longer spins on the bus. F4 steps its polled
state machine from the realtime callbacks, F7/H7 use the HAL interrupt
driven read and its existing handlers, AT32F43x gets a small polled state
machine. A blocking transfer on the same bus lets the read finish first.
DPS310 and BMP280 use it; on SPI and software I2C the read completes at once.
@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

Read barometers without blocking on supported I2C buses

✨ Enhancement 🕐 40+ Minutes

Grey Divider

AI Description

• Add asynchronous I2C reads so barometer tasks can return while transfers complete.
• Convert DPS310 and BMP280-family reads while preserving blocking behavior on unsupported buses.
• Poll pending reads without sending empty barometer updates to the position estimator.
Diagram

graph TD
    BARO["Baro scheduler"] --> SENSOR["Sensor drivers"] --> BUS["Async bus API"] --> PATH{"Backend type"} --> HAL["HAL IRQ read"] --> BARO
    PATH --> POLL["Polled I2C"] --> BARO
    PATH --> SYNC["Blocking fallback"] --> BARO
    BARO -->|Fresh result| EST["Position estimator"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. DMA-backed I2C reads
  • ➕ Could reduce interrupt or polling work during longer transfers.
  • ➖ Requires DMA resource allocation and additional platform-specific integration for short barometer reads.
2. Completion-driven barometer scheduling
  • ➕ Could eliminate fixed 500 µs checks of pending reads.
  • ➖ Requires coordinating interrupt and polled backends with the task scheduler, substantially widening the change.

Recommendation: Keep the PR's common API and existing per-platform transfer mechanisms: they avoid bus-wait CPU time without introducing DMA ownership or scheduler callbacks. Review should concentrate on completion, timeout, and competing-transfer behavior across the untested hardware paths.

Files changed (14) +559 / -25

Enhancement (14) +559 / -25
barometer.hMark barometer drivers that can return a pending read +2/-0

Mark barometer drivers that can return a pending read

• Adds an asyncRead capability flag so the barometer scheduler can distinguish an in-progress read from legacy driver behavior.

src/main/drivers/barometer/barometer.h

barometer_bmp280.cMake BMP280-family measurement reads asynchronous +16/-3

Make BMP280-family measurement reads asynchronous

• Keeps the measurement buffer alive across calls, checks transfer state on later calls, and retains the last valid values after failure. The conversion-start write remains blocking.

src/main/drivers/barometer/barometer_bmp280.c

barometer_dps310.cStage DPS310 status and measurement reads +50/-14

Stage DPS310 status and measurement reads

• Replaces consecutive blocking reads with a status-then-measurement state machine. Compensation runs only after a successful measurement read.

src/main/drivers/barometer/barometer_dps310.c

bus.cExpose asynchronous reads through the common bus API +24/-0

Expose asynchronous reads through the common bus API

• Routes I2C reads and state checks to the I2C bus adapter. Other bus types retain synchronous reads and report immediate completion.

src/main/drivers/bus.c

bus.hDeclare asynchronous bus read and state functions +5/-0

Declare asynchronous bus read and state functions

• Adds public bus-level entry points and I2C adapter declarations for starting reads and checking their state.

src/main/drivers/bus.h

bus_busdev_i2c.cBridge bus-device reads to hardware I2C async support +25/-0

Bridge bus-device reads to hardware I2C async support

• Passes device address and raw-register settings to the asynchronous I2C implementation. Builds without that support fall back to blocking reads.

src/main/drivers/bus_busdev_i2c.c

bus_i2c.hDefine supported I2C async targets and transfer states +19/-0

Define supported I2C async targets and transfer states

• Enables asynchronous hardware I2C on STM32F4/F7/H7 and AT32F43x, and declares transfer-state and polling interfaces.

src/main/drivers/bus_i2c.h

bus_i2c_at32f43x.cAdd an AT32F43x polled asynchronous read path +180/-0

Add an AT32F43x polled asynchronous read path

• Splits memory reads into flag-driven steps with progress tracking and failure recovery. Blocking transfers drain a pending read before using the same bus.

src/main/drivers/bus_i2c_at32f43x.c

bus_i2c_hal.cAdd interrupt-driven reads on STM32F7/H7 +126/-0

Add interrupt-driven reads on STM32F7/H7

• Starts HAL interrupt reads, observes completion and errors, and recovers stalled transfers. Blocking operations first wait for an outstanding read on that bus.

src/main/drivers/bus_i2c_hal.c

bus_i2c_stm32f40x.cReuse the STM32F4 I2C state machine for async reads +80/-1

Reuse the STM32F4 I2C state machine for async reads

• Allows the existing state machine to advance through realtime polls rather than a blocking loop. Records completion state and drains pending reads before blocking operations.

src/main/drivers/bus_i2c_stm32f40x.c

fc_core.cAdvance asynchronous I2C from realtime callbacks +5/-0

Advance asynchronous I2C from realtime callbacks

• Calls the I2C poller from the existing realtime callback path on targets with asynchronous I2C support.

src/main/fc/fc_core.c

fc_tasks.cSkip estimator updates for pending-read checks +4/-1

Skip estimator updates for pending-read checks

• Prevents a barometer task run that only checks an in-flight read from feeding the position estimator.

src/main/fc/fc_tasks.c

barometer.cReschedule pending barometer reads without advancing samples +22/-6

Reschedule pending barometer reads without advancing samples

• Retries asynchronous reads after 500 µs while keeping the current barometer phase. Tracks polling-only runs and restores normal task timing after completion.

src/main/sensors/barometer.c

barometer.hExpose barometer polling status +1/-0

Expose barometer polling status

• Declares the polling-status accessor used to suppress estimator updates during pending reads.

src/main/sensors/barometer.h

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

qodo-free-for-open-source-projects Bot commented Sep 28, 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. Stuck reads can lock a bus forever ✓ Resolved
Description
i2cAsyncUpdate() resets asyncProgressUs after any polling gap above I2C_ASYNC_LOOK_GAP_US,
even when the peripheral has made no transfer progress, and bypasses the I2C_TIMEOUT check on that
poll. With sustained polling intervals above 1 ms, a stalled transfer remains busy indefinitely,
leaving the barometer read unfinished and later synchronous users of the same bus waiting in the
drain loop; the AT32 implementation has the same behavior.
Code

src/main/drivers/bus_i2c_hal.c[R267-270]

+    if (state->handle.XferCount != state->asyncXferCount || cmpTimeUs(now, state->asyncLookUs) > I2C_ASYNC_LOOK_GAP_US) {
+        state->asyncXferCount = state->handle.XferCount;
+        state->asyncProgressUs = now;
+    } else if (cmpTimeUs(now, state->asyncProgressUs) > I2C_TIMEOUT) {
Evidence
In both async implementations, a polling gap above I2C_ASYNC_LOOK_GAP_US assigns the current time
to asyncProgressUs regardless of transfer progress, while the I2C_TIMEOUT check runs only in the
alternative branch. Repeated delayed polls therefore cannot reach timeout recovery, and blocking
transfers spin until the outstanding async operation ends, extending the stall to other devices on
the bus.

src/main/drivers/bus_i2c_hal.c[266-279]
src/main/drivers/bus_i2c_at32f43x.c[342-349]
src/main/drivers/bus_i2c_hal.c[174-179]
src/main/fc/fc_core.c[1109-1115]
src/main/drivers/bus_i2c_hal.c[243-244]
src/main/fc/fc_core.c[1109-1114]

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

## Issue description
A stalled asynchronous I2C transfer can remain busy indefinitely when successive polls are more than 1 ms apart: each delayed poll resets the progress timestamp without observed transfer progress and bypasses timeout recovery.
## Fix Focus Areas
- src/main/drivers/bus_i2c_hal.c[266-279]
- src/main/drivers/bus_i2c_at32f43x.c[342-349]
## Recommended Fix
In both HAL and AT32 implementations, track observed peripheral transfer progress separately from polling gaps and reset `asyncProgressUs` only when a byte or state transition occurs. Preserve an allowance for an occasional delayed poll without repeatedly erasing accumulated no-progress time, and enforce a bounded timeout when no progress is observed.

ⓘ 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 route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/main/drivers/bus_i2c_hal.c Outdated
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Test firmware build ready — commit 5e927be

Download firmware for PR #12063

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.

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

RAM / Flash usage vs. base commit e87050f — commit 5e927be

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

Target Flash Δ RAM Δ
MATEKF405 +712 B (+0.10%) CCM: ±0 B (±0.00%)
RAM: +8 B (+0.01%)
MATEKF722 +2812 B (+0.58%) ITCM_RAM: +16 B (+0.13%)
RAM: +112 B (+0.13%)
TCM: +4 B (+0.02%)
MATEKF765 +3776 B (+0.50%) DTCM_RAM: ±0 B (±0.00%)
SRAM1: +112 B (+0.09%)
MATEKH743 +3528 B (+0.44%) D2_RAM: ±0 B (±0.00%)
DTCM_RAM: ±0 B (±0.00%)
ITCM_RAM: ±0 B (±0.00%)
RAM: +128 B (+0.09%)

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

@MrScothh
MrScothh force-pushed the feature/baro-i2c-async branch 2 times, most recently from af3ee38 to e2302d4 Compare September 28, 2026 19:20
@MrScothh

MrScothh commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Tested today on a second board and barometer: a Holybro Kakute H7 Mini with its BMP280 on I2C1, this branch merged on the current maintenance-10.x against that base alone, 12 tasks samples each after 100 s of uptime.

Kakute H7 Mini, BMP280 maintenance-10.x with this PR
BARO task average 135 to 138 us 25 to 27 us
BARO task max 203 us 72 us (the forced-mode write is still a blocking 2-byte write)
I2C errors 50 (boot) 50 (boot)
System load 1 % 1 %

One thing for anyone reading tasks on this branch: the BARO line now alternates between "43 Hz, 0.6 %" and "2000 Hz, 5.5 %" from one sample to the next. The CLI prints 1 / latest delta as the rate and average x that rate as the load, and the task now runs three or four times per 23 ms cycle, two of them the 500 us polls while the 6-byte read is in flight, so a sample taken right after a poll shows 2000 Hz. The real cost is those three or four runs of about 25 us per 23 ms, around 0.3 to 0.4 % against 0.6 % before. Altitude at rest unchanged (10.7 cm of noise), barometer OK throughout.

Earlier on the Lucid's DPS310 the BARO task measured 3 us. F4 and AT32 still untested.

@MrScothh

MrScothh commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Ran on an F7 now: a Diatone Mamba F722 2022B with a DPS310 on I2C1 (baro_hardware = DPS310), against maintenance-10.x 3f9fde4, same board and config.

maintenance-10.x this PR
BARO task, average 124 us 11 us
BARO task, max 297 us 17 us
I2C errors 50 at boot, not growing 50 at boot, not growing
baro altitude at rest, 50 samples in 10 s 14.5 cm std dev 10.2 cm std dev

@sensei-hacker

sensei-hacker commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

It looks like this and #12108 are 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.

@sensei-hacker sensei-hacker mentioned this pull request Oct 7, 2026
3 of 51 tasks
@MrScothh

MrScothh commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Yes, same problem. #12108 does it for every I2C sensor and for writes too, mine only for baro reads, so I'd take #12108 as the base and close #12063 once we've sorted out what's worth keeping.

A few bits of mine might still help. On F4 I kept the existing polled driver and just step it from the realtime callbacks, which could be a fallback if the new interrupt driver gives trouble, since nobody has run that one on an F4 yet. On AT32 my read doesn't block during the address phase. And my timeout only counts the time the transfer is actually being stepped, so a long gap in the main loop doesn't kill a good read.

Two things in #12108 worth a look, @error414:

If you want the F4/AT32/timeout parts I can send them as a PR against your branch, and I'll rerun my tests on #12108 on the H743 and the F722.

@error414

error414 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Ahhh, I did not notice that similar PR is already created. You PR just change baro and my PR covers whole. Probably would be best to go somewhere beetween. In INAV is every I2C driver blocking and must be rewrite all. But do it in one big PR is not best way.

I did not F4 tested yet because I have to buy better F4 board with exposed SWD pins :(.

So my proposal is:

  1. create universal async functions in inav libraries
  2. rewrite each driver one by one

what we have to be sure about

  1. two sync drivers works on same bus (for example I2C)
  2. error of driver (for example disconecting mag) can't block whole and other drivers must work

First step and first PR could be core + baro + mag (qmc) + OLED, these devices I can test.

Could you check https://github.com/error414/inav/tree/error414/feature/ic_async/core each driver could use small state machine if needed (only steps if needs read more registers), in driver just call busReadBufStart or busWriteStart, end of transfer is handled in parent of driver for example compas.c, baro.c etc..... for special drivers like OLED we would need create parent task to handle end of transfer.

So far I don't know if I like this aproach. But it's simple neat solution which can be used for every driver. And so far I did not checked memory usage of my solution.

What do you think?

BTW: so far I did not pay attention to new changes in SPL06 driver

@MrScothh

MrScothh commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@error414 I like the plan: a core first, then the drivers one at a time, so each PR stays small enough to review and test. I went through your core branch (a7d4a0e) with your two goals in mind.

Two devices on one bus look fine from the code: one transfer per bus, a result kept per address, and blocking calls wait for the bus to go idle first. The progress-based timeout and keeping barometer_spl06.c are good changes compared to #12108.

A failing device can still hurt the others in three places:

  • F4: on a bus error with a START pending, the error interrupt waits for START and STOP and then calls i2cHandleHardwareFailure(), unstick included (bus_i2c_stm32f40x.c around line 510), and the BTF path does the same through i2cRecoverStuckTransfer() (around 610). The I2C interrupts run at priority 2, above the motor timers and their DMA at 3, so a bus fault can hold those off for up to a few ms. Setting an error and a "needs reset" flag in the interrupt and doing the reinit from i2cBusy() or the next start would avoid it.
  • AT32, and soft I2C: a start that fails on an address NACK goes through i2cHandleHardwareFailure() and returns false (bus_i2c_at32f43x.c around 294-302), which baro.c and compass.c read as "bus busy, retry in 1 ms". An unplugged sensor then means a reinit and unstick every millisecond. Recording the NACK in the per-address result and returning true would let busIsBusy() report the error instead.
  • HAL and AT32: when the bus stays busy, for example a slave holding SDA low, every start is refused (HAL_BUSY, STEP_1) and nothing runs the recovery, since the stuck check only looks at transfers in flight. With only async devices on that bus the sensors go quiet without an error.

Smaller things: the AT32 address phase still busy-waits; the doc says a NACK doesn't reinit the peripheral, but AT32 and HAL (other than HAL_BUSY) do; and nothing checks the BMP280 trigger write, so a NACKed trigger followed by a good read gives a stale sample.

From #12063, the AT32 read that also does the address phase without blocking would cover the AT32 points, and I can bring it over. I can also test the core on an H743 (DPS310, BMP280) and an F722 (DPS310) with the same tests I used for #12063, the NACK one included. I have no QMC, OLED or F4 here.

@error414

error414 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

I just wanted to show direction how to make it universal. In specific driver just call aync reading, state machine in parent driver like baro.c or compas.c

I knew that in my commit will be bugs :).

Are you going to change this pr, or should I create new one?

@MrScothh

MrScothh commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@error414 Better you open it: it's your design, and you have the F4, QMC and OLED to test it on. If you want, I'll bring #12063's AT32 read that doesn't block onto it, and test the core here with the tests I used for #12063, the NACK one included. What I have on I2C:

  • H743: a DPS310 on two TBS Lucid H7 Wings, one of them with an IST8310 compass in its GPS module, and a BMP280 on a Kakute H7 Mini;
  • F722: a DPS310 on a Diatone Mamba F722 and on a Flywoo F722 Pro V2, which INAV detects as SPL06 (no genuine SPL06 here).

#12063 can stay open until yours is in, then I'll close it.

@MrScothh

Copy link
Copy Markdown
Contributor Author

Flight data: the four flights of 2026-10-17 and 18 on a fixed wing (21 minutes in the air), with this PR at its current
head (5e927be). DPS310 on I2C2, which that board ran at 800 kHz (i2c2_speed from #12148):

flight new baro values longest stretch with the same value baro health baro temperature
1 16/s 1.3 s (level flight) OK 100 % 35-45 C
2 16/s 0.5 s OK 100 % 35-45 C
3 16/s 0.6 s OK 100 % 35-48 C
4 17/s 0.3 s OK 100 % 38-53 C

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