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_Compass: stop CAN auto replacement by default add option to re-enable #23706

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented May 6, 2023

Currently if you have multiple CAN compasses but only use and calibrate the first if that compass ever does not show up at boot it will be replaced by the second. This removes that ability so compasses cannot change ordering unexpectedly. This can happen if the flight controller is powered from USB and the primary compass from battery.

Because the second compass had not been calibrated its dev-id is never saved, this means the expected_dev_id is 0 and it is detected as a replacement compass.

This can be avoided by setting all to use and calibrating all and then deselecting use for the unwanted compasses.

A alternate solution would be to save all dev-id's in a calibration to "lock" the ordering, however we would then also need to force offsets to 0 for the un-calibrated compasses so they still show up as un-calibrated.

TLDR: You have to got to a GCS to calibrate the new compass that has come in as a replacement anyway, I don't see that manually removing the old one and moving the new one up is significantly more effort.

@pompecukor
Copy link

@IamPete1 Thank you for not giving up on this so easily. From you explanation, now I understand why you first found our problem hard to replicate. It need the other compasses not to be calibrated previously at all. Makes sense.

@tridge
Copy link
Contributor

tridge commented May 9, 2023

@bugobliterator can you have a look like this?

@@ -1462,62 +1462,6 @@ void Compass::_detect_backends(void)
}
#endif
}

#if COMPASS_MAX_UNREG_DEV > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a COMPASS_OPTIONS bit, maybe "DisableDroneCANReplacement" which can be set on vehicles where the GPS/compass combos are powered separately from the main flight control MCU

@tridge tridge removed the DevCallEU label May 24, 2023
@IamPete1
Copy link
Member Author

I have done some more playing about, QGC does not allow you to remove missing compasses. On this patch starting a calibration does not clear any missing compasses. So we cannot remove this code without requiring users who genuinely need to remove a mission compass to use mission planner.

I'm still of the view that this is strange and un-expected behavior. I suspect reports are going to become more common, especially with new devices, such as this, https://holybro.com/collections/flight-controller-accessories/products/can-hub

I'm happy to make this a parameter, but I don't think current behavior should be the default. Much better to cause a little extra effort to those who are replacing a compass than to cause compasses to disappear and calibrations to be lost for those who are operating normally.

@IamPete1 IamPete1 force-pushed the compass_remove_CAN_autoreplacment branch from 7c9e82a to e7dbf23 Compare June 18, 2023 18:26
@IamPete1
Copy link
Member Author

I have added a compass option which defaults to not do auto replacement.

@IamPete1 IamPete1 changed the title AP_Compass: remove CAN auto replacement AP_Compass: stop CAN auto replacement by default add option to re-enable Jun 25, 2023
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.

We're worried about support load but we're going to give it a try.

@tridge
Copy link
Contributor

tridge commented Jun 26, 2023

approved with some trepidation

@tridge tridge merged commit 262b11f into ArduPilot:master Jun 26, 2023
@tridge tridge added the WikiNeeded needs wiki update label Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants