-
Notifications
You must be signed in to change notification settings - Fork 16
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
Improving visual QA + System-initiated notifications #432
Conversation
ovenmitt
commented
May 30, 2024
…ion and preparing prompt for visual input
… and adding ROS msg for next/previous step
@ovenmitt For any binary files included here, do you think we could add those as git-LFS tracked items? I know there are some complexities around quotas and bandwidth limitations, so I'm not sure if you are aware of those for public github projects. It is ultimately the "right thing to do" with git to keep binaries files out of the git history. If we go this route, we will have to filter through commits to replace file contributions with GitLFS usage, or we could possible squash all of these changes into a single commit for simplification. |
I will remove the files, since they are not necessary for the ARUI to work at this point. |
Hi @ovenmitt! I have a rebase of this branch onto latest master as well as a fix for the code style of the files causing that CI failure. Do you mind if I force-push that to here, or are you still working on anything regarding this topic branch? |
{ | ||
"type": "image_url", | ||
"image_url": { | ||
"url": "data:image/jpeg;base64,"+self.image_msg |
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.
Oh this is soooo exciting!
@@ -29,7 +30,8 @@ | |||
PARAM_CONFIG_FILE = "config_file" | |||
PARAM_ACTIVITY_CONFIG_FILE = "activity_config_file" | |||
PARAM_TASK_STATE_TOPIC = "task_state_topic" | |||
PARAM_TASK_ERROR_TOPIC = "task_error_topic" | |||
PARAM_TASK_ERROR_TOPIC = "system_notification_topic" |
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.
This change in parameter name is going to break all of our other configurations. I would prefer that this parameter name not be changed, but you just assign the value to it as is appropriate for your local usage in your configuration file.
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.
Just updated it
I have a fix for what is causing the code-style CI to fail, but I can't seem to push it here, despite the |
Can you run |