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

joint_limits_interface should not wrap JointHandle in shared_ptr #191

Closed
bmagyar opened this issue Oct 10, 2020 · 6 comments
Closed

joint_limits_interface should not wrap JointHandle in shared_ptr #191

bmagyar opened this issue Oct 10, 2020 · 6 comments
Labels
good first issue

Comments

@bmagyar
Copy link
Member

bmagyar commented Oct 10, 2020

@bmagyar @Karsten1987 @destogl In this refactor for #134 I went with JointHandle arguments wrapped in std::shared_ptr<>, should these have been without shared_ptr? This was due to a commit I was using as a reference that had shared_ptr, but after reviewing more code such as robot_hardware.cpp I dont see any reason to use shared_ptr here, the JontHandle by value would work fine and probably more compatible. Thoughts?

Originally posted by @guru-florida in #181 (comment)
@bmagyar
Copy link
Member Author

bmagyar commented Oct 10, 2020

@guru-florida please go ahead and make these changes on a new PR, I've merged the old one.
You may want to branch off from a fresh checkout of master, we squash-merge PRs.

@guru-florida
Copy link
Contributor

Will do. I can take care of #188 and #189 at the same time. Would you like them as individual PRs or just one?

@bmagyar
Copy link
Member Author

bmagyar commented Oct 10, 2020 •
edited
Loading

Thanks!
I've already submitted for #189 and #190 in #193 and #192.

Happy for you to do #188, just leave a note that you are doing it so nobody else starts it. It's fine to mash them up in the same PR 👍

@guru-florida guru-florida mentioned this issue Oct 10, 2020
Closed
guru-florida added a commit to guru-florida/ros2_control that referenced this issue Oct 11, 2020
@guru-florida
ros-controls#191 removed std::shared_ptr from Joint Limits handles
7f8c299
guru-florida added a commit to guru-florida/ros2_control that referenced this issue Oct 11, 2020
@guru-florida
ros-controls#191 cleaner default custruction of Joint Handles with DE…
42ac3d8 
…FAULT_POSITION|VELOCITY|COMMAND_HANDLE macros
guru-florida added a commit to guru-florida/ros2_control that referenced this issue Oct 11, 2020
@guru-florida
ros-controls#191 added operator-bool to Handle<> template class for t…
20253fe 
…esting if handle has a reference set. uncrustify fixes on joint limits macros
@guru-florida
Copy link
Contributor

guru-florida commented Oct 13, 2020 •
edited
Loading

@bmagyar PR #194 fixes issue #191 and #188. Please, review caveat in PR comment about Handle template class.

(edit: review requested mentioned wrong class)

bmagyar pushed a commit that referenced this issue Oct 19, 2020
@guru-florida
Feature/191 joint limits without sharedptr (#194)
f9d4e61 
* #188 refactored enforceLimits to enforce_limits

* #191 removed std::shared_ptr from Joint Limits handles

* #191 cleaner default custruction of Joint Handles with DEFAULT_POSITION|VELOCITY|COMMAND_HANDLE macros

* #191 added operator-bool to Handle<> template class for testing if handle has a reference set. uncrustify fixes on joint limits macros

* added Handle<> constructors initializing interface_name only, and setter for interface_name,value

* updated joint limit handles to use new Handle constructors

* uncrustify and cpplint fixes

* renamed base class JointSaturationLimitHandle to just JointLimitHandle since it is the common base class for Saturation and SoftLimit derived limit classes

* fixed some constructor calls that required explicit JointHandle cast
@destogl
Copy link
Member

destogl commented Oct 21, 2020

@guru-florida this seams to be solved in #194. Is it OK to close the issue?

@guru-florida
Copy link
Contributor

Yes, this and #188 can be closed.

guru-florida added a commit to guru-florida/ros2_control that referenced this issue Oct 23, 2020
@guru-florida
ros-controls#191 removed std::shared_ptr from Joint Limits handles
006cc50
guru-florida added a commit to guru-florida/ros2_control that referenced this issue Oct 23, 2020
@guru-florida
ros-controls#191 cleaner default custruction of Joint Handles with DE…
ddd222b 
…FAULT_POSITION|VELOCITY|COMMAND_HANDLE macros
guru-florida added a commit to guru-florida/ros2_control that referenced this issue Oct 23, 2020
@guru-florida
ros-controls#191 added operator-bool to Handle<> template class for t…
f76ae53 
…esting if handle has a reference set. uncrustify fixes on joint limits macros
@destogl destogl mentioned this issue Dec 20, 2020
Open
6 tasks
@bmagyar bmagyar closed this as completed May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@destogl @bmagyar @guru-florida