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

Jump to bottom

Privatize AP_VisualOdom #16460

Merged
merged 2 commits into from Feb 9, 2021
Merged

Privatize AP_VisualOdom #16460

merged 2 commits into from Feb 9, 2021

Conversation

hendjoshsr71
Copy link
Member

Privatizes visual odometry logging

@peterbarker
Copy link
Contributor

@amilcarlucas it still seems to be there?
image

@hendjoshsr71 needs a rebase....

@hendjoshsr71
Copy link
Member Author

Rebased... for the moment

peterbarker
peterbarker approved these changes Feb 5, 2021
View reviewed changes
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.

LGTM - we've merged others....

@rmackay9
Copy link
Contributor

rmackay9 commented Feb 7, 2021

@hendjoshsr71 has this been tested? It's probably fine but we'd like all changes tested. It should be possible to test in SITL and the wiki has instructions on how to run a simulated vicon. https://ardupilot.org/dev/docs/using-sitl-for-ardupilot-testing.html#testing-vicon-aka-vision-positioning

@hendjoshsr71
Copy link
Member Author

@rmackay9 Sorry about that. I should have asked in regards to testing this one.

I didn't find a way through searching the wiki to simulate the input for the VISO log message. Is there a setting for SITL to generate mavlink vision_position_delta messages?

private_VISV_VISP

@tridge
Copy link
Contributor

tridge commented Feb 9, 2021

we need something that shows that this has been tested

@hendjoshsr71
Copy link
Member Author

hendjoshsr71 commented Feb 9, 2021 •
edited

Here are before and after graphs for this PR using a SITL mission. Note the VISO messages required additional code to SITL as the Vicon simulation was not sending MavLink vision_position_delta messages. PR here #16572.

Master
pr_master_VISO_with_sitl_vicon_pos_delta

After PR
pr_privateLog_VISO_with_sitl_vicon_pos_delta

Master
pr_master_VISP_with_sitl_vicon_pos_delta

After PR
pr_privateLog_VISP_with_sitl_vicon_pos_delta

Master
pr_master_VISV_with_sitl_vicon_pos_delta

After PR
pr_privateLog_VISV_with_sitl_vicon_pos_delta

@rmackay9 rmackay9 merged commit 8135d9e into ArduPilot:master Feb 9, 2021
@rmackay9
Copy link
Contributor

rmackay9 commented Feb 9, 2021

Merged, thanks!

@hendjoshsr71 hendjoshsr71 deleted the pr-private-VisualOdom branch March 24, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khancyr khancyr khancyr approved these changes

@peterbarker peterbarker peterbarker approved these changes

@amilcarlucas amilcarlucas amilcarlucas approved these changes

Assignees
No one assigned
Labels
NeedsTesting
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@hendjoshsr71 @peterbarker @rmackay9 @tridge @khancyr @amilcarlucas @CraigElder