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

Further simplify timer usage flags. #9288

Merged
merged 4 commits into from
Sep 22, 2023
Merged

Conversation

mmosca
Copy link
Collaborator

@mmosca mmosca commented Sep 10, 2023

A servo is a servo, no matter what the platform.
A motor is a motor, no matter what the platform.

Enum values changed, and compatibility macros added.

Since the enum is not used in configuration, I don't expect regressions from this particular change.

A servo is a servo, no matter what the platform.
A motor is a motor, no matter what the platform.

Enum values changed, and compatibility macros added.

Since the enum is not used in configuration, I don't expect
regressions from this particular changes.
@mmosca
Copy link
Collaborator Author

mmosca commented Sep 10, 2023

Needs updates to the configurator, since it checks for the old bits for motors/servos.

@stronnag
Copy link
Collaborator

Regardless, legacy tricopter looks better (latest on this branch and master configurator (6900ed57d / 1821)
image

@mmosca
Copy link
Collaborator Author

mmosca commented Sep 10, 2023

Regardless, legacy tricopter looks better (latest on this branch and master configurator (6900ed57d / 1821) image

This firmware, with master configurator will break for FW ;)

I unified all the MOTOR/SERVO USE_TIM to the bits formelly assigned to MC.

@mmosca mmosca marked this pull request as ready for review September 10, 2023 18:08
@mmosca mmosca added this to the 7.0 milestone Sep 10, 2023
@mmosca
Copy link
Collaborator Author

mmosca commented Sep 21, 2023

@DzikuVx I fixed the conflict, but I don't have access to hardware to test until at least saturday.

If you can just check that nothing outrageously wrong happened, it should be ready to merge.

The conflict was just the the changes needed for he global override, which was removed.

There are some targets where the motor for fixed wing was not on the first outputs that may see a different output mapping in fixed wing mode with this change though, but in most cases I checked, they seemed to be quad centric flight controllers, so hopefully, not a super common issue.

@mmosca mmosca merged commit 03734bc into master Sep 22, 2023
@sensei-hacker
Copy link
Collaborator

sensei-hacker commented Sep 22, 2023

There are some targets where the motor for fixed wing was not on the first outputs that may see a different output mapping in fixed wing mode with this change though

It might be worth noting that four of those boards were set up that way because they need to be.
For example:

DALRCF722DUAL
DEF_TIM(TIM8, CH1, PC6, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0),
DEF_TIM(TIM8, CH2, PC7, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0),
DEF_TIM(TIM8, CH3, PC8, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0),
DEF_TIM(TIM8, CH4, PC9, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0),
DEF_TIM(TIM1, CH1, PA8, TIM_USE_MC_MOTOR | TIM_USE_FW_MOTOR,0,0),
DEF_TIM(TIM1, CH2, PA9, TIM_USE_MC_MOTOR | TIM_USE_FW_MOTOR,0,0),

These four boards will not be flyable in fixed with the default "motors first".

The other three affected are:
Skystars405HD
PIXRACER
AIKONF4

Some other boards will be okay either way. Others had definitions that were completely non-sensical and wouldn't work, with motors and servos on the same channel.

@mmosca
Copy link
Collaborator Author

mmosca commented Sep 22, 2023

There are some targets where the motor for fixed wing was not on the first outputs that may see a different output mapping in fixed wing mode with this change though

It might be worth noting that four of those boards were set up that way because they need to be. For example:

DALRCF722DUAL DEF_TIM(TIM8, CH1, PC6, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0), DEF_TIM(TIM8, CH2, PC7, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0), DEF_TIM(TIM8, CH3, PC8, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0), DEF_TIM(TIM8, CH4, PC9, TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,0,0), DEF_TIM(TIM1, CH1, PA8, TIM_USE_MC_MOTOR | TIM_USE_FW_MOTOR,0,0), DEF_TIM(TIM1, CH2, PA9, TIM_USE_MC_MOTOR | TIM_USE_FW_MOTOR,0,0),

These four boards will not be flyable in fixed with the default "motors first".

The other three affected are: Skystars405HD PIXRACER AIKONF4

Some other boards will be okay either way. Others had definitions that were completely non-sensical and wouldn't work, with motors and servos on the same channel.

You can get the same effect as INAV 6, by overriding one of the timers.

AIKONF4 is another example.

    DEF_TIM(TIM8,  CH1,  PC6,  TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,                     0, 0), // S1
    DEF_TIM(TIM8,  CH2,  PC7,  TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,                     0, 0), // S2
    DEF_TIM(TIM8,  CH3,  PC8,  TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,                     0, 0), // S3
    DEF_TIM(TIM8,  CH4,  PC9,  TIM_USE_MC_MOTOR | TIM_USE_FW_SERVO,                     0, 0), // S4
    DEF_TIM(TIM3,  CH3,  PB0,  TIM_USE_MC_MOTOR | TIM_USE_MC_SERVO | TIM_USE_FW_MOTOR,  0, 0), // S5
    DEF_TIM(TIM3,  CH4,  PB1,  TIM_USE_MC_MOTOR | TIM_USE_MC_SERVO | TIM_USE_FW_MOTOR,  0, 0), // S6

With this change, it will allocate TIM8 as motor, and TIM3 as servo by default, for FW and MC.

User would need to override TIM8 as servo, or TIM3 as motor to get INAV 6 FW allocation.

If we set the timer with 2 outputs to motor I believe that would result in M1-M4 matching S1-S3 if you are setting up something with more than 2 motors. This is probably the less disruptive way to do it.

I have another pr in the oven to go over all targets servo/motor allocations. We can probably move the discussion on how to address these there.

@mmosca
Copy link
Collaborator Author

mmosca commented Sep 22, 2023

#9309 will have the target updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants