Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_Proximity: LD19/LD06 data length sanity check fix #29375

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

boa-pe
Copy link

@boa-pe boa-pe commented Feb 25, 2025

AP_Rangefinder: LD19/LD06 data length sanity check fix

root cause: number of data points in frame stored only on 5bits. value been used without proper bitmask
to sanity check number of datapoints in frame

@tpwrules tpwrules requested a review from peterbarker February 25, 2025 15:04
@rmackay9
Copy link
Contributor

Hi @boa-pe,

thanks for this. one small thing, could you modify the commit title to start with "AP_Rangefinder: "? We do this because it makes backporting easier because we can immediately see which subsystem a change affects. To be clear I mean the commit title, not the PR title. thanks!

@boa-pe boa-pe changed the title LD19/LD06 data length sanity check fix AP_Rangefinder: LD19/LD06 data length sanity check fix Feb 25, 2025
@boa-pe boa-pe force-pushed the bugfix/ld19_sanity_check_fix branch 2 times, most recently from 3d70595 to bb4c3c4 Compare February 25, 2025 23:42
@boa-pe
Copy link
Author

boa-pe commented Feb 25, 2025

Hi @boa-pe,

thanks for this. one small thing, could you modify the commit title to start with "AP_Rangefinder: "? We do this because it makes backporting easier because we can immediately see which subsystem a change affects. To be clear I mean the commit title, not the PR title. thanks!

Hmmm.... Shouldn't that be AP_Proximity?

@boa-pe boa-pe force-pushed the bugfix/ld19_sanity_check_fix branch from bb4c3c4 to e381273 Compare February 25, 2025 23:56
@peterbarker
Copy link
Contributor

Hmmm.... Shouldn't that be AP_Proximity?

Yes, it definitely should be :-)

@rmackay9
Copy link
Contributor

oops :-)

@peterbarker peterbarker force-pushed the bugfix/ld19_sanity_check_fix branch from e381273 to 3f71876 Compare February 26, 2025 05:45
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Tested on my LD06 and it works!

Many thanks for chasing this one down properly. I should have gone to the datasheet :-/

I've fixed the commit message and marking for merge.

@peterbarker peterbarker changed the title AP_Rangefinder: LD19/LD06 data length sanity check fix AP_Proximity: LD19/LD06 data length sanity check fix Feb 26, 2025
@peterbarker peterbarker merged commit 3863535 into ArduPilot:master Feb 26, 2025
102 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants