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

v4.0 updates #26

Merged
merged 13 commits into from
Feb 16, 2024
Merged

v4.0 updates #26

merged 13 commits into from
Feb 16, 2024

Conversation

marioprats
Copy link
Contributor

Merge branch v4.0 into main.
Tested in mock and all seems to work, except for a tool offset I'm seeing in Servo, which I think is an issue in main:
https://github.com/PickNikRobotics/moveit_studio/issues/6498

Copy link
Contributor

@pac48 pac48 left a comment

Choose a reason for hiding this comment

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

Looks good. I tested on hardware. I added two comments for the issues I ran into.

@@ -54,6 +60,38 @@ joint_trajectory_controller:
joint_7:
goal: 0.05

streaming_controller:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be named servo_controller

Copy link
Contributor Author

Choose a reason for hiding this comment

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

servo_controller exists already, which I think it's how it should be called now, right?
So I think we can remove the streaming_controller, can't we? It's not even referenced in the configs in this package.
I removed it and things seem to still work in hardware.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I didn't see it because I was looking at the diff. I think streaming_controller should be deleted

src/Dockerfile Show resolved Hide resolved
Copy link
Contributor

@pac48 pac48 left a comment

Choose a reason for hiding this comment

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

Looks good, I would just remove the streaming_controller since it is not used anymore.

@marioprats marioprats merged commit 7f6d0ed into main Feb 16, 2024
1 check passed
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.

3 participants