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

Fix operator== constness in ClientGoalHandle and ManagedList::Handle #10

Closed
wants to merge 2 commits into from

Conversation

po1
Copy link

@po1 po1 commented Aug 8, 2013

Now we can compare GoalHandles, which is handy for data structures.

@dirk-thomas
Copy link
Member

The pull request should only contain your commit. Please rebase your pull request branch.

@po1
Copy link
Author

po1 commented Aug 8, 2013

Yeah I don't know why it included this other one, I only noticed it afterwards

@@ -251,7 +251,7 @@
}

template<class ActionSpec>
bool ClientGoalHandle<ActionSpec>::operator==(const ClientGoalHandle<ActionSpec>& rhs)
bool ClientGoalHandle<ActionSpec>::operator==(const ClientGoalHandle<ActionSpec>& rhs) const
Copy link
Member

Choose a reason for hiding this comment

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

This method uses a ScopedProtector internally which is not possible if it would be const I guess?

Copy link
Author

Choose a reason for hiding this comment

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

My compiler (gcc 4.6.3) didn't complain, and my code makes use of the comparison.
The only problem in the method is the dereferencing of the guard_ shared_ptr, which does not affect the shared_tr itself. (shared_ptr::operator* is defined const)

@dirk-thomas
Copy link
Member

The same would apply to the server classes server_goal_handle.h and server_goal_handle_imp.h.

@dirk-thomas
Copy link
Member

I have created an updated pull request #12. Can you please give it a try if it suites your use case?

dirk-thomas added a commit that referenced this pull request Aug 21, 2013
@dirk-thomas
Copy link
Member

Closing in favor of #12.

@po1 po1 deleted the comparison_patch branch November 29, 2013 18:41
severin-lemaignan referenced this pull request in severin-lemaignan/robotpkg Aug 18, 2014
Changes since 1.9.12:

1.11.2 (2014-05-20)
-------------------
* Update python publishers to define queue_size.
* Use the correct queue for processing MessageEvents
* Contributors: Esteve Fernandez, Michael Ferguson, Nican

1.11.1 (2014-05-08)
-------------------
* Fix uninitialised execute_thread_ member pointer
* Make rostest in CMakeLists optional
* Use catkin_install_python() to install Python scripts
* Contributors: Dirk Thomas, Esteve Fernandez, Jordi Pages, Lukas Bulwahn

1.11.0 (2014-02-13)
-------------------
* replace usage of __connection_header with MessageEvent (`#20
<https://github.com/ros/actionlib/issues/20>`_)

1.10.3 (2013-08-27)
-------------------
* Merged pull request `#15 <https://github.com/ros/actionlib/issues/15>`_
  Fixes a compile issue for actionlib headers on OS X

1.10.2 (2013-08-21)
-------------------
* separating ActionServer implementation into base class and
ros-publisher-based class (`#11 <https://github.com/ros/actionlib/issues/11>`_)
* support CATKIN_ENABLE_TESTING
* add isValid to ServerGoalHandle (`#14
<https://github.com/ros/actionlib/issues/14>`_)
* make operators const (`#10 <https://github.com/ros/actionlib/issues/10>`_)
* add counting of connections to avoid reconnect problem when callbacks are
invoked in different order (`#7 <https://github.com/ros/actionlib/issues/7>`_)
* fix deadlock in simple_action_server.py (`#4
<https://github.com/ros/actionlib/issues/4>`_)
* fix missing runtime destination for library (`#3
<https://github.com/ros/actionlib/issues/3>`_)

1.10.1 (2013-06-06)
-------------------
* fix location of library before installation (`#1
<https://github.com/ros/actionlib/issues/1>`_)

1.10.0 (2013-04-11)
-------------------
* define DEPRECATED only if not defined already
* modified dependency type of catkin to buildtool
@lubiluk lubiluk mentioned this pull request Jul 20, 2017
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