RSDK-11613 — Update Go camera API with new GetImages signature #5183

hexbabe · 2025-07-29T19:13:04Z

Todo:

Write better unit tests instead of just getting stuff to compile
Remove POC stream server changes (deferred to another ticket)
Manual testing

Manual Tests

Data Manager GetImages

Re-ran this test #5209 (comment) exactly as outlined and made sure it still works off of this feature branch.

DataManager ReadImage

Changed config to use "method": "ReadImage" instead of "method": "GetImages", filtered_camera still works as expected.

Stream server

Live streams working as expected.

GetImages filter_source_names

Using a go client script to call Images on an image_file camera with below config

    {
      "name": "image-file-1",
      "api": "rdk:component:camera",
      "model": "rdk:builtin:image_file",
      "attributes": {
        "preloaded_image": "pizza",
        "color_image_file_path": "/Users/seanyu/Downloads/image.png"
      }
    }

Requesting invalid source

2025-08-19T18:53:05.415Z        ERROR   client  go-sdk-client/client.go:50      camera client: could not gets images from the camera rpc error: code = Unknown desc = camera server GetImages could not call Images on the camera: invalid source name: RAWWWWWWRRRRRR

Requesting with no sources specified

	for _, image := range namedImages {
		logger.Info(image.MimeType())
	}
	logger.Info(respMetadata)

Logged:

2025-08-19T18:54:38.159Z        INFO    client  go-sdk-client/client.go:53      image/jpeg
2025-08-19T18:54:38.159Z        INFO    client  go-sdk-client/client.go:53      image/jpeg
2025-08-19T18:54:38.159Z        INFO    client  go-sdk-client/client.go:55      {2025-08-19 18:54:38.149384 +0000 UTC}

Requesting source "preloaded" specified

2025-08-19T18:56:30.575Z        INFO    client  go-sdk-client/client.go:53      image/jpeg
2025-08-19T18:56:30.575Z        INFO    client  go-sdk-client/client.go:55      {2025-08-19 18:56:30.572539 +0000 UTC}

Number of sources checks out.

Requesting duplicate sources "color" and "color"

2025-08-19T18:57:30.835Z        ERROR   client  go-sdk-client/client.go:50      duplicate source name in filter: color
exit status 1

Nice.

github-actions · 2025-07-29T19:13:15Z

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
button	Push
genericcomponent	DoCommand
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
genericservice	DoCommand
motion	GetPose
vision	GetProperties

SebastianMunozP

Reviewed these changes synchronously, LGTM

bhaney

Nice work!

some non-blocking requests, mostly about adding more tests

bhaney · 2025-08-26T15:37:04Z

components/camera/camera.go

+
+	data, err := rimage.EncodeImage(ctx, ni.img, ni.mimeType)
+	if err != nil {
+		return nil, fmt.Errorf("could not encode image: %w", err)


could you make this more descriptive by including what encoding was attempted? e.g.
"could not encode image with encoding %s : %w", ni.mimeType, err)

great call I'll add

bhaney · 2025-08-26T15:44:50Z

components/camera/client_test.go

+			images, _, err := camClient.Images(ctx, []string{"color", "depth"}, nil)
+			test.That(t, err, test.ShouldBeNil)
+			test.That(t, len(images), test.ShouldEqual, 2)
+			test.That(t, images[0].SourceName, test.ShouldEqual, "color")
+			test.That(t, images[1].SourceName, test.ShouldEqual, "depth")
+


Question: if the filter was in the opposite order []string{"depth", "color"}, would it be expected that images[0] is the depth image, and images[1] is the color image? Is the order of request required in the order of response? Can you make a test?

How to handle order would be entirely on the module implementer, client and server should have no opinions.

I'll add a test to make that clear

bhaney · 2025-08-26T15:45:50Z

components/camera/client_test.go

+			test.That(t, rimage.ImagesExactlyEqual(depthImg, expectedDepth), test.ShouldBeTrue)
+		})
+
+		t.Run("empty and nil filters", func(t *testing.T) {


can you add a few more more tests, which test a filter that doesn't exist, and duplicates?

case 1: []string{"no_such_source"}
case 2: []string{"color", "no_such_source"}
case 3: []string{"color", "color"}

I have case 1 and 2 in camera_test.go

case 3 is not handled by the client, it is handled by the server

I'll add a test for case 3 to make it more explicit though

Added all three to be extra safe

bhaney · 2025-08-26T15:49:34Z

components/camera/server.go

-func encodeImageFromUnderlyingType(ctx context.Context, img image.Image) (pb.Format, []byte, error) {
-	switch v := img.(type) {
-	case *rimage.LazyEncodedImage:
-		format := pb.Format_FORMAT_UNSPECIFIED
-		switch v.MIMEType() {
-		case utils.MimeTypeRawDepth:
-			format = pb.Format_FORMAT_RAW_DEPTH
-		case utils.MimeTypeRawRGBA:
-			format = pb.Format_FORMAT_RAW_RGBA
-		case utils.MimeTypeJPEG:
-			format = pb.Format_FORMAT_JPEG
-		case utils.MimeTypePNG:
-			format = pb.Format_FORMAT_PNG
-		default:
-		}
-		return format, v.RawData(), nil


Feels really nice getting to delete this

…obotics#5183)

viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jul 29, 2025

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 Jul 29, 2025

hexbabe force-pushed the get-images-poc branch from b664d29 to 202cece Compare July 29, 2025 19:16

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 Jul 29, 2025

hexbabe force-pushed the get-images-poc branch from 202cece to c73c7d9 Compare July 29, 2025 19:19

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 Jul 29, 2025

hexbabe force-pushed the get-images-poc branch from c14d952 to d038ca2 Compare July 31, 2025 15:13

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 Jul 31, 2025

hexbabe force-pushed the get-images-poc branch from 4987baa to fa020c0 Compare August 12, 2025 16:10

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 Aug 12, 2025

viambot added the safe to test This pull request is marked safe to test from a trusted zone label Aug 25, 2025

DRY up with ImagesExactlyEqual helper

9bf4078

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 Aug 25, 2025

hexbabe requested review from SebastianMunozP and removed request for SebastianMunozP August 25, 2025 19:50

seanavery approved these changes Aug 25, 2025

View reviewed changes

SebastianMunozP approved these changes Aug 26, 2025

View reviewed changes

bhaney approved these changes Aug 26, 2025

View reviewed changes

Make encoding err more informative

dcad36a

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 Aug 26, 2025

Add test coverage in client_test.go

edf294c

hexbabe force-pushed the get-images-poc branch from 2d5876b to edf294c Compare August 26, 2025 18:59

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 Aug 26, 2025

Add get images list order test

319e306

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 Aug 26, 2025

Fix error string assert now that there's a new error string

0c8254d

hexbabe force-pushed the get-images-poc branch from 073cfe3 to 0c8254d Compare August 26, 2025 19:12

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 Aug 26, 2025

hexbabe merged commit 1ab734a into viamrobotics:main Aug 26, 2025
18 checks passed

hexbabe deleted the get-images-poc branch August 26, 2025 19:32

danielbotros pushed a commit to danielbotros/rdk that referenced this pull request Aug 29, 2025

RSDK-11613 — Update Go camera API with new GetImages signature (viamr…

d53b898

…obotics#5183)

RSDK-11613 — Update Go camera API with new GetImages signature #5183

RSDK-11613 — Update Go camera API with new GetImages signature #5183

Uh oh!

Conversation

hexbabe commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Manual Tests

Data Manager GetImages

DataManager ReadImage

Stream server

GetImages filter_source_names

Requesting invalid source

Requesting with no sources specified

Requesting source "preloaded" specified

Requesting duplicate sources "color" and "color"

Uh oh!

github-actions bot commented Jul 29, 2025

Uh oh!

SebastianMunozP left a comment

Choose a reason for hiding this comment

Uh oh!

bhaney left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bhaney Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hexbabe commented Jul 29, 2025 •

edited

Loading

bhaney left a comment •

edited

Loading

bhaney Aug 26, 2025 •

edited

Loading