search

PickNikRobotics / abb_ros2

Apache License 2.0
82 stars 34 forks source link

Fixed the wrong type for time parameterization in ompl #55

Closed jodle001 closed 8 months ago

jodle001 commented 10 months ago

54 related to this issue I created. Here is the change.

jodle001 commented 10 months ago

Okay I have pushed the changes you requested @Yadunund. Please review these changes to abb_moveit.launch.py to see if this is what you were asking for. Thank you for the feedback.

jodle001 commented 10 months ago

I have also noticed that a lot of the same files are referenced in the abb_control.launch.py file. What I have done before is had a separate file called moveit_utils.py or something like that, which has an object that builds the MoveItConfigsBuilder, then both files (abb_control and abb_moveit) can just call that utils file object. This would reduce redundant code. But this was not requested so I didn't do that for this commit.

jodle001 commented 8 months ago

I have made the changes requested, let me know if there is anything else that you would like to have changed.

jodle001 commented 8 months ago

Added one more change to publish robot description and robot semantic.

Yadunund commented 8 months ago

Thanks for addressing the additional feedback so quickly. Do you mind fixing the formatting of the code so that Format CI job passes? I think the Build and Test CI failing is due to bug upstream that was patched but a Rolling Sync has not happened in a while so the binaries are still outdated -> we can ignore this CI failure for now.

jodle001 commented 8 months ago

Okay I believe the formatting changes have been made to pass your test.

Yadunund commented 8 months ago

Format CI is still failing. Maybe you could try setting up the pre-commit check? Once installed, it should format the code before your commit.

jodle001 commented 8 months ago

Format CI is still failing. Maybe you could try setting up the pre-commit check? Once installed, it should format the code before your commit.

Sorry about that, I set that up and used the local command to fix the formatting. Thank you for your help.