Skip to content

Ackermann and control node adjusted for param passing#7

Closed
LoganOT11 wants to merge 1 commit intomainfrom
exec_params/logan
Closed

Ackermann and control node adjusted for param passing#7
LoganOT11 wants to merge 1 commit intomainfrom
exec_params/logan

Conversation

@LoganOT11
Copy link
Contributor

@LoganOT11 LoganOT11 commented Dec 4, 2025

Merge Request / Pull Request

Summary of Changes

Allows control to take in ros2 params for config path to load values used for ackermann controller. Uses delayed initalization, so defaults are loaded first and then overwritten based on config file. Otherwise, if file can't be laoded defaults are continued to be used

Type of Change

  • [ X ] Bug fix
  • [ X ] New feature / task
  • [ X ] Refactor
  • Documentation

Checklist (to be completed before review)

  • [ X ] Code follows team standards (maybe)
  • Tested locally
  • [ X ] Tests added / updated if relevant (safety checks added no tests)
  • Documentation updated if relevant
  • CI / checks pass

Related Issue / Task

  • Closes #[issue_number] (if applicable)

Notes / Additional Context

Any context for reviewers (limitations, known issues, future work)

Copy link
Member

@alyashour alyashour left a comment

Choose a reason for hiding this comment

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

It's likely this isn't needed anymore, closing for now.

Comment on lines +49 to +54
// Safety check if controller is initialized
if (!ackermann_controller_) {
RCLCPP_WARN_THROTTLE(this->get_logger(), *this->get_clock(), 5000, "Controller not initialized!");
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

Not best to do this in every loop, should be checked once during init.

@alyashour alyashour closed this Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants