[RSDK-11780, RSDK-11704] CaptureAllFromCamera in vision/service.py still expects Image.mime_type to be Format type, not str by hexbabe · Pull Request #993 · viamrobotics/viam-python-sdk

hexbabe · 2025-08-28T16:13:08Z

CLOSING since #995 fixes the issue

Fix bug by handling mime type type conversion in vision service server and client

Manual Tests

Test with python vision service

Created a fake vision model that serves only capture_all_from_camera and returns a blank white JPEG https://github.com/hexbabe/heart-beat-module/blob/6e167e9119c0d096bf0fb2cdef50cdcfdb5c3f4a/src/fake_vision.py#L59-L77

Uninstalled the pypi version of viam_sdk and manually installed my current feature branch built local wheel of the SDK to the virtual environment of the module

Used a Python client to call capture_all_from_camera

async def main():
    try:
        machine = await connect()
        fake_vision = VisionClient.from_robot(machine, "fake-vision-1")

        resp = await fake_vision.capture_all_from_camera("camera", True, True, True, True)
        print("captured all from camera")
        print("image.mime_type:", resp.image.mime_type)
        print("image.height:", resp.image.height)
        print("image.width:", resp.image.width)
        img = Image.open(io.BytesIO(resp.image.data))
        img.show()
        await fake_vision.close()
    except Exception as e:
        print("caught exception: ", e)
    finally:
        await machine.close()


if __name__ == '__main__':
  asyncio.run(main())

Output

(.venv) seanyu@ROBOT-G26PRFHC2H python-sdk-client % python3 test_capture_all_from_camera.py
2025-08-28 16:56:16,849         INFO    viam.rpc.dial (dial.py:336)     Connecting to socket: /tmp/proxy-TOxCLdoi.sock
captured all from camera
image.mime_type: CameraMimeType.JPEG
image.height: 480
image.width: 640

Also opened the image and confirmed it was a blank white jpeg

Regression test with `oak-d`

Ran the same test as from #982 (comment) and verified no regression for GetImages server and client.

github-actions · 2025-08-28T16:13:20Z

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	is_moving
board	gpio_pin_by_name
button	push
genericcomponent	do_command
camera	get_image
encoder	get_position
motor	is_moving
sensor	get_readings
servo	get_position
switch	get_position
arm	get_end_position
gantry	get_lengths
gripper	is_moving
movement_sensor	get_linear_acceleration
input_controller	get_controls
audio	get_properties
pose_tracker	get_poses
power_sensor	get_power
motion	get_pose
vision	get_properties
mlmodel	metadata
genericservice	do_command
slam	get_point_cloud_map

njooma

I don't think we need to use .value everywhere. According to the docs, an enum that subclasses from str should behave just like a string

njooma · 2025-08-28T18:40:06Z

src/viam/components/camera/camera.py

+        The extra parameter can be used to pass additional options to the camera resource. The filter_source_names parameter can be used to filter
+        only the images from the specified source names. When unspecified, all images are returned.
+


Add the filter_source_names arg like we do in the get_image docstring. no need to add extra to this list of args

njooma · 2025-08-28T18:40:59Z

src/viam/components/camera/client.py

            else:
                # TODO(RSDK-11728): remove this once we deleted the format field
-                mime_type = str(CameraMimeType.from_proto(img_data.format))
+                mime_type = CameraMimeType.from_proto(img_data.format).value


Does this need to happen? CameraMimeType is a string, and should be accepted wherever strs are accepted

Sorry this PR wasn't really ready to review yet. This repo seems to auto-request reviews. I was just doing this to see if more tests would fail as an experiment. I've changed them back

src/viam/components/camera/client.py

njooma · 2025-08-28T18:42:37Z

src/viam/media/utils/pil/__init__.py

    Args:
        image (Image.Image): The image to convert.
-        mime_type (CameraMimeType): The mime type to convert the image to.
+        mime_type (str): The mime type to convert the image to. Must be one of the `CameraMimeType` enum string literals.


If this must be one of the enum properties, why not keep this as CameraMimeType?

njooma · 2025-08-28T19:52:18Z

src/viam/media/video.py

+    def __str__(self) -> str:
+        return self.value
+


Really only helpful for printing

njooma · 2025-08-28T22:02:37Z

src/viam/media/utils/pil/__init__.py

+    # Make sure at runtime the mime_type string is actually a CameraMimeType
+    if not isinstance(mime_type, CameraMimeType):
+        raise ValueError(f"Unsupported mimetype str: {mime_type}")
+


Were you running into issues here?

No, but the user can pass str in runtime regardless of the hint, so I figured it'd be good to give a more explicit error when that happens.

njooma · 2025-08-28T22:03:52Z

src/viam/services/vision/service.py

        request = await stream.recv_message()
        assert request is not None
-        vision = self.get_resource(request.name)
+        vision: Vision = self.get_resource(request.name)


What you can also do to fix this type issue is on line 29, update the class inheritance to be VisionRPCService(UnimplementedVisionServiceBase, ResourceRPCServiceBase[Vision])

Nice I like that. Done and removed inline hint

bhaney

mostly questions about things for this PR

src/viam/components/camera/client.py

src/viam/components/camera/service.py

src/viam/services/vision/service.py

src/viam/components/camera/service.py

bhaney · 2025-08-29T15:32:52Z

tests/test_vision_service.py

-VISION_IMAGE = ViamImage(bytes([0, 100]), CameraMimeType.JPEG)
+# Use string value of CameraMimeType because ViamImage accepts a string mime type in the worst case
+# and it may not have the expected CameraMimeType methods defined on it.
+VISION_IMAGE = ViamImage(bytes([0, 100]), CameraMimeType.JPEG.value)


Still curious why we want to use a second VISION_IMAGE here when we already have an IMAGE variable (used in other tests) that could be used in the same way.

I believe it's to make writing the mock easier? @njooma ?

I still don't have an answer to this question, so won't approve until then

…on logic

…sts fail due to them

…a service

hexbabe requested a review from a team as a code owner August 28, 2025 16:13

hexbabe requested review from dhritinaidu and njooma August 28, 2025 16:13

hexbabe force-pushed the RSDK-11704 branch from 0669c03 to 24d280d Compare August 28, 2025 16:20

njooma requested changes Aug 28, 2025

View reviewed changes

hexbabe force-pushed the RSDK-11704 branch 3 times, most recently from 769b30b to f8c39c7 Compare August 28, 2025 20:30

hexbabe marked this pull request as draft August 28, 2025 20:52

hexbabe force-pushed the RSDK-11704 branch 2 times, most recently from be404a7 to 162aa7e Compare August 28, 2025 21:05

hexbabe requested review from bhaney and njooma August 28, 2025 21:06

hexbabe marked this pull request as ready for review August 28, 2025 21:06

njooma reviewed Aug 28, 2025

View reviewed changes

bhaney reviewed Aug 29, 2025

View reviewed changes

hexbabe requested review from bhaney and njooma August 29, 2025 18:09

njooma mentioned this pull request Sep 3, 2025

Update CameraMimeType to allow for custom mimes #995

Merged

hexbabe added 6 commits September 7, 2025 14:21

Cast mime type to str always in SDK tests; Fix failing tests and visi…

54e811b

…on logic

Clarify comment

40937f3

Remove .value for mime types as that was just for checking if unit te…

c01d9ab

…sts fail due to them

Remove str override method

f8d7140

Invalid -> unsupported

18a84cd

Change so class inherits

0e4fd50

hexbabe force-pushed the RSDK-11704 branch from ee76831 to 36e3400 Compare September 7, 2025 18:22

Switch order of kwargs to correspond between vision service and camer…

73a6150

…a service

hexbabe force-pushed the RSDK-11704 branch from 36e3400 to 73a6150 Compare September 7, 2025 18:25

hexbabe closed this Sep 7, 2025

		The extra parameter can be used to pass additional options to the camera resource. The filter_source_names parameter can be used to filter
		only the images from the specified source names. When unspecified, all images are returned.

Comments

Conversation

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

Uh oh!

CLOSING since #995 fixes the issue

Manual Tests

Test with python vision service

Regression test with oak-d

Uh oh!

github-actions bot commented Aug 28, 2025

Uh oh!

njooma left a comment

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!

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!

hexbabe Aug 29, 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!

bhaney left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hexbabe commented Aug 28, 2025 •

edited

Loading

Regression test with `oak-d`

hexbabe Aug 29, 2025 •

edited

Loading