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.

Sign up for GitHub

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

Jump to bottom

<feat> implemented Liner Quadratic Regulator #500

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

<feat> implemented Liner Quadratic Regulator #500

wants to merge 10 commits into from

Conversation

Q3rkses
Copy link

@Q3rkses Q3rkses commented Nov 10, 2024

Fixed everything, AUV works flawlessly now.

@Q3rkses Q3rkses added the Auto label Nov 10, 2024
@Q3rkses Q3rkses self-assigned this Nov 10, 2024
Andeshog
Andeshog requested changes Nov 10, 2024
View reviewed changes
control/velocity_controller_lqr/CMakeLists.txt Outdated Show resolved Hide resolved
control/velocity_controller_lqr/CMakeLists.txt Outdated Show resolved Hide resolved
control/velocity_controller_lqr/CMakeLists.txt Outdated Show resolved Hide resolved
control/velocity_controller_lqr/config/param_velocity_controller_lqr.yaml Outdated Show resolved Hide resolved
control/velocity_controller_lqr/launch/velocity_controller_lqr.launch.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Show resolved Hide resolved
@chrstrom chrstrom removed their request for review November 10, 2024 17:53
Talhanc
Talhanc requested changes Nov 10, 2024
View reviewed changes
Copy link

@Talhanc Talhanc left a comment

Choose a reason for hiding this comment

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

Everything else was nice : )

control/velocity_controller_lqr/CMakeLists.txt Outdated

ament_python_install_package(${PROJECT_NAME})

#install python scripts
Copy link

Choose a reason for hiding this comment

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

Why do you comment here?

Copy link
Author

Choose a reason for hiding this comment

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

what are you doing here?

control/velocity_controller_lqr/scripts/velocity_controller_lqr_node.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/tests/test_velocity_controller_lqr.py
controller is not None
) # Simple test to ensure the controller initializes

def test_ssa(self):
Copy link

Choose a reason for hiding this comment

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

I think you should have doc-string to explain the tests

Copy link
Author

Choose a reason for hiding this comment

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

Then we agree to disagree.

Copy link

Choose a reason for hiding this comment

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

Lets go

control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated
Comment on lines 2 to 7
import math as math

import numpy as np

import control as ct

Copy link

Choose a reason for hiding this comment

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

Why do you have so much space here?

Copy link
Author

Choose a reason for hiding this comment

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

good point

control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Show resolved Hide resolved
@kluge7 kluge7 force-pushed the 435-task-velocity-controller-pog branch from 8ee813a to 9a171f1 Compare November 10, 2024 18:07
@Andeshog Andeshog removed their assignment Nov 10, 2024
@kluge7 kluge7 linked an issue Nov 10, 2024 that may be closed by this pull request
[TASK] Velocity Controller #435
Open
3 tasks
kluge7
kluge7 requested changes Nov 15, 2024
View reviewed changes
control/velocity_controller_lqr/package.xml Outdated
Comment on lines 18 to 19
<test_depend>ament_lint_auto</test_depend>
<test_depend>ament_lint_common</test_depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed. Linting is tested through pre-commit hooks

control/velocity_controller_lqr/tests/test_velocity_controller_lqr.py Outdated
Comment on lines 68 to 70
def test_final(self):
print("¯\_(ツ)_/¯ ehh good enough pass anyway")
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this

Copy link
Author

Choose a reason for hiding this comment

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

never

requirements.sh Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This bash script will be used resolve dependencies not supported by rosdep.

Andeshog
Andeshog requested changes Nov 15, 2024
View reviewed changes
control/velocity_controller_lqr/launch/velocity_controller_lqr.launch.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/package.xml Outdated Show resolved Hide resolved
control/velocity_controller_lqr/scripts/velocity_controller_lqr_node.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/scripts/velocity_controller_lqr_node.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/scripts/velocity_controller_lqr_node.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/scripts/velocity_controller_lqr_node.py
def main(args=None): # main function
rclpy.init(args=args)
node = LinearQuadraticRegulator()
rclpy.spin(node)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using [MultiThreadedExecutor]https://docs.ros.org/en/humble/Concepts/Intermediate/About-Executors.htmll)

Copy link
Author

Choose a reason for hiding this comment

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

why htmll is it like new html

control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
control/velocity_controller_lqr/velocity_controller_lqr/velocity_controller_lqr_lib.py Outdated Show resolved Hide resolved
@kluge7 kluge7 self-requested a review November 17, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Talhanc Talhanc Talhanc requested changes

@Andeshog Andeshog Andeshog requested changes

@jorgenfj jorgenfj Awaiting requested review from jorgenfj

@AbubakarAliyuBadawi AbubakarAliyuBadawi Awaiting requested review from AbubakarAliyuBadawi

@kluge7 kluge7 Awaiting requested review from kluge7

Requested changes must be addressed to merge this pull request.

Assignees

@Q3rkses Q3rkses

Labels
Auto
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TASK] Velocity Controller
4 participants
@Q3rkses @Talhanc @kluge7 @Andeshog