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

RSDK-7531 - Add GetProperties to vision service #493

Merged
merged 4 commits into from
May 9, 2024

Conversation

kharijarrett
Copy link
Member

The file worth viewing here (to see the changes) is the vision.proto . Adding this as scoped here (https://docs.google.com/document/d/1P0eFy1Y5zBT91oFxz32isku3l5HJUHfdzSKYuS-MFMI/edit?usp=sharing)

Everything else is autogenerated.

@github-actions github-actions bot added the safe to test committer is a member of this org label May 8, 2024
@kharijarrett kharijarrett requested review from njooma and bhaney May 8, 2024 18:41
Comment on lines 180 to 182
bool classifications_supported = 1;
bool detections_supported = 2;
bool object_point_clouds_supported = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have docstrings here, even though the docstrings would be super self explanatory. It's just very useful with auto-generated documentation in the SDKs

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotchu

@bhaney
Copy link
Member

bhaney commented May 8, 2024

I see that in some places, the protoc version gets downgraded to a previous version. Can you make sure your local setup is up to date ('make setup' on your api repo) and that you are basing your commit on top of the latest commit of the api repo, too?

@kharijarrett
Copy link
Member Author

Okay great catch on the versioning thing. It's fixed now, I had to nuke my old /opt folder before the make setup.

Copy link
Member

@bhaney bhaney left a comment

Choose a reason for hiding this comment

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

Thanks for doing the checks! LGTM

@kharijarrett kharijarrett added the ready-for-protos add this when you want protos to compile on every commit label May 9, 2024
@kharijarrett kharijarrett removed the ready-for-protos add this when you want protos to compile on every commit label May 9, 2024
@kharijarrett kharijarrett merged commit 023f273 into viamrobotics:main May 9, 2024
3 checks passed
@kharijarrett kharijarrett deleted the RSDK-7531 branch May 9, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protos-compiled safe to test committer is a member of this org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants