iNavFlight / inav

INAV: Navigation-enabled flight control software
https://inavflight.github.io
GNU General Public License v3.0
3.14k stars 1.48k forks source link

Further cleanup of pwmInit - target-dependent channel maps for Multicopter+Servo configurations #271

Closed digitalentity closed 6 years ago

digitalentity commented 8 years ago

Currently we have a few possibilities of output channel mappings: 1) Multicopter w/o servos 2) Multicopter + servo outputs 3) Airplane

While 1 and 3 are straightforward - multiPPM, multiPWM, airPPM and airPWM mappings are defined for them, the 2 is handled within pwmInit() and we have loads of target-specific code like this:

#if defined(NAZE)
            // remap PWM9+10 as servos
            if ((timerIndex == PWM9 || timerIndex == PWM10) && timerHardwarePtr->tim == TIM1)
                type = MAP_TO_SERVO_OUTPUT;
#endif

#if defined(COLIBRI_RACE) || defined(LUX_RACE)
            // remap PWM1+2 as servos
            if ((timerIndex == PWM6 || timerIndex == PWM7 || timerIndex == PWM8 || timerIndex == PWM9) && timerHardwarePtr->tim == TIM2)
                type = MAP_TO_SERVO_OUTPUT;
#endif

I suggest extending channel mapping furhter: multiPPM and multiPWM for multicopters w/o servo outputs multiPPM_servo and multiPWM_servo for multicopters with servo outputs (tricopters included) airPPM and airPWM for airplanes

As a second step I suggest creating a callback from pwmInit() to target-specific target.c to avoid ifdefs like these:

#ifdef CC3D_PPM1
            if (init->useOneshot || isMotorBrushed(init->motorPwmRate)) {
                ppmAvoidPWMTimerClash(timerHardwarePtr, TIM4);
            }
#endif
#ifdef OLIMEXINO_UNCUT_LED2_E_JUMPER
        // PWM2 is connected to LED2 on the board and cannot be connected unless you cut LED2_E
        if (timerIndex == PWM2)
            continue;
#endif

#ifdef STM32F10X
        // skip UART2 ports
        if (init->useUART2 && (timerIndex == PWM3 || timerIndex == PWM4))
            continue;
#endif

@martinbudden what do you think?

martinbudden commented 8 years ago

@digitalentity , yes, it would certainly be good to see pwmInit cleaned up. I'm not sure the best approach though.

I don't really like the multiPPM, multiPWM, airPPM and airPWM arrays, so I'm not sure if extending them is the best approach - perhaps it's better to get rid of them altogether and replace them with your idea of a target specific callback.

But to be honest, I'm not really sure of the best approach - I need to think about this more.

DzikuVx commented 8 years ago

I agree with @martinbudden here. It should be completely refactored to be usable.

martinbudden commented 8 years ago

@digitalentity , some thought is also being given to this idea by @blckmn as part of the betaflight release, see PR https://github.com/betaflight/betaflight/pull/491

I don't think we should do any work in this area until after the F4 port - doing so would make bringing F4 code across from betaflight much more difficult. The aim should be to get a shared solution used by betaflight and iNav which can then be ported back to cleanflight.

digitalentity commented 8 years ago

@martinbudden agreed, if possible we should walk the same path with Betaflight to allow code portability.

martinbudden commented 8 years ago

I've added the blocked label, since this depends on changes that come with F4 support, issue https://github.com/iNavFlight/inav/issues/274

martinbudden commented 8 years ago

See also PR https://github.com/betaflight/betaflight/pull/1189 - Initial IO remapping

DzikuVx commented 6 years ago

@digitalentity I belive we close that