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

[jsk_fetch_startup] Use email address instead of yaml for smach_to_mail #1634

Merged

Conversation

tkmtnt7000
Copy link
Member

This change depends on #1588 .
We use rosparam instead of yaml file to set email information.

@tkmtnt7000 tkmtnt7000 added hacktoberfest-accepted https://hacktoberfest.digitalocean.com/hacktoberfest-update develop/fetch Fetch labels Oct 2, 2022
Copy link
Member

@knorth55 knorth55 left a comment

Choose a reason for hiding this comment

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

this is not good because, it only supports fetch1075.
how about generating email_topic yaml in CMakeLists.txt?
like this:

macro(configure_dialogflow_hotword_yaml dname)
set(FETCH_NAME ${dname})
if (${FETCH_NAME} STREQUAL "fetch15")
set(ROBOT_NICKNAME_KANA "ヘンゼル")
set(ROBOT_NICKNAME_HIRA "へんぜる")
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config/dialogflow_hotword.yaml.in
${CMAKE_CURRENT_SOURCE_DIR}/config/${FETCH_NAME}_dialogflow_hotword.yaml)
elseif (${FETCH_NAME} STREQUAL "fetch1075")
set(ROBOT_NICKNAME_KANA "グレーテル")
set(ROBOT_NICKNAME_HIRA "ぐれーてる")
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config/dialogflow_hotword.yaml.in
${CMAKE_CURRENT_SOURCE_DIR}/config/${FETCH_NAME}_dialogflow_hotword.yaml)
endif()
endmacro(configure_dialogflow_hotword_yaml)

configure_dialogflow_hotword_yaml(fetch15)
configure_dialogflow_hotword_yaml(fetch1075)

@tkmtnt7000
Copy link
Member Author

tkmtnt7000 commented Oct 3, 2022

Thank you very much for reviewing. I mistakenly set email address without thinking about fetch1075 and fetch15.
I think it may be good to generate a email config file from CMakeLists.txt.

@knorth55 knorth55 force-pushed the develop/fetch branch 2 times, most recently from 91adad1 to b6bc4ca Compare October 3, 2022 15:34
@tkmtnt7000 tkmtnt7000 force-pushed the PR-not-use-yaml-fetch branch from a712450 to 7541c82 Compare October 4, 2022 07:23
@tkmtnt7000
Copy link
Member Author

Sorry for force-pushing...
Config yaml file is generated when we build workspace.

@tkmtnt7000 tkmtnt7000 merged commit 7982978 into jsk-ros-pkg:develop/fetch Oct 5, 2022
@tkmtnt7000 tkmtnt7000 deleted the PR-not-use-yaml-fetch branch October 5, 2022 02:40
@k-okada
Copy link
Member

k-okada commented Oct 5, 2022

@tkmtnt7000 @knorth55
Sorry, I did not look the changes of this PR carefully, I originaly intend to use rosparam directly within launch file, instead of using yaml file,

see
k-okada@37b71ce, may be this is too complex....

@knorth55
Copy link
Member

knorth55 commented Oct 5, 2022

receiver name should be different from the hostname, but we can write the address directly.
another point is that we cannot share the address information if we directly write it on launch file.
but i dont know if it is necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
develop/fetch Fetch hacktoberfest-accepted https://hacktoberfest.digitalocean.com/hacktoberfest-update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants