Selective031 / RazorBoard

GNU General Public License v3.0
8 stars 2 forks source link

MotorBackward function limitations #12

Closed MagnusPer closed 3 years ago

MagnusPer commented 3 years ago

The MotorBackward function has limitation for how long it could run in time.,

#define MotorMaxSpeed 2950  // 3359 is max PWM for 25Khz at 84 MHz with APB1 Clock
#define MotorMinSpeed 2000        // Minimum speed to start with

in my case I want to to back harder and for longer time, but the function don't allow that due to two reasons. MotorBackward(MotorMinSpeed+500, MotorMaxSpeed, 3000);

  1. if (currentSpeed >= maxSpeed) { break; }
  2. if (timeCount >= time_ms) { break; }

My expectation is that when maxSpeed has been reached the time left shall be used to be driven till it has elapsed.

void MotorBackward(uint16_t minSpeed, uint16_t maxSpeed, uint16_t time_ms) {

    State = BACKWARD;
    uint16_t timeCount = 0;

for (uint16_t currentSpeed = minSpeed; currentSpeed < maxSpeed; currentSpeed++) {

      currentSpeed += 3;
      if (currentSpeed >= maxSpeed) {
          break;
      }

      TIM4->CCR1 = currentSpeed;
      TIM4->CCR2 = 0;

      TIM4->CCR3 = 0;
      TIM4->CCR4 = currentSpeed;

      HAL_Delay(1);
      timeCount++;

      CheckSecurity();

      if (timeCount >= time_ms) {
          break;
      }
 }
Selective031 commented 3 years ago

Correct, this is one of the small quirks that needs to be fixed. The fix will be something like this, at the beginning of the function before the loop starts, create a timer, if the timer reaches it end during the windup, break. If there is still time left when the windup if done, loop and check security until the timer reaches the end, then break.

Selective031 commented 3 years ago

This is untested, but something like this:

void MotorBackward(uint16_t minSpeed, uint16_t maxSpeed, uint16_t time_ms) {

    uint32_t motor_timer;
    State = BACKWARD;
//  uint16_t timeCount = 0;

    motor_timer = HAL_GetTick();

for (uint16_t currentSpeed = minSpeed; currentSpeed < maxSpeed; currentSpeed++) {

      currentSpeed += 3;
      if (currentSpeed >= maxSpeed) {
          break;
      }

      TIM4->CCR1 = currentSpeed;
      TIM4->CCR2 = 0;

      TIM4->CCR3 = 0;
      TIM4->CCR4 = currentSpeed;

      HAL_Delay(1);
//    timeCount++;

//    CheckSecurity();

      if (HAL_GetTick() - motor_timer >= time_ms || CheckSecurity() != SECURITY_OK) {
          break;
      }
 }
while (HAL_GetTick() - motor_timer < time_ms) {
    if (CheckSecurity() != SECURITY_OK) break;
}
    MotorStop();
}
Selective031 commented 3 years ago

The fix should also be included in MotorLeft and MotorRight as well.

MagnusPer commented 3 years ago

Yes - you are correct in also update this for other functions (MotorLeft/Right) as well

One other idea to make the function even more configurable is also to include step-up current_speed +=3 as a argument, but that may be to much, don't really know my self if that is needed or not? Maybe a future enhancement if any need that functionality.

Selective031 commented 3 years ago

The step-up count is mainly because of different motors, RPM and power.. you want it to windup slowly and not too fast, so its a balance, but it could be moved to settings,

Selective031 commented 3 years ago

A fix has been committed to dev branch, which will fix this.