Skip to content

Adopt Google Chat ROS to smach_to_mail and adding launch file for it#1578

Merged
k-okada merged 10 commits intojsk-ros-pkg:masterfrom
mqcmd196:smach_to_state
Oct 12, 2022
Merged

Adopt Google Chat ROS to smach_to_mail and adding launch file for it#1578
k-okada merged 10 commits intojsk-ros-pkg:masterfrom
mqcmd196:smach_to_state

Conversation

@mqcmd196
Copy link
Member

Same with k-okada#72 .
Using Google Chat in smach_to_mail.py and adding launch file.
Example output
chat_notification

@mqcmd196 mqcmd196 requested review from k-okada and tkmtnt7000 August 27, 2022 04:11
"({})".format(x['INFO']) if x['INFO'] else ''))
self._send_mail(self.smach_state_subject[caller_id], self.smach_state_list[caller_id])
self._send_twitter(self.smach_state_subject[caller_id], self.smach_state_list[caller_id])
if self.use_mail:
Copy link
Member

Choose a reason for hiding this comment

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

If you do not want to publish /email /twitter and /google_chat all time, how about write code to check if there are topic subscrribers, instead of asking users to manually set use_email manually.

if rospy.get_num_connections() > 0:
   rospy.publish("email")

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the node for handling email, twitter, chatting should be launched all time, and if we want to suppress the notification, the demo code or this smach notification system should do it.

Copy link
Member

@tkmtnt7000 tkmtnt7000 Oct 6, 2022

Choose a reason for hiding this comment

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

I think the node for handling email, twitter, chatting should be launched all time, and if we want to suppress the notification, the demo code or this smach notification system should do it.

I am for this idea. We set default notification interfaces when robot starts, but robots should switch to better interfaces dependent on demo or behavior.
For example, we can use info data for switching notification interface or embed the message which interface should be used in user_data, though I still don't know what is the better way so much..

Copy link
Member

Choose a reason for hiding this comment

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

So, I think this implementation is almost OK and the function of switching to better interfaces may be our near future work.

Copy link
Member

Choose a reason for hiding this comment

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

how about using dynamic reconfigure to change parameters dynamically?
rosparam only loaded at the beginning, so other node cannot change it.

or user_data can contain the info, too.
also I think we dont need to suppress the notification from the beginning.
When someone says it is annoying, we can think about it probably 🤣

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm looking into ways to set the importance of smach information

Copy link
Member Author

Choose a reason for hiding this comment

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

how about using dynamic reconfigure to change parameters dynamically?
rosparam only loaded at the beginning, so other node cannot change it.

or user_data can contain the info, too.
also I think we dont need to suppress the notification from the beginning.
When someone says it is annoying, we can think about it probably rofl

Sorry I overlooked this comment... and I've just created suppressing node by setting notification level dyn param. Please check #1648

@knorth55
Copy link
Member

knorth55 commented Oct 3, 2022

please resolve the conflicts

@k-okada k-okada merged commit 9dc0bf1 into jsk-ros-pkg:master Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants