Skip to content

autotest: print beacon position delta as float, not truncated %u#33558

Open
peterbarker wants to merge 1 commit into
ArduPilot:masterfrom
peterbarker:pr-claude2/beacon-delta-print-fix
Open

autotest: print beacon position delta as float, not truncated %u#33558
peterbarker wants to merge 1 commit into
ArduPilot:masterfrom
peterbarker:pr-claude2/beacon-delta-print-fix

Conversation

@peterbarker

Copy link
Copy Markdown
Contributor

Summary

Adjusts output to add more precision

Classification & Testing (check all that apply and add your own)

  • Checked by a human programmer
  • Non-functional change
  • No-binary change
  • Infrastructure change (e.g. unit tests, helper scripts)
  • Automated test(s) verify changes (e.g. unit test, autotest)
  • Tested manually, description below (e.g. SITL)
  • Tested on hardware
  • Logs attached
  • Logs available on request

Description

BeaconPosition() (also exercised by the Replay Beacon bit) waits for the EKF position to settle within max_delta=1 m of the pre-switch GPS fix and logs the distance with "delta=%u". pos_delta is a float in metres, so %u truncated it: a genuine 1.55 m miss printed as "delta=1", which reads as though it satisfied "want <= 1" yet the test still timed out -- actively misleading when diagnosing the beacon-convergence flake. Print %.2f.

Discovered as part of chasing this outlier down:

image

Comment thread Tools/autotest/arducopter.py Outdated
Comment on lines +10200 to +10202
# :.2f, not %u: pos_delta is a float distance in metres; %u
# truncated it, so e.g. a real 1.55 m miss printed as "delta=1"
# and looked like it satisfied "want <= 1".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this comment is unnecessary.
But also OK as-is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it really wasn't. Fixed.

BeaconPosition() (also exercised by the Replay Beacon bit) waits for the
EKF position to settle within max_delta=1 m of the pre-switch GPS fix and
logs the distance with "delta=%u".  pos_delta is a float in metres, so %u
truncated it: a genuine 1.55 m miss printed as "delta=1", which reads as
though it satisfied "want <= 1" yet the test still timed out -- actively
misleading when diagnosing the beacon-convergence flake.  Print %.2f.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@peterbarker peterbarker force-pushed the pr-claude2/beacon-delta-print-fix branch from a3b0795 to e99a0c6 Compare June 25, 2026 23:15

@tpwrules tpwrules left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this just make the issue happen at 1.004? Can you think of any reason to limit the number of digits at all? Do you think that is a likely case?

@peterbarker

Copy link
Copy Markdown
Contributor Author

Does this just make the issue happen at 1.004? Can you think of any reason to limit the number of digits at all? Do you think that is a likely case?

Pretty sure we have reasonable epsilons here, so we don't need all the digits.

@tpwrules

tpwrules commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

I do not see any epsilon in the next line.

I'm not hoping to be difficult, just pointing out that according to my understanding this PR does not fully solve the problem laid out in the description. If you think it's close enough I'm fine, just want to make sure we share an understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants