-
Notifications
You must be signed in to change notification settings - Fork 42
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
refactor #116 remove callback_type #154
Conversation
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
return ret; | ||
} | ||
} | ||
rclc_executor_handle_t * handle = _rclc_executor_find_handle(executor, subscription); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You considered it not necessary to check for the type
? Of course, you could argue that if the pointer matches then it must be the same object (with the same type). LGTM.
return false; | ||
} | ||
if (obj == handle_obj_ptr) { | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mergify backport galactic rolling |
* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24)
Command
Hey, I reacted but my real name is @Mergifyio |
@mergify backport master |
* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24)
Command
Hey, I reacted but my real name is @Mergifyio |
* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]>
* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]>
* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]>
* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]>
* ci workflow for galactic (#90) Signed-off-by: Jan Staschulat <[email protected]> * galactic: updated ci-job and codecov (#91) * bump version to 2.0.1 of galactic branch for new bloom release (#95) * Dummy (#100) * test of galactic bloom release see also #100 (#101) * Adds a context pointer to subscriptions (backport #107) (#120) * Adds a context pointer to subscriptions (#107) (cherry picked from commit 5b11c5a) # Conflicts: # rclc_examples/CMakeLists.txt * resolved merge confict Signed-off-by: Jan Staschulat <[email protected]> Co-authored-by: Brett Downing <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> * version bump and updated changelog. (#139) Signed-off-by: Jan Staschulat <[email protected]> * prepare bloom release v2.0.3 for galactic (#159) Signed-off-by: Jan Staschulat <[email protected]> * refactor #116 remove callback_type (#154) (#163) * refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]> * updated rcl dependency to galactic (#184) Signed-off-by: Jan Staschulat <[email protected]> * added pingpong example (#172) (#188) * added pingpong example Signed-off-by: Jan Staschulat <[email protected]> * added two executor configuration to pingpong Signed-off-by: Jan Staschulat <[email protected]> * example_pingpong multiple files Signed-off-by: Jan Staschulat <[email protected]> * added class member function to executor Signed-off-by: Jan Staschulat <[email protected]> (cherry picked from commit e0252bd) Co-authored-by: Jan Staschulat <[email protected]> * prepare bloom release 2.0.4-galactic (#193) Signed-off-by: Jan Staschulat <[email protected]> * Copy client to action client Initial mods to action client Initial executor support for action client Updates Fix Updates Copy client action to server action Initial action server changes Fix Revert "Copy client to action client" This reverts commit 304c70a. Initial Update Working state Uncrustify fix Update Update Add client Uncrustify Uncrus Apply suggestions from code review Co-authored-by: Ralph Lange <[email protected]> Initial test Fix data available logic Update tests Update Uncrust Revert "Update" This reverts commit 940aa96. Working state Working Building state Update Working state Working state Working state Uncrust Updates Fix cancelling reject Test approach modified Complete server tests Uncrusti Add multigoal test Fix Initial action client tests Add executor client cancel handlers Update action client Add cancel tests Tests update Uncrustify Refactor cancel response handling Update * Revert "Copy client to action client" This reverts commit d65adc7. * Initial version * Hide non public API * Fix abort API * Minor fix * Update * Fix * Add rclcpp dep * Update actions implementation * Delete COLCON_IGNORE * Fix dependencies export * cpplint * Initial Ralph changes Co-authored-by: Ralph Lange <[email protected]> * Add RCLC error * Refactor pop/take inner functions * Argument renaming * Argument renaming * Rename get API to find * Reafctor functio naming * Refactor naming * Remove explicit == true in conditionals * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> * Remove white lines * Update rclc/src/rclc/action_server.c * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> * Add pointer check suggestions * Apply suggestions on executor * Add check for feedback msg on client * Put goal on default switch case * Revert code delete * Add comments to executor loops * Update loop comments * Apply for loop style where possible * Fix variable name * Fix rebase * ci workflow for galactic (#90) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * galactic: updated ci-job and codecov (#91) Signed-off-by: Antonio Cuadros <[email protected]> * bump version to 2.0.1 of galactic branch for new bloom release (#95) Signed-off-by: Antonio Cuadros <[email protected]> * Dummy (#100) Signed-off-by: Antonio Cuadros <[email protected]> * test of galactic bloom release see also #100 (#101) Signed-off-by: Antonio Cuadros <[email protected]> * Adds a context pointer to subscriptions (backport #107) (#120) * Adds a context pointer to subscriptions (#107) (cherry picked from commit 5b11c5a) # Conflicts: # rclc_examples/CMakeLists.txt * resolved merge confict Signed-off-by: Jan Staschulat <[email protected]> Co-authored-by: Brett Downing <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * version bump and updated changelog. (#139) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * prepare bloom release v2.0.3 for galactic (#159) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * refactor #116 remove callback_type (#154) (#163) * refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * updated rcl dependency to galactic (#184) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * added pingpong example (#172) (#188) * added pingpong example Signed-off-by: Jan Staschulat <[email protected]> * added two executor configuration to pingpong Signed-off-by: Jan Staschulat <[email protected]> * example_pingpong multiple files Signed-off-by: Jan Staschulat <[email protected]> * added class member function to executor Signed-off-by: Jan Staschulat <[email protected]> (cherry picked from commit e0252bd) Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * prepare bloom release 2.0.4-galactic (#193) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Copy client to action client Initial mods to action client Initial executor support for action client Updates Fix Updates Copy client action to server action Initial action server changes Fix Revert "Copy client to action client" This reverts commit 304c70a. Initial Update Working state Uncrustify fix Update Update Add client Uncrustify Uncrus Apply suggestions from code review Co-authored-by: Ralph Lange <[email protected]> Initial test Fix data available logic Update tests Update Uncrust Revert "Update" This reverts commit 940aa96. Working state Working Building state Update Working state Working state Working state Uncrust Updates Fix cancelling reject Test approach modified Complete server tests Uncrusti Add multigoal test Fix Initial action client tests Add executor client cancel handlers Update action client Add cancel tests Tests update Uncrustify Refactor cancel response handling Update Signed-off-by: Antonio Cuadros <[email protected]> * Revert "Copy client to action client" This reverts commit d65adc7. Signed-off-by: Antonio Cuadros <[email protected]> * Initial version Signed-off-by: Antonio Cuadros <[email protected]> * Hide non public API Signed-off-by: Antonio Cuadros <[email protected]> * Fix abort API Signed-off-by: Antonio Cuadros <[email protected]> * Minor fix Signed-off-by: Antonio Cuadros <[email protected]> * Update Signed-off-by: Antonio Cuadros <[email protected]> * Fix Signed-off-by: Antonio Cuadros <[email protected]> * Add rclcpp dep Signed-off-by: Antonio Cuadros <[email protected]> * Update actions implementation Signed-off-by: Antonio Cuadros <[email protected]> * Delete COLCON_IGNORE Signed-off-by: Antonio Cuadros <[email protected]> * Fix dependencies export Signed-off-by: Antonio Cuadros <[email protected]> * cpplint Signed-off-by: Antonio Cuadros <[email protected]> * Initial Ralph changes Co-authored-by: Ralph Lange <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Add RCLC error Signed-off-by: Antonio Cuadros <[email protected]> * Refactor pop/take inner functions Signed-off-by: Antonio Cuadros <[email protected]> * Argument renaming Signed-off-by: Antonio Cuadros <[email protected]> * Argument renaming Signed-off-by: Antonio Cuadros <[email protected]> * Rename get API to find Signed-off-by: Antonio Cuadros <[email protected]> * Reafctor functio naming Signed-off-by: Antonio Cuadros <[email protected]> * Refactor naming Signed-off-by: Antonio Cuadros <[email protected]> * Remove explicit == true in conditionals Signed-off-by: Antonio Cuadros <[email protected]> * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Remove white lines Signed-off-by: Antonio Cuadros <[email protected]> * Update rclc/src/rclc/action_server.c Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Add pointer check suggestions Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions on executor Signed-off-by: Antonio Cuadros <[email protected]> * Add check for feedback msg on client Signed-off-by: Antonio Cuadros <[email protected]> * Put goal on default switch case Signed-off-by: Antonio Cuadros <[email protected]> * Revert code delete Signed-off-by: Antonio Cuadros <[email protected]> * Add comments to executor loops Signed-off-by: Antonio Cuadros <[email protected]> * Update loop comments Signed-off-by: Antonio Cuadros <[email protected]> * Apply for loop style where possible Signed-off-by: Antonio Cuadros <[email protected]> * Fix variable name Signed-off-by: Antonio Cuadros <[email protected]> * Fix rebase Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> * removed unneccary reset of handle->data_available , in _rclc_check_for_new_data this flag is assigned again. Signed-off-by: Jan Staschulat (CR/ADA1.2) <[email protected]> * Change put goal function name Co-authored-by: Jan Staschulat <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Brett Downing <[email protected]> Co-authored-by: Antonio Cuadros <[email protected]> Co-authored-by: Ralph Lange <[email protected]> Co-authored-by: Antonio Cuadros <[email protected]>
This refactor eliminates
handle->callback_type
, and expandshandle->type
to all availablerclc_executor_add_*()
functions.This resolves both: #116 and #115
I've permitted
NULL
contexts for services, so that we can eventually drop the API that relies on global variables.rclc_executor_remove_*
can be drastically simplified if the check forhandle->type
is dropped, moving the search into a helper function and leveragingrclc_executor_handle_get_ptr
@jst3si left suggestions for a new service message and response struct, which look like a good idea for rolling.