From fa75db126cec748bac19229142cfacc99107828d Mon Sep 17 00:00:00 2001 From: Josh Stewart Date: Fri, 16 Jun 2017 17:20:24 +1000 Subject: [PATCH] MISRA compliant idle.ino --- speeduino/idle.h | 4 +- speeduino/idle.ino | 135 ++++++++++++++++++++++-------------------- speeduino/scheduler.h | 2 +- 3 files changed, 73 insertions(+), 68 deletions(-) diff --git a/speeduino/idle.h b/speeduino/idle.h index 234eea06..929a7755 100644 --- a/speeduino/idle.h +++ b/speeduino/idle.h @@ -25,7 +25,7 @@ struct StepperIdle volatile unsigned long stepStartTime; //The time the curren }; -#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega2561__) +#if defined(CORE_AVR) #define IDLE_COUNTER TCNT4 #define IDLE_COMPARE OCR4C @@ -46,7 +46,7 @@ struct StepperIdle #define IDLE_COMPARE 0 #define IDLE_TIMER_ENABLE() - #define IDLE_TIMER_DISABLE() + #define IDLE_TIMER_DISABLE() #endif diff --git a/speeduino/idle.ino b/speeduino/idle.ino index 28bb9563..af41d507 100644 --- a/speeduino/idle.ino +++ b/speeduino/idle.ino @@ -19,12 +19,12 @@ void initialiseIdle() { //By default, turn off the PWM interrupt (It gets turned on below if needed) IDLE_TIMER_DISABLE(); - #if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega2561__) //AVR chips use the ISR for this + #if defined(CORE_AVR) //AVR chips use the ISR for this //No timer work required for AVRs. Timer is shared with the schedules and setup in there. #elif defined (CORE_TEENSY) - if(configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_OL || configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_CL) + if( (configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_OL) || (configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_CL) ) { //FlexTimer 2 is used for idle FTM2_MODE |= FTM_MODE_WPDIS; // Write Protection Disable @@ -154,7 +154,6 @@ void initialiseIdle() iacCrankStepsTable.axisX = configPage4.iacCrankBins; iacStepTime = configPage4.iacStepTime * 1000; - //homeStepper(); //Returns the stepper to the 'home' position completedHomeSteps = 0; idleStepper.curIdleStep = 0; idleStepper.stepperStatus = SOFF; @@ -181,6 +180,10 @@ void initialiseIdle() idlePID.SetTunings(configPage3.idleKP, configPage3.idleKI, configPage3.idleKD); idlePID.SetMode(AUTOMATIC); //Turn PID on break; + + default: + //Well this just shouldn't happen + break; } idleInitComplete = configPage4.iacAlgorithm; //Sets which idle method was initialised currentStatus.idleLoad = 0; @@ -242,48 +245,53 @@ void idleControl() case IAC_ALGORITHM_STEP_OL: //Case 4 is open loop stepper control //First thing to check is whether there is currently a step going on and if so, whether it needs to be turned off - if( checkForStepping() ) { return; } //If this is true it means there's either a step taking place or - if( !isStepperHomed() ) { return; } //Check whether homing is completed yet. - - //Check for cranking pulsewidth - if( BIT_CHECK(currentStatus.engine, BIT_ENGINE_CRANK) ) + if( isStepperHomed() && !checkForStepping() ) //Check that homing is complete and that there's not currently a step already taking place { - //Currently cranking. Use the cranking table - idleStepper.targetIdleStep = table2D_getValue(&iacCrankStepsTable, (currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET)) * 3; //All temps are offset by 40 degrees. Step counts are divided by 3 in TS. Multiply back out here - doStep(); - } - else if( (currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET) < iacStepTable.axisX[IDLE_TABLE_SIZE-1]) - { - //Standard running - if ((mainLoopCount & 255) == 1) + //Check for cranking pulsewidth + if( BIT_CHECK(currentStatus.engine, BIT_ENGINE_CRANK) ) { - //Only do a lookup of the required value around 4 times per second. Any more than this can create too much jitter and require a hyster value that is too high - idleStepper.targetIdleStep = table2D_getValue(&iacStepTable, (currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET)) * 3; //All temps are offset by 40 degrees. Step counts are divided by 3 in TS. Multiply back out here - iacStepTime = configPage4.iacStepTime * 1000; + //Currently cranking. Use the cranking table + idleStepper.targetIdleStep = table2D_getValue(&iacCrankStepsTable, (currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET)) * 3; //All temps are offset by 40 degrees. Step counts are divided by 3 in TS. Multiply back out here + doStep(); } - doStep(); + else if( (currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET) < iacStepTable.axisX[IDLE_TABLE_SIZE-1]) + { + //Standard running + if ((mainLoopCount & 255) == 1) + { + //Only do a lookup of the required value around 4 times per second. Any more than this can create too much jitter and require a hyster value that is too high + idleStepper.targetIdleStep = table2D_getValue(&iacStepTable, (currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET)) * 3; //All temps are offset by 40 degrees. Step counts are divided by 3 in TS. Multiply back out here + iacStepTime = configPage4.iacStepTime * 1000; + } + doStep(); + } + currentStatus.idleLoad = idleStepper.curIdleStep >> 1; //Current step count (Divided by 2 for byte) } - currentStatus.idleLoad = idleStepper.curIdleStep >> 1; //Current step count (Divided by 2 for byte) break; case IAC_ALGORITHM_STEP_CL://Case 5 is closed loop stepper control //First thing to check is whether there is currently a step going on and if so, whether it needs to be turned off - if( checkForStepping() ) { return; } //If this is true it means there's either a step taking place or - if( !isStepperHomed() ) { return; } //Check whether homing is completed yet. - if( (idleCounter & 31) == 1) + if( isStepperHomed() && !checkForStepping() ) //Check that homing is complete and that there's not currently a step already taking place { - //This only needs to be run very infrequently, once every 32 calls to idleControl(). This is approx. once per second - idlePID.SetTunings(configPage3.idleKP, configPage3.idleKI, configPage3.idleKD); - iacStepTime = configPage4.iacStepTime * 1000; + if( (idleCounter & 31) == 1) + { + //This only needs to be run very infrequently, once every 32 calls to idleControl(). This is approx. once per second + idlePID.SetTunings(configPage3.idleKP, configPage3.idleKI, configPage3.idleKD); + iacStepTime = configPage4.iacStepTime * 1000; + } + + idle_cl_target_rpm = table2D_getValue(&iacClosedLoopTable, currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET) * 10; //All temps are offset by 40 degrees + idlePID.Compute(); + idleStepper.targetIdleStep = idle_pid_target_value; + + doStep(); + currentStatus.idleLoad = idleStepper.curIdleStep >> 1; //Current step count (Divided by 2 for byte) + idleCounter++; } + break; - idle_cl_target_rpm = table2D_getValue(&iacClosedLoopTable, currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET) * 10; //All temps are offset by 40 degrees - idlePID.Compute(); - idleStepper.targetIdleStep = idle_pid_target_value; - - doStep(); - currentStatus.idleLoad = idleStepper.curIdleStep >> 1; //Current step count (Divided by 2 for byte) - idleCounter++; + default: + //There really should be a valid idle type break; } } @@ -296,6 +304,7 @@ False: If the motor has not yet been homed. Will also perform another homing ste */ static inline byte isStepperHomed() { + bool isHomed = true; //As it's the most common scenario, default value is true if( completedHomeSteps < (configPage4.iacStepHome * 3) ) //Home steps are divided by 3 from TS { digitalWrite(pinStepperDir, STEPPER_BACKWARD); //Sets stepper direction to backwards @@ -304,9 +313,9 @@ static inline byte isStepperHomed() idleStepper.stepperStatus = STEPPING; completedHomeSteps++; idleOn = true; - return false; + isHomed = false; } - return true; + return isHomed; } /* @@ -317,7 +326,8 @@ False: If the motor is ready for another step */ static inline byte checkForStepping() { - if(idleStepper.stepperStatus == STEPPING || idleStepper.stepperStatus == COOLING) + bool isStepping = false; + if( (idleStepper.stepperStatus == STEPPING) || (idleStepper.stepperStatus == COOLING) ) { if(micros() > (idleStepper.stepStartTime + iacStepTime) ) { @@ -327,7 +337,7 @@ static inline byte checkForStepping() digitalWrite(pinStepperStep, LOW); //Turn off the step idleStepper.stepStartTime = micros(); idleStepper.stepperStatus = COOLING; //'Cooling' is the time the stepper needs to sit in LOW state before the next step can be made - return true; + isStepping = true; } else { @@ -339,10 +349,10 @@ static inline byte checkForStepping() else { //Means we're in a step, but it doesn't need to turn off yet. No further action at this time - return true; + isStepping = true; } } - return false; + return isStepping; } /* @@ -350,36 +360,31 @@ Performs a step */ static inline void doStep() { - if ( idleStepper.targetIdleStep > (idleStepper.curIdleStep - configPage4.iacStepHyster) && idleStepper.targetIdleStep < (idleStepper.curIdleStep + configPage4.iacStepHyster) ) { return; } //Hysteris check - else if(idleStepper.targetIdleStep < idleStepper.curIdleStep) { digitalWrite(pinStepperDir, STEPPER_BACKWARD); idleStepper.curIdleStep--; }//Sets stepper direction to backwards - else if (idleStepper.targetIdleStep > idleStepper.curIdleStep) { digitalWrite(pinStepperDir, STEPPER_FORWARD); idleStepper.curIdleStep++; }//Sets stepper direction to forwards + if ( (idleStepper.targetIdleStep <= (idleStepper.curIdleStep - configPage4.iacStepHyster)) || (idleStepper.targetIdleStep >= (idleStepper.curIdleStep + configPage4.iacStepHyster)) ) //Hysteris check + { + if(idleStepper.targetIdleStep < idleStepper.curIdleStep) { digitalWrite(pinStepperDir, STEPPER_BACKWARD); idleStepper.curIdleStep--; }//Sets stepper direction to backwards + else if (idleStepper.targetIdleStep > idleStepper.curIdleStep) { digitalWrite(pinStepperDir, STEPPER_FORWARD); idleStepper.curIdleStep++; }//Sets stepper direction to forwards - digitalWrite(pinStepperEnable, LOW); //Enable the DRV8825 - digitalWrite(pinStepperStep, HIGH); - idleStepper.stepStartTime = micros(); - idleStepper.stepperStatus = STEPPING; - idleOn = true; + digitalWrite(pinStepperEnable, LOW); //Enable the DRV8825 + digitalWrite(pinStepperStep, HIGH); + idleStepper.stepStartTime = micros(); + idleStepper.stepperStatus = STEPPING; + idleOn = true; + } } //This function simply turns off the idle PWM and sets the pin low static inline void disableIdle() { - if(configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_CL || configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_OL) + if( (configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_CL) || (configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_OL) ) { IDLE_TIMER_DISABLE(); digitalWrite(pinIdle1, LOW); } - else if (configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_CL || configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_OL) + else if ( (configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_CL) || (configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_OL) ) { digitalWrite(pinStepperEnable, HIGH); //Disable the DRV8825 idleStepper.targetIdleStep = idleStepper.curIdleStep; //Don't try to move anymore - //The below appears to be causing issues, so for now this will simply halt the stepper entirely rather than taking it back to step 1 - /* - idleStepper.targetIdleStep = 1; //Home the stepper - if( checkForStepping() ) { return; } //If this is true it means there's either a step taking place or - if( !isStepperHomed() ) { return; } //Check whether homing is completed yet. - doStep(); - */ } } @@ -387,22 +392,22 @@ static inline void disableIdle() //Typically this is enabling the PWM interrupt static inline void enableIdle() { - if(configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_CL || configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_OL) + if( (configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_CL) || (configPage4.iacAlgorithm == IAC_ALGORITHM_PWM_OL) ) { IDLE_TIMER_ENABLE(); } - else if (configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_CL || configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_OL) + else if ( (configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_CL) || (configPage4.iacAlgorithm == IAC_ALGORITHM_STEP_OL) ) { } } -#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega2561__) //AVR chips use the ISR for this +#if defined(CORE_AVR) //AVR chips use the ISR for this ISR(TIMER4_COMPC_vect) #elif defined (CORE_TEENSY) || defined (CORE_STM32) static inline void idleInterrupt() //Most ARM chips can simply call a function #endif -#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega2561__) || defined (CORE_TEENSY) +#if defined(CORE_AVR) || defined (CORE_TEENSY) { if (idle_pwm_state) { @@ -410,13 +415,13 @@ static inline void idleInterrupt() //Most ARM chips can simply call a function { //Normal direction *idle_pin_port &= ~(idle_pin_mask); // Switch pin to low (1 pin mode) - if(configPage4.iacChannels) { *idle2_pin_port |= (idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 + if(configPage4.iacChannels == 1) { *idle2_pin_port |= (idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 } else { //Reversed direction *idle_pin_port |= (idle_pin_mask); // Switch pin high - if(configPage4.iacChannels) { *idle2_pin_port &= ~(idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 + if(configPage4.iacChannels == 1) { *idle2_pin_port &= ~(idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 } IDLE_COMPARE = IDLE_COUNTER + (idle_pwm_max_count - idle_pwm_cur_value); idle_pwm_state = false; @@ -427,13 +432,13 @@ static inline void idleInterrupt() //Most ARM chips can simply call a function { //Normal direction *idle_pin_port |= (idle_pin_mask); // Switch pin high - if(configPage4.iacChannels) { *idle2_pin_port &= ~(idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 + if(configPage4.iacChannels == 1) { *idle2_pin_port &= ~(idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 } else { //Reversed direction *idle_pin_port &= ~(idle_pin_mask); // Switch pin to low (1 pin mode) - if(configPage4.iacChannels) { *idle2_pin_port |= (idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 + if(configPage4.iacChannels == 1) { *idle2_pin_port |= (idle2_pin_mask); } //If 2 idle channels are in use, flip idle2 to be the opposite of idle1 } IDLE_COMPARE = IDLE_COUNTER + idle_pwm_target_value; idle_pwm_cur_value = idle_pwm_target_value; diff --git a/speeduino/scheduler.h b/speeduino/scheduler.h index b8a67720..824e9681 100644 --- a/speeduino/scheduler.h +++ b/speeduino/scheduler.h @@ -26,7 +26,7 @@ See page 136 of the processors datasheet: http://www.atmel.com/Images/doc2549.pd #define SCHEDULER_H -#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega2561__) +#if defined(CORE_AVR) #include #include