-
Notifications
You must be signed in to change notification settings - Fork 0
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/constants #39
Conversation
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.
fk it we ball
hear_config = os.path.join( | ||
get_package_share_directory("speech"), "config", "hear.yaml" | ||
) | ||
speaker_config = os.path.join( | ||
get_package_share_directory("speech"), "config", "speaker.yaml" |
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.
im not getting why are we changing this??
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.
yaml config has keys with values "REPLACE". parse_ros_config
updates such values, using the constants defined in ModuleNames.HRI.value
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.
lmao, seems like a lot isnt it?
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.
I would prefer to have all in one place, why should we keep the input name in the config?
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.
but seems ok either way, this also seems pretty elegant, I dont dislike it
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.
feel free to close this, just wanted to know why
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.
I would prefer to have all in one place, why should we keep the input name in the config?
Hmm, so that we know which values to insert / which values are being replaced?
Use cases:
Notes:
frida_interfaces
was not possible: apparently,rosidl_generate_interfaces
andament_python_install_package
don't work in the same CMake project. There are several workarounds but will leave as it is for the moment.