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

Add agent side ROS #82

Open
wants to merge 5 commits into
base: rosllm
Choose a base branch
from
Open

Add agent side ROS #82

wants to merge 5 commits into from

Conversation

W9YBZ
Copy link
Collaborator

@W9YBZ W9YBZ commented Jul 8, 2024

No description provided.

@W9YBZ W9YBZ requested a review from cmower July 8, 2024 13:15
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Yuhui seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Collaborator

@cmower cmower left a comment

Choose a reason for hiding this comment

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

@W9YBZ this code is not clean and I am not sure it will work out of the box yet, need to think about the flow of information through the new design for the framework, please familiarize with new packages/nodes in repo and re-implement.

Please see the comments, but I think a re-write of most scripts is required so probably better to see them as useful pointers for the re-design.

Let me know if you have questions and we can discuss.


To run the agent with Flask interface with ROS

In conda environment using the following command:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Specify where the user should change directory

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to follow the tutorials in last step

version: v0.1

max_env_steps: 1
max_episodes: 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

add newline at end of file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

import requests


class RosApi:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't want this to be a ROS specific interface, ideally this should be able to communicate with other environments - maybe HttpApi?

Make sure to update filename too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Documentation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added: To change communication port, in HEBO/Agent/src/agent/tasks/http_api.py replace http://localhost:5000 with with your actual port.


class RosApi:

# Defined on ros side
Copy link
Collaborator

Choose a reason for hiding this comment

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

inconsistent indentation - use black to format code

self._response = None

@staticmethod
def bad_response(obs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Documentation?


def request_human_feedback(self):
"""Prompt for human feedback from the terminal."""
self.latest_human_feedback = input(" Human, please enter input: ")
Copy link
Collaborator

Choose a reason for hiding this comment

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

why input here too? human feedback will be using a chat window in another ros node

"""Prompt for human feedback from the terminal."""
self.latest_human_feedback = input(" Human, please enter input: ")
if self.latest_human_feedback.lower() == "exit":
print("Killing program...")
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove print statement and use rospy.logwarn

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed

self.latest_human_feedback = input(" Human, please enter input: ")
if self.latest_human_feedback.lower() == "exit":
print("Killing program...")
os.kill(os.getpid(), signal.SIGKILL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

sys.exit is the cleaner way to kill ROS nodes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

otherwise it take 5000

Comment on lines +86 to +89
# To read the humanfeedback from a topic
# def feedback_callback(self, msg):
# """Callback function to update the latest human feedback."""
# self.latest_human_feedback = msg.data
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

Comment on lines +100 to +102
while not rospy.is_shutdown():
self.request_human_feedback()
rospy.spin()
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not c++, rospy.spin is not equivalent to ros::spinOnce``, rospy.spin` will catch this and loop internally forever until the node is killed

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.

3 participants