Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2c17f0b
Add velocity saturation checks and clamping
sachinkum0009 Jul 10, 2026
8fe1ab0
Merge branch 'master' into feat/jerk-limit-implicit-vel
sachinkum0009 Jul 10, 2026
a87a707
Remove redundant velocity saturation logic
sachinkum0009 Jul 10, 2026
95f64db
Recalculate effective acceleration limits based on dt
sachinkum0009 Jul 10, 2026
34ef75e
test added when impl vel exceeds max vel, limits are enforced with
sachinkum0009 Jul 13, 2026
635117e
Merge branch 'master' into feat/jerk-limit-implicit-vel
sachinkum0009 Jul 13, 2026
4c01a70
Merge branch 'master' into feat/jerk-limit-implicit-vel
sachinkum0009 Jul 14, 2026
4db6261
Use RCLCPP_WARN_THROTTLE instead of stream variant
sachinkum0009 Jul 14, 2026
ffac33a
Optimize implicit velocity check in joint saturation limiter
sachinkum0009 Jul 14, 2026
1e007cc
Scale acceleration and deceleration limits uniformly
sachinkum0009 Jul 14, 2026
6925a6e
Add test case for derivative saturation in joint limiter
sachinkum0009 Jul 14, 2026
43e907e
Update joint saturation limiter test values
sachinkum0009 Jul 14, 2026
eaa4ae5
Add code to check implicit position and limit max vel if exceeds
sachinkum0009 Jul 15, 2026
11f6542
Merge branch 'master' into feat/jerk-limit-implicit-vel
sachinkum0009 Jul 15, 2026
da37967
Fix velocity limit check in joint saturation limiter
sachinkum0009 Jul 15, 2026
393804c
Fix comparison against max_position in saturation limiter
sachinkum0009 Jul 15, 2026
550050e
Add tests for position and velocity limit enforcement
sachinkum0009 Jul 15, 2026
7d0b1b5
Merge branch 'master' into feat/jerk-limit-implicit-vel
sachinkum0009 Jul 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions joint_limits/include/joint_limits/joint_saturation_limiter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ class JointSaturationLimiter : public JointLimiterInterface<JointLimitsStateData
bool on_configure(const JointLimitsStateDataType & current_joint_states) override
{
prev_command_ = current_joint_states;
const auto num_joints = this->number_of_joints_;
prev_dt_seconds_ = 0.0;
effective_max_acc_.assign(num_joints, 0.0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need here additional variables? Why? If this is just calculated, we don't need to keep the values. And if we are just storing them locally within a method, this should be on a heap and should not initialize any memory.

This makes sense as we need to later adjust all joint togeather.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I decided to remove the dt recalculation logic as in RT system dt should be constant. Now will remove these additional variables and update the on_enforce to update the max acc/dec when implicit vel exceeds the max vel.

Pls LMK if is approach is good?

effective_max_dec_.assign(num_joints, 0.0);

return true;
}

Expand Down Expand Up @@ -91,6 +96,11 @@ class JointSaturationLimiter : public JointLimiterInterface<JointLimitsStateData
rclcpp::Clock::SharedPtr clock_;
JointLimitsStateDataType prev_command_;
std::mutex mutex_;

private:
std::vector<double> effective_max_acc_;
std::vector<double> effective_max_dec_;
double prev_dt_seconds_;
};

template <typename JointLimitsStateDataType>
Expand Down
62 changes: 53 additions & 9 deletions joint_limits/src/joint_saturation_limiter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,53 @@ bool JointSaturationLimiter<trajectory_msgs::msg::JointTrajectoryPoint>::on_enfo
return false;
}

// TODO(gwalck) compute if the max are not implicitly violated with the given dt
// e.g. for max vel 2.0 and max acc 5.0, with dt >0.4
// velocity max is implicitly already violated due to max_acc * dt > 2.0
// if the dt is changed then recalculate the max_acceleration/max_deceleration if implicit vel

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it mean that dt is changend?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this also a problem for the velocity too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is also problem for velocity. I updated the code to check if max vel * dt > max position, then cal min reduction ratio for all joints max_position/max vel * dt, then update the vel limits by reduction ratio factor.

// exceeds max vel
if (std::fabs(dt_seconds - prev_dt_seconds_) > 1e-9)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance that the dt won't be changed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Earlier, I was thinking as the enforce fn has dt argument, then the user can change it. But now I think for the real-time control system, dt should be constant. I will remove this comparison of dt and only update the params max_acceleration and max_deceleration to effective acc/dec when impl vel exceeds the max acc/dec.
Pls LMK if this approach is good?

{
for (size_t i = 0; i < number_of_joints_; ++i)
{
if (joint_limits_[i].has_velocity_limits && joint_limits_[i].has_acceleration_limits)
{
const double implicit_vel = joint_limits_[i].max_acceleration * dt_seconds;
if (implicit_vel > joint_limits_[i].max_velocity)
{
RCLCPP_WARN_STREAM_THROTTLE(
node_logging_itf_->get_logger(), *clock_, ROS_LOG_THROTTLE_PERIOD,
"Joint '" << joint_names_[i] << "': dt (" << dt_seconds
<< ") is too large for max_acceleration ("
<< joint_limits_[i].max_acceleration << ") and max_velocity ("
<< joint_limits_[i].max_velocity << "); max_acc * dt = " << implicit_vel
<< " exceeds max_velocity");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this to non STREAM version please. STREAM versions are not realtime safe

effective_max_acc_[i] = joint_limits_[i].max_velocity / dt_seconds;
}
else
{
effective_max_acc_[i] = joint_limits_[i].max_acceleration;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might cause a problem if done only for one joint. We should scale down then all other joints appropriately...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are for now not scaling down also in other cases, then we don't have to do now, but in the followup PR, as this is important to not change robot trajectories.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I am thinking of calculating the max_reduction_factor = max_vel/implicit_vel, for all the joints and then use the min max_reduction_factor to scale down all the joints acc/dec.

Pls LMK if this approach is good?

}
}
if (joint_limits_[i].has_velocity_limits && joint_limits_[i].has_deceleration_limits)
{
const double implicit_vel = joint_limits_[i].max_deceleration * dt_seconds;
if (implicit_vel > joint_limits_[i].max_velocity)
{
RCLCPP_WARN_STREAM_THROTTLE(
node_logging_itf_->get_logger(), *clock_, ROS_LOG_THROTTLE_PERIOD,
"Joint '" << joint_names_[i] << "': dt (" << dt_seconds
<< ") is too large for max_deceleration ("
<< joint_limits_[i].max_deceleration << ") and max_velocity ("
<< joint_limits_[i].max_velocity << "); max_dec * dt = " << implicit_vel
<< " exceeds max_velocity");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here non STREAM version

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the code for non stream version

effective_max_dec_[i] = joint_limits_[i].max_velocity / dt_seconds;
}
else
{
effective_max_dec_[i] = joint_limits_[i].max_deceleration;
}
}
}
prev_dt_seconds_ = dt_seconds;
}

// check for required inputs combination
const bool has_desired_position = (desired_joint_states.positions.size() == number_of_joints_);
Expand Down Expand Up @@ -185,17 +229,17 @@ bool JointSaturationLimiter<trajectory_msgs::msg::JointTrajectoryPoint>::on_enfo
// limit deceleration
if (joint_limits_[index].has_deceleration_limits)
{
limit_applied = apply_acc_or_dec_limit(
joint_limits_[index].max_deceleration, desired_acc, limited_jnts_dec);
limit_applied =
apply_acc_or_dec_limit(effective_max_dec_[index], desired_acc, limited_jnts_dec);
deceleration_limit_applied = true;
}
}

// limit acceleration (fallback to acceleration if no deceleration limits)
if (joint_limits_[index].has_acceleration_limits && !deceleration_limit_applied)
{
limit_applied = apply_acc_or_dec_limit(
joint_limits_[index].max_acceleration, desired_acc, limited_jnts_acc);
limit_applied =
apply_acc_or_dec_limit(effective_max_acc_[index], desired_acc, limited_jnts_acc);
}

if (limit_applied)
Expand Down Expand Up @@ -253,11 +297,11 @@ bool JointSaturationLimiter<trajectory_msgs::msg::JointTrajectoryPoint>::on_enfo
double stopping_deccel = std::fabs(desired_vel[index] / dt_seconds);
if (joint_limits_[index].has_deceleration_limits)
{
stopping_deccel = joint_limits_[index].max_deceleration;
stopping_deccel = effective_max_dec_[index];
}
else if (joint_limits_[index].has_acceleration_limits)
{
stopping_deccel = joint_limits_[index].max_acceleration;
stopping_deccel = effective_max_acc_[index];
}

double stopping_distance =
Expand Down
33 changes: 31 additions & 2 deletions joint_limits/test/test_joint_saturation_limiter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,15 @@ TEST_F(JointSaturationLimiterTest, when_within_limits_expect_no_limits_applied_w
// within limits
desired_joint_states_.positions[0] = 1.0;
desired_joint_states_.velocities[0] = 1.5; // valid pos derivative as well
desired_joint_states_.accelerations[0] = 2.9; // valid pos derivative as well
desired_joint_states_.accelerations[0] = 2.0; // valid pos derivative as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @destogl, when recalculating the max acc/dec if impl vel exceed max vel, this test fails. So I have updated the test. I want to ask if it is fine or should I do something else?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pleaes create a new test for this and call it "when_derivative_too_high_expect_lower_effective_limits".

Also make the exact values checks.

ASSERT_FALSE(joint_limiter_->enforce(current_joint_states_, desired_joint_states_, period));

// check if no limits applied
CHECK_STATE_SINGLE_JOINT(
desired_joint_states_, 0,
1.0, // pos unchanged
1.5, // vel unchanged
2.9 // acc = vel / 1.0
2.0 // acc = vel / 1.0
);
}
}
Expand Down Expand Up @@ -517,6 +517,35 @@ TEST_F(JointSaturationLimiterTest, when_deceleration_exceeded_with_no_maxdec_exp
}
}

TEST_F(JointSaturationLimiterTest, when_implicit_vel_exceeds_max_vel_expect_vel_enforced)
{
SetupNode("joint_saturation_limiter_nodeclimit");
Load();

if (joint_limiter_)
{
Init();
Configure();

rclcpp::Duration period(0, 500000000); // 0.5 seconds
// Joint limits: max_vel = 2.0, max_acc = 5.0, no decel limits
// implicit_vel = max_acc * dt = 5.0 * 0.5 = 2.5 > max_vel (2.0)
// effective_max_acc = max_vel / dt = 2.0 / 0.5 = 4.0

current_joint_states_.positions[0] = 0.0;
current_joint_states_.velocities[0] = -0.5;
desired_joint_states_.positions.clear();
desired_joint_states_.velocities[0] = 3.0; // exceeds max_vel -> clamped to 2.0
// After vel clamping: desired_acc = (2.0 - (-0.5)) / 0.5 = 5.0 > effective_max_acc (4.0)
ASSERT_TRUE(joint_limiter_->enforce(current_joint_states_, desired_joint_states_, period));

// Acc limited to effective_max_acc = 4.0 (not raw max_acc = 5.0)
// Vel recomputed: -0.5 + 4.0 * 0.5 = 1.5
ASSERT_NEAR(desired_joint_states_.velocities[0], 1.5, COMMON_THRESHOLD);
ASSERT_NEAR(desired_joint_states_.accelerations[0], 4.0, COMMON_THRESHOLD);
}
}

int main(int argc, char ** argv)
{
::testing::InitGoogleMock(&argc, argv);
Expand Down
Loading