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

Fixed mode_request_cb() when missing robot_name/fleet_name and changed print() into node.get_logger(). #254

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

DoppiaEffe94
Copy link

Bug fix

Fixed bug

#253

Fix applied

In the RobotCommandHandle.py, mode_request_cb() is the callback of the subscriber to the topic /action_execution_notice.

This cb function is supposed to check if the fleet_name or robot_name have a value.
However, in python 2 and python3, this verification is done with the not ... operation.

This PR fixes the if() condition in mode_request_cb(), as accordingly to the python2 and python3, considering the fields as Boolean variables.

@xiyuoh
Copy link
Member

xiyuoh commented Aug 8, 2024

Hey @DoppiaEffe94 , thank you for the PR!

As you can see the workflow for DCO has failed, this is because for RMF repositories we need the commits to be signed. This applies to your current and future commits. Could you follow the instructions here and here to create a signing key and sync it to your Github account, then rebase your branch with the signoff? You may want to click on Details next to the failed DCO run for more info.

francesco.fallica and others added 4 commits August 9, 2024 21:35
Signed-off-by: francesco.fallica <[email protected]>
Signed-off-by: DoppiaEffe94 <[email protected]>
Signed-off-by: francesco.fallica <[email protected]>
Signed-off-by: DoppiaEffe94 <[email protected]>
Signed-off-by: DoppiaEffe94 <[email protected]>
@DoppiaEffe94
Copy link
Author

Hello @xiyuoh !

I apologize for the DCO problem, I didn't know it was missing.

I added the line msg.robot_name != self.name, as suggested, and restored the self.node.get_logger().info() into a normal print().

@xiyuoh xiyuoh merged commit 0ce8f93 into open-rmf:humble Aug 12, 2024
2 of 3 checks passed
@DoppiaEffe94 DoppiaEffe94 deleted the humble branch August 12, 2024 15:12
@DoppiaEffe94 DoppiaEffe94 restored the humble branch August 12, 2024 15:13
@Yadunund
Copy link
Member

Just noticed that this was merged into humble branch and not main.

Our general policy is to merged fixes first into main and backport them to other distro branches.

I don't think we need to revert this PR, but could you open another PR to target main?

@xiyuoh
Copy link
Member

xiyuoh commented Aug 12, 2024

@Yadunund this PR doesn't affect main as the fix is in RobotCommandHandle.py while we're using EFC adapter on main. The fix is also not applicable to main as there is already a proper check to make sure that the message is ignored if robot name is empty or None that was missing in the humble adapter.

@Yadunund
Copy link
Member

Ah that's great to hear!

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.

[Bug]: RobotCommandHandle crashes when mode_request_cb() is called with missing robot_name/fleet_name
3 participants