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

AR_Motors: fix brushed motor support for omni vehicles #29187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mattp256
Copy link

Fixes an issue where omni motors (BrushedWithRelay/BrushedBiplor) are not configured correctly.

I ran into the same problem another user mentioned here: OmniX frame using MOT_PWM_TYPE = 3 (BrushedWithRelay) only outputs RC servo pulses instead of a duty cycle PWM.

Proposed fix worked great when tested with Cytron MDD10A motor driver.

Fixes an issue where omni motors (BrushedWithRelay/BrushedBiplor)
are not configured correctly because _motors_num is always zero at
the time the loop executes, due to initialization order.
@IamPete1
Copy link
Member

This looks like a init ordering issue to me. We setup the frame type here:

But we do the PWM setup a few lines above here:

So _motors_num is not set when its used. Can you test if moving setup_omni to the top of the function fixes it too.

@mattp256
Copy link
Author

Yes exactly, _motors_num is not yet set at the time it is used in setup_pwm_type. Moving setup_omni ahead of setup_pwm_type is also confirmed to fix the problem. I was worried there may be some other unintended side effect if I changed the init order so thought forcing the for loop to iterate may be the least disruptive fix - I suppose that carries its own risk of side effects though. Let me know if you prefer changing the init order instead and I will update the PR.

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.

2 participants