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 case function misuse in simple-action-server :set-preempted #673

Merged
merged 3 commits into from
Jan 25, 2023

Conversation

Affonso-Gui
Copy link
Member

While working on #672 I found out that when interrupted the euslisp server return the status 6 (preempting), while the same actionlib_tutorials node returns 2 (preempted).

This turned out to be happening from a misuse of the case function with multiple match cases in a single sentence, which are evaluated as quoted strings (symbols not values) and therefore are virtually unreachable. As a proof there was also a typo in the actionlib_msgs::GoalStatus::*preepmpting* variable.

Copy link
Member

@k-okada k-okada left a comment

Choose a reason for hiding this comment

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

can we add test code for this?

@Affonso-Gui
Copy link
Member Author

@k-okada Does the added tests serve you well?

@k-okada
Copy link
Member

k-okada commented Apr 13, 2022

test is always welcome

@Affonso-Gui
Copy link
Member Author

@k-okada Test was added here ec7e18d .
Can we merge this?

@Affonso-Gui
Copy link
Member Author

@k-okada Kindly ping.

@Affonso-Gui Affonso-Gui force-pushed the fix_actionilb_preempt_status branch from 157dd28 to f575cc8 Compare October 26, 2022 03:30
Copy link
Member

@k-okada k-okada left a comment

Choose a reason for hiding this comment

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

  1. we need to pass CI, c.f .github/workflows/config.yml: use checkout v3.0.2 and skip X for 14.04 #726
  2. This turned out to be happening from a misuse of the case function with multiple match cases in a single sentence, which are evaluated as quoted strings

I see...., do we have to change other lines, for example

(case state
(actionlib_msgs::GoalStatus::*recalling*
(setq state actionlib_msgs::GoalStatus::*pending*))
(actionlib_msgs::GoalStatus::*preempting*
(setq state actionlib_msgs::GoalStatus::*active*)))

@Affonso-Gui Affonso-Gui force-pushed the fix_actionilb_preempt_status branch from f575cc8 to 6f69f28 Compare October 28, 2022 06:21
@Affonso-Gui
Copy link
Member Author

@k-okada I have added similar fixes and the tests are now passing after a few resets (pr2 tests timing out).

@Affonso-Gui Affonso-Gui force-pushed the fix_actionilb_preempt_status branch from 6f69f28 to 930ca9c Compare November 3, 2022 12:34
@k-okada k-okada merged commit bc103da into jsk-ros-pkg:master Jan 25, 2023
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.

None yet

2 participants