Skip to content

Conversation

@hexbabe
Copy link
Member

@hexbabe hexbabe commented Jun 16, 2025

RSDK-10991

Create Go helpers for implementing GetImage if you have GetImages and vice versa

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jun 16, 2025
@github-actions
Copy link
Contributor

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component function
base IsMoving
board GPIOPinByName
camera Properties
encoder Properties
motor IsMoving
sensor Readings
servo Position
arm EndPosition
audio MediaProperties
gantry Lengths
gripper IsMoving
input_controller Controls
movement_sensor LinearAcceleration
power_sensor Power
pose_tracker Poses
motion GetPose
vision GetProperties

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 16, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 16, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 16, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 16, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 16, 2025
@hexbabe hexbabe requested a review from seanavery June 16, 2025 19:31
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 17, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 17, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 17, 2025
@nicksanford
Copy link
Member

This approach will force us to do decodes and encodes. I agree we need this behavior as a fallback but I'm wondering:

If the method we call returns an image.Image that happens to be a LazyImage can we save ourselves some decodes and encodes?

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 20, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 20, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 20, 2025
@hexbabe
Copy link
Member Author

hexbabe commented Jun 20, 2025

This approach will force us to do decodes and encodes. I agree we need this behavior as a fallback but I'm wondering:

If the method we call returns an image.Image that happens to be a LazyImage can we save ourselves some decodes and encodes?

Let me think through this:

For GetImageFromGetImages, we call GetImages that returns us the image.Image, which could be a lazy image. If it is indeed a lazy image and the requested mime type matches the response mime type, we hit this path when we call EncodeImage and save ourselves an encode ✅

For GetImagesFromGetImage, we call GetImage which returns bytes, we have to decode to an image.Image so no lazy benefits here ❌

Copy link
Member

@seanavery seanavery left a comment

Choose a reason for hiding this comment

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

Should we also add test cases for mimetypes with lazy suffix?

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 27, 2025
@hexbabe hexbabe merged commit cba228d into viamrobotics:main Jun 27, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test This pull request is marked safe to test from a trusted zone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants