fix(commander): convert the baro timestamp delta to seconds for the home altitude filter [1.18] - #28535
Merged
dakejahl merged 1 commit intoSep 4, 2026
Conversation
…ome altitude filter (PX4#28415) The low pass filter smoothing barometric altitude for the in-air home position correction was fed the raw timestamp difference. uORB timestamps are microseconds, but AlphaFilter::setParameters() documents both of its arguments as seconds and the filter is constructed with a 5 second time constant, so the sample interval arrived a million times too large: alpha = dt / (tau + dt) rate dt alpha (before) alpha (intended) 50 Hz 0.0200 s 0.999750 0.003984 100 Hz 0.0100 s 0.999500 0.001996 200 Hz 0.0050 s 0.999001 0.000999 At an alpha of 0.9997 the filter passes essentially every raw sample through, giving an effective time constant of 5 us instead of 5 s, so _lpf_baro.getState() has been effectively unfiltered barometric altitude. That state feeds the in-air home altitude correction: it is offset by _baro_gps_static_offset and then compared against the GNSS altitude, and home.alt is shifted when the two differ by more than kAltitudeDifferenceThreshold. A GNSS velocity integral gates that comparison for consistency. The same conversion is already done correctly for the GNSS integral a few lines below in this file, and for the geoid height filter in EKF2. Note that this does change behaviour: the filter now actually applies its 5 s time constant, so _lpf_baro.getState() lags during a climb by roughly the time constant times the climb rate. _baro_gps_static_offset is captured once when the correction window opens, so that lag does not cancel and it biases baro_alt_corrected while climbing. Reviewers who know this feature should say whether the 5 s constant and the 1 m threshold, both tuned while the filter was effectively a pass-through, still want the same values now that it filters. Assisted-by: Claude:claude-fable-5 Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu> (cherry picked from commit 0bdf8c2)
github-actions
Bot
added
kind:bug
dakejahl
approved these changes
Sep 4, 2026
dakejahl
left a comment
dakejahl
left a comment
Contributor
There was a problem hiding this comment.
wasn't there a follow-on PR from this?
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 0 byte (0 %)]px4_fmu-v6x [Total VM Diff: -8 byte (-0 %)]Updated: 2026-09-04T01:37:12 |
Contributor
Author
|
There was, #28421 and then #28452 on top of it. #28421 is the API change, the filter takes microseconds and every caller passes the timestamp delta straight in, so the unit mistake cannot come back in new code. That belongs on main for now, the release branch only needs the fix, which this PR is. #28452 is up as #28536, adapted to the seconds API here. If you want the API change on 1.18 as well, I can work on it :) |
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.
Backport of #28415 to
release/1.18, the same commit as on main.The timestamp delta went into the home altitude filter in the wrong unit.