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

EntityBase::requestSpeedChange sets target_speed only when it's reached #1392

Open
gmajrobotec opened this issue Sep 23, 2024 · 0 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@gmajrobotec
Copy link
Contributor

Description
The statement target_speed_ = target_speed.getAbsoluteValue(getCanonicalizedStatus(), other_status_); in line 483 sets the target speed only in cases where the target speed has already been reached, which seems counter intuitive. This statement’s position is inconsistent with lines 469

Solution
Line 483 should be moved down, outside of the “if” statement. This way the most up to date target speed will be set on each iteration until the target speed is reached. Then it will be consistent with line 469.

void EntityBase::requestSpeedChange(
  const speed_change::RelativeTargetSpeed & target_speed, bool continuous)
{
  if (!continuous && isTargetSpeedReached(target_speed)) {
    return;
  }
  if (continuous) {
    job_list_.append(
      /**
       * @brief If the target entity reaches the target speed, return true.
       */
      [this, target_speed](double) {
        if (other_status_.find(target_speed.reference_entity_name) == other_status_.end()) {
          return true;
        }
        target_speed_ = target_speed.getAbsoluteValue(getCanonicalizedStatus(), other_status_);
        return false;
      },
      [this]() {}, job::Type::LINEAR_VELOCITY, true, job::Event::POST_UPDATE);
  } else {
    job_list_.append(
      /**
       * @brief If the target entity reaches the target speed, return true.
       */
      [this, target_speed](double) {
        if (other_status_.find(target_speed.reference_entity_name) == other_status_.end()) {
          return true;
        }
        if (isTargetSpeedReached(target_speed)) {
-          target_speed_ = target_speed.getAbsoluteValue(getCanonicalizedStatus(), other_status_);
          return true;
        }
+          target_speed_ = target_speed.getAbsoluteValue(getCanonicalizedStatus(), other_status_);
        return false;
      },
      /**
       * @brief Cancel speed change request.
       */
      [this]() { target_speed_ = std::nullopt; }, job::Type::LINEAR_VELOCITY, true,
      job::Event::POST_UPDATE);
  }
}
@hakuturu583 hakuturu583 added the bug Something isn't working label Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants