refactor(ina226): rewrite driver for robustness and clarity#27364
Open
dakejahl wants to merge 2 commits into
Open
refactor(ina226): rewrite driver for robustness and clarity#27364dakejahl wants to merge 2 commits into
dakejahl wants to merge 2 commits into
Conversation
Mirrors the INA238 rewrite (#27359): - Non-blocking UNINITIALIZED -> RESET -> CONFIGURE -> MEASURE state machine. Each transition is its own RunImpl tick with an explicit ScheduleDelayed; init failures retry without losing the driver instance. - Tolerate ~2 s of consecutive collect() failures (MAX_CONSECUTIVE_FAILURES = DISCONNECT_DEBOUNCE_US / SAMPLE_INTERVAL_US) before a full reinit. Isolated I2C glitches just skip a cycle instead of dropping battery_status. - SAMPLE_INTERVAL_US = 100 ms, comfortably above the default ADC period of ~75.3 ms (588 us * 2 channels * 64 avg). MEASURE compensates for in-tick I2C time so each tick is exactly one interval apart. - CONFIGURE waits SAMPLE_INTERVAL_US + 5 ms before the first MEASURE read so the first averaged sample is ready. - checkConfigurationRotating() reads one of {CONFIGURATION, CALIBRATION} per cycle and compares it against the value we wrote, so an externally reset device is detected within two cycles. - '-t' arg validated against 1-3 at parse time; out-of-range values now exit with an error. - New perf counters ina226_bad_register and ina226_reinit for field diagnosis; current driver state is surfaced in 'ina226 status'. File layout: ina226_main.cpp folded into ina226.cpp. Constants/enums namespaced under ina226. Params switched to DEFINE_PARAMETERS, and the INA226_CONFIG raw-register override is removed in favor of a hardcoded continuous-conversion config. AuterionAutostarter::ina226_probe() updated to the new namespaced ina226::Register and ina226::MANFID / ina226::DIEID constants. Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
The INA226 CONFIGURATION register has D15 (RST) self-clearing and D14:D12 reserved, with D14 reading back as 1 regardless of what is written. The verify step compared the raw read-back to the value we wrote, so the check failed on every cycle and the driver re-initialized itself every ~2 s. Mask the non-R/W bits before comparing. Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
github-actions
Bot
added
kind:refactor
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 0 byte (0 %)]px4_fmu-v6x [Total VM Diff: 104 byte (0.01 %)]Updated: 2026-05-18T01:25:45 |
julianoes
approved these changes
May 18, 2026
| max: 65535 | ||
| decimal: 1 | ||
| increment: 1 | ||
| reboot_required: true |
Contributor
There was a problem hiding this comment.
Odd, wasn't aware of this one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rewrite of the INA226 driver to modernize the code layout (state machine, namespaced constants,
DEFINE_PARAMETERS, file consolidation) and pick up the same robustness fixes from #27359.Problem
The INA226 driver shares the same anti-patterns the INA238 PR identified as causes of intermittent
battery_statusdropouts:collect()flipped_initialized = falseand re-ranReset()inline on the next tick — at 10 Hz that lost a sample period on every isolated I2C glitch.setConnected(false)could fire repeatedly per failed cycle, collapsing the intended ~2 s debounce.-kwas set; with-k, the only recovery path was the same blocking re-init.-t(battery index) was parsed with no range check; out-of-range values silently clamped to 1 insideBattery.INA226_CONFIGparam exposed the raw config register to users, with a default that disagreed with the C#definedefault (param defaulted to triggered mode; macro defaulted to continuous).Solution
UNINITIALIZED → RESET → CONFIGURE → MEASUREstate machine. Each transition is its ownRunImpltick with an explicitScheduleDelayed; init failures retry without losing the driver instance.collect()failures (MAX_CONSECUTIVE_FAILURES = DISCONNECT_DEBOUNCE_US / SAMPLE_INTERVAL_US) before a full reinit. Isolated glitches just skip a cycle.SAMPLE_INTERVAL_US = 100 ms, comfortably above the default ADC period of ~75.3 ms (588 µs × 2 channels × 64 avg).MEASUREcompensates for in-tick I2C time so each tick is exactlySAMPLE_INTERVAL_USapart.CONFIGUREwaitsSAMPLE_INTERVAL_US + 5 msbefore the firstMEASUREread so the first averaged sample is ready.checkConfigurationRotating()reads one of{CONFIGURATION, CALIBRATION}per cycle and compares it against the value we wrote, so an externally reset device is detected within two cycles.-targ validated against 1–3 at parse time; out-of-range values now exit with an error.ina226_bad_registerandina226_reinitfor field diagnosis; current driver state is surfaced inina226 status.ina226_main.cppfolded intoina226.cpp. Constants/enums namespaced underina226. Params switched toDEFINE_PARAMETERS.INA226_CONFIGraw-register override is removed; the driver hardcodes continuous-conversion mode with the same588 µs / 588 µs / 64-avgdefaults the C macro previously specified.AuterionAutostarter::ina226_probe()updated to the new namespaced constants.Testing
Tested with an ARK PAB Power Module. Induced I2S bus glitches by momentarily shorting SDA/SCL to GND to trigger bus errors. Shorting for >2s triggers a full re-init.
https://review.px4.io/plot_app?log=015893d0-bb6a-47b9-8a7a-9e7dd19525cf