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_Periph arm timeout bug #84

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

robertlong13
Copy link
Collaborator

Cherry picking the arm timeout feature was incomplete. Was causing the motor to stop every 2s periodically.

@robertlong13 robertlong13 force-pushed the OV3-1027-ap-periph-arm-timeout-bug branch from 46922d8 to 3748b56 Compare December 6, 2023 07:11
@robertlong13
Copy link
Collaborator Author

@loki077 I had to force-push a fix real quick because I was using #if instead of #ifdef for HAL_PERIPH_ARM_MONITORING_ENABLE.

Did we go the #ifdef route to avoid needing to put

#ifndef HAL_PERIPH_ARM_MONITORING_ENABLE
 # define HAL_PERIPH_ARM_MONITORING_ENABLE          0
#endif

in a couple places?

This implementation will cause unexpected results if someone defines HAL_PERIPH_ARM_MONITORING_ENABLE 0 in the hwdef for testing or something. Not a big deal in the near-term, especially for this cherry-pick of the functionality, but I wanted to flag that.

@loki077
Copy link
Contributor

loki077 commented Dec 6, 2023

Thanks for finding the missing piece. @robertlong13

will discuss the point you mentioned tomorrow in the office.

@robertlong13 robertlong13 force-pushed the OV3-1027-ap-periph-arm-timeout-bug branch from 3748b56 to 9be53e2 Compare December 6, 2023 22:34
This flag was never getting set to true

OV3-1027
@robertlong13 robertlong13 force-pushed the OV3-1027-ap-periph-arm-timeout-bug branch from 9be53e2 to 1393257 Compare December 6, 2023 22:35
@Pradeep-Carbonix Pradeep-Carbonix merged commit d9da9cc into CxPilot Dec 6, 2023
@robertlong13 robertlong13 deleted the OV3-1027-ap-periph-arm-timeout-bug branch June 17, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants