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

Ensure the model base link is set to the correct model #9

Merged

Conversation

srmainwaring
Copy link
Collaborator

This PR fixes a bug where the base model link associated with the ArduPilotPlugin may be incorrectly associated with the wrong model, leading to an incorrect pose being published. The issue is apparent in multi-vehicle simulations.

The fix is to ensure that the link is set in the same if clause as the IMU topic.

Before fix

When running a multi-vehicle simulation, some vehicles will report incorrect position leading to poor EFK status and a loss of control of the vehicle in controlled modes.

After fix

All vehicles will report the correct position and expected EKF status. Tested with a 3-Iris quadcopter configuration.

multi-vehicle-test_small

- Set the link for the IMU contained in this model - the ECM foreach iterates over all model components in the world.

Signed-off-by: Rhys Mainwaring <[email protected]>
@srmainwaring srmainwaring added the bug Something isn't working label Jan 18, 2022
Copy link
Collaborator

@khancyr khancyr left a comment

Choose a reason for hiding this comment

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

I don't know the current code, but it looks right. We should ensure that the IMU grandparent is the model where the imu is attached on

@srmainwaring srmainwaring merged commit a805ef2 into ArduPilot:ignition-edifice Jan 19, 2022
@srmainwaring
Copy link
Collaborator Author

Thanks for the review @khancyr. The check could be relaxed to be any link in the sub-tree of the model rather than require the IMU is a child of the base link, but that's for another day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants