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

Correct handling of more than 256 fence items #28683

Merged
merged 8 commits into from
Nov 25, 2024

Conversation

peterbarker
Copy link
Contributor

No description provided.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

LGTM. Have you checked for any other callers to max_items? Just wondering if there are downstream users (like Dijkstras) that may not have been exposed to more than 255 fence items

@peterbarker
Copy link
Contributor Author

LGTM. Have you checked for any other callers to max_items? Just wondering if there are downstream users (like Dijkstras) that may not have been exposed to more than 255 fence items

I did grep for max_items in the code to spot obvious problems there.

I didn't stop to think about avoidance. I just have and it seems to use uint16_t throughout

@peterbarker
Copy link
Contributor Author

.... Dijkstra does seem to impose a 255 point limit, however.
OA_DIJKSTRA_POLYGON_SHORTPATH_NOTSET_IDX

@peterbarker
Copy link
Contributor Author

Found and fixed a fence-transfered-via-FTP bug in here now.

We also were not gracefully handling an attempt to upload a fence with >255 vertexes - that's been fixed, too.

@peterbarker
Copy link
Contributor Author

In addition to the new autotests I've done a bunch of tootling around in SITL to make sure fences still generally work. Including Dijkstras with 765 points.

@peterbarker peterbarker merged commit 650b978 into ArduPilot:master Nov 25, 2024
99 checks passed
@peterbarker peterbarker deleted the pr/fence-on-sd-card branch November 25, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Pending
Development

Successfully merging this pull request may close these issues.

3 participants