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_Motors: Heli: Single: rework tail rotor handling #25603

Merged
merged 3 commits into from
Dec 12, 2023

Conversation

IamPete1
Copy link
Member

This is a none-functional change to re-work tail rotor handling into what is hopefully easier to understand. This moves the tail types to be a enum class and uses switch statements where possible.

Helper functions have been added to check if DDFP and direct drive var pitch.

Yaw offset had been moved to its own function.

As usual the motors example has been updated to allow testing of different tail types, it shows no difference before vs after.

Single_Heli H3 Servo_only:
        Inputs max change:
                Roll: 0.000000
                Pitch: 0.000000
                Yaw: 0.000000
                Thr: 0.000000
        Outputs max change:
                Mot1: 0.000000
                Mot2: 0.000000
                Mot3: 0.000000
                Mot4: 0.000000
                Mot5: 0.000000
                Mot6: 0.000000
                Mot7: 0.000000
                Mot8: 0.000000
        Limits max change:
                LimR: 0.000000
                LimP: 0.000000
                LimY: 0.000000
                LimThD: 0.000000
                LimThU: 0.000000


Single_Heli H3 Servo_with_ExtGyro:
        Inputs max change:
                Roll: 0.000000
                Pitch: 0.000000
                Yaw: 0.000000
                Thr: 0.000000
        Outputs max change:
                Mot1: 0.000000
                Mot2: 0.000000
                Mot3: 0.000000
                Mot4: 0.000000
                Mot5: 0.000000
                Mot6: 0.000000
                Mot7: 0.000000
                Mot8: 0.000000
        Limits max change:
                LimR: 0.000000
                LimP: 0.000000
                LimY: 0.000000
                LimThD: 0.000000
                LimThU: 0.000000


Single_Heli H3 DirectDrive_VarPitch:
        Inputs max change:
                Roll: 0.000000
                Pitch: 0.000000
                Yaw: 0.000000
                Thr: 0.000000
        Outputs max change:
                Mot1: 0.000000
                Mot2: 0.000000
                Mot3: 0.000000
                Mot4: 0.000000
                Mot5: 0.000000
                Mot6: 0.000000
                Mot7: 0.000000
                Mot8: 0.000000
        Limits max change:
                LimR: 0.000000
                LimP: 0.000000
                LimY: 0.000000
                LimThD: 0.000000
                LimThU: 0.000000


Single_Heli H3 DirectDrive_FixedPitch_CW:
        Inputs max change:
                Roll: 0.000000
                Pitch: 0.000000
                Yaw: 0.000000
                Thr: 0.000000
        Outputs max change:
                Mot1: 0.000000
                Mot2: 0.000000
                Mot3: 0.000000
                Mot4: 0.000000
                Mot5: 0.000000
                Mot6: 0.000000
                Mot7: 0.000000
                Mot8: 0.000000
        Limits max change:
                LimR: 0.000000
                LimP: 0.000000
                LimY: 0.000000
                LimThD: 0.000000
                LimThU: 0.000000


Single_Heli H3 DirectDrive_FixedPitch_CCW:
        Inputs max change:
                Roll: 0.000000
                Pitch: 0.000000
                Yaw: 0.000000
                Thr: 0.000000
        Outputs max change:
                Mot1: 0.000000
                Mot2: 0.000000
                Mot3: 0.000000
                Mot4: 0.000000
                Mot5: 0.000000
                Mot6: 0.000000
                Mot7: 0.000000
                Mot8: 0.000000
        Limits max change:
                LimR: 0.000000
                LimP: 0.000000
                LimY: 0.000000
                LimThD: 0.000000
                LimThU: 0.000000


Single_Heli H3 DDVP_with_external_governor:
        Inputs max change:
                Roll: 0.000000
                Pitch: 0.000000
                Yaw: 0.000000
                Thr: 0.000000
        Outputs max change:
                Mot1: 0.000000
                Mot2: 0.000000
                Mot3: 0.000000
                Mot4: 0.000000
                Mot5: 0.000000
                Mot6: 0.000000
                Mot7: 0.000000
                Mot8: 0.000000
        Limits max change:
                LimR: 0.000000
                LimP: 0.000000
                LimY: 0.000000
                LimThD: 0.000000
                LimThU: 0.000000

@IamPete1 IamPete1 changed the title AP_Motors: Heli: Single: rework tail rotor headlining AP_Motors: Heli: Single: rework tail rotor handling Nov 22, 2023
@MattKear
Copy link
Contributor

MattKear commented Nov 27, 2023

Are you thinking of this as a starting PR and doing more later as incremental changes?

The reason I ask is because IMHO it would be nice if we moved tail into it's own class much like RSC and swashplate. If we used a backend and front end style we could sperate out the different tail types quite cleanly. This way we could handle the params for the different tail types separately with enables (i think we have 1 level of params left which would be enough to do this>) We could also move all of the switch case statements out of Heli_Single and into "TailRotor.cpp".

In any case, I like this rework, as it can just be a stepping stone to what I mentioned above, if others feel there is value in that?

@IamPete1
Copy link
Member Author

Are you thinking of this as a starting PR and doing more later as incremental changes?

I started off doing a class, however its not as clean as the swash stuff. Firstly because its only used for single heli there is no duplicated code. Secondly it uses params and values from motors so we would have to add hooks to get the data we need.

On the whole I think the bigger re-work will cost a bunch of flash and have little benefit. Especially if we have to move around params.

However there are certainly tidy ups that could follow this PR.

@bnsgeyer
Copy link
Contributor

@IamPete1

I started off doing a class, however it’s not as clean as the swash stuff. Firstly because it’s only used for single heli there is no duplicated code.

This actually could be useful because we have other single main rotor styles that could become separate heli frames and take on their own class. We have the pulse motor which could be its own frame and I have always been looking to put a compound heli frame which could make use of a tail type class

So this may be something to consider in the future.

@IamPete1
Copy link
Member Author

IamPete1 commented Dec 8, 2023

I have added the ability to set the COL2YAW param and the autorotation flag from the test. I re-ran the testes with no COL2YAW and no autorotation, COL2YAW only and COL2YAW with autorotation. As before, no change in output.

@bnsgeyer
Copy link
Contributor

@IamPete1 Thanks for including COL2YAW in your tests. Did you use a non-zero value for the COL2YAW parameter? if so what was the value?

@IamPete1
Copy link
Member Author

@IamPete1 Thanks for including COL2YAW in your tests. Did you use a non-zero value for the COL2YAW parameter? if so what was the value?

I used 1, I figured so long as the value was not 0 any number would prove equivalence before vs after this change.

Copy link
Contributor

@bnsgeyer bnsgeyer left a comment

Choose a reason for hiding this comment

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

LGTM

@tridge tridge merged commit 6d546ee into ArduPilot:master Dec 12, 2023
88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants