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

HITL - Change object state check function signature. #2069

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

0mdc
Copy link
Contributor

@0mdc 0mdc commented Sep 5, 2024

Motivation and Context

This changes the object state check function signature.
A result struct is used instead of a tuple.

How Has This Been Tested

Tested locally.

Types of changes

  • [Development]

Checklist

  • My code follows the code style of this project.
  • I have updated the documentation if required.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes if required.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 5, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we already have the UI knobs locally which show the available object-states and the ability to click on them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@zephirefaith zephirefaith left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks! Left a comment.

Comment on lines +163 to +165
if result.success:
self.set_object_state(object_handle, state_name, state_value)
return (success, error_message)
return ObjectStateChangeResult(result.success, result.error_message)
Copy link
Contributor

Choose a reason for hiding this comment

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

If result is not a success, is the error communicated anywhere to the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, in this case, it's just failing silently.

The UI calls can_execute_action every frame to check whether the user can execute the action. The message is passed here to be reactive to user's actions.

@0mdc 0mdc merged commit fae939e into main Sep 5, 2024
3 of 4 checks passed
@0mdc 0mdc deleted the 0mdc/hitl_llm_signature_update branch September 5, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants