refactor(franka_gazebo): remove FrankaHWSim robot namespace warning #366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rickstaa
force-pushed
the
remove_robot_namespace_warning
branch
from
8224ad9
to
1d62e7a
Compare
rickstaa
changed the title
rickstaa
changed the title
rickstaa
changed the title
This was referenced Sep 3, 2023
rickstaa
force-pushed
the
remove_robot_namespace_warning
branch
2 times, most recently
from
ef6b24f
to
0e7cc2e
Compare
5 tasks
This commit removes the robot namespace warning found in the FrankaHWSim since it is no longer needed now that frankaemika#196 has been merged. It also replaced the 'arm_id' default with the model name.
rickstaa
force-pushed
the
remove_robot_namespace_warning
branch
from
0e7cc2e
to
24321ee
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In commit f2f82b2, a robot namespace warning was introduced in the FrankaHWSim package. This warning was added to address a specific issue at the time (see #187 (comment)). However, with the recent merge of PR #196, the conditions that necessitated this warning are no longer applicable.
To enhance the user experience and ensure clarity in error messages, I propose the following changes:
Remove Redundant Robot Namespace Warning: The warning introduced in f2f82b2 is no longer needed since this warning only would be printed if the URDF is found. The URDF will only be found when the user specifies the correct robot namespace.
Change arm_id default value: In the original code, the
robot_namespacewas used as the default value for thearm_idattribute. This could trigger cryptic hardware transmission warnings if users forget to set thearm_idparameter on therobot_namespace. This can be solved by replacing the defaultrobot_namespacewith the model name as the default value for thearm_idattribute.Introduce a warning mechanism that notifies users when they fail to load the arm_id parameter within the model parameter namespace. This will help users identify and resolve namespace-related issues more effectively.
Please let me know if you have any questions about my pull request.