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

Plane: ensure the dshot type gets set #27093

Merged
merged 1 commit into from
May 22, 2024

Conversation

andyp1per
Copy link
Collaborator

@andyp1per andyp1per commented May 18, 2024

When in plane only mode the dshot type was not getting set late enough meaning that BlueJay ESCs would not work.

I worry that RC speed also does not appear to be being set.

@andyp1per andyp1per added the BUG label May 18, 2024
@tridge
Copy link
Contributor

tridge commented May 18, 2024

@andyp1per the main thing with the ordering is for IOMCU. Can you please check this works with a plane with Q_ENABLE=0 both on main and aux outputs on a board with an IOMCU?

@andyp1per
Copy link
Collaborator Author

Verified on a CubeOrangePlus MAIN1 with BlueJay

@IamPete1
Copy link
Member

Why is the existing setting in AP_BLHeli not sufficient? If it does not work there should we remove it? What about all the other setup that is done in AP_BLHeli init function, does that work correctly?

Setting the same thing in two places seem like a recipe for future bugs to me.

@andyp1per
Copy link
Collaborator Author

Why is the existing setting in AP_BLHeli not sufficient? If it does not work there should we remove it? What about all the other setup that is done in AP_BLHeli init function, does that work correctly?

Setting the same thing in two places seem like a recipe for future bugs to me.

There already were many previous bugs related to this. The blheli stuff was always done lazily which was one of the problems. I really have no interest in trying to prove whether the call is safe to remove in blheli - far too much time spent in those codepaths already!

@IamPete1
Copy link
Member

Maybe you could at least explain why this fix works and the current code does not?

@andyp1per
Copy link
Collaborator Author

Maybe you could at least explain why this fix works and the current code does not?

I can tell you the symptoms that this fixes. The current code ends up setting the dshot output frequency using the default bitwidths rather than the blheli_s bitwidths, but at some later(?) point the correct bitwidths are set and you end up in this situation where the bitwidths being used are not the same as the ones that were used for the frequency calculation.

As to the exact interaction that causes this - I have no idea. Note that copter sets the output mode together with this a couple of times - and always has - and that's why it gets the correct values at the right time (see AP_Motors::rc_set_freq).

It's possible that simply by moving the call up in AP_BLHeli that might also work.

@tridge tridge merged commit 17083b5 into ArduPilot:master May 22, 2024
61 checks passed
@IamPete1 IamPete1 removed the DevCallEU label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Pending
Development

Successfully merging this pull request may close these issues.

None yet

3 participants