Skip to content

Conversation

purplenicole730
Copy link
Member

@purplenicole730 purplenicole730 commented May 5, 2023

RSDK-1623

GetImage might return a custom MIME type, but for a user to use the data, they need to turn the data into a standard representation.

Changes:

  • Add a helper function that converts the data of a custom MIME type to an array of integers
  • Add a test to make sure that an artifact image converts correctly
  • Add documentation
  • Add a new folder named data/ where we store a fake image

@cheukt
Copy link
Member

cheukt commented May 5, 2023

nit: PR name should be RSDK-1623 - XXX - useful for linking back to JIRA

@purplenicole730 purplenicole730 changed the title Rsdk 1623 decode custom depth mime type Rsdk-1623-decode custom depth mime type May 6, 2023
@purplenicole730 purplenicole730 marked this pull request as ready for review May 9, 2023 16:33
@purplenicole730 purplenicole730 requested a review from a team as a code owner May 9, 2023 16:33
Copy link

@zaporter-work zaporter-work left a comment

Choose a reason for hiding this comment

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

lgtm


width = int.from_bytes(self.data[8:16], "big")
height = int.from_bytes(self.data[16:24], "big")
depth_arr = array("H", self.data[24:])
Copy link

@zaporter-work zaporter-work May 9, 2023

Choose a reason for hiding this comment

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

I am surprised you dont specify the endianness of these unsigned shorts but you do for the other values.

Co-authored-by: Naveed Jooma <naveed@joo.ma>
@purplenicole730 purplenicole730 changed the title Rsdk-1623-decode custom depth mime type RSDK-1623 - decode custom depth mime type May 9, 2023
@purplenicole730 purplenicole730 requested a review from bhaney May 9, 2023 19:03
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.

looking good, could you just add some explicit checks to the tests as well

@clintpurser
Copy link
Member

Looks like you are covered here, I will un assign myself so there aren't too many cooks 👨‍🍳

@clintpurser clintpurser removed their request for review May 10, 2023 15:19
@purplenicole730 purplenicole730 requested a review from bhaney May 10, 2023 18:57
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.

LGTM

@purplenicole730 purplenicole730 merged commit 1334824 into viamrobotics:main May 12, 2023
@purplenicole730 purplenicole730 deleted the RSDK-1623-decode-custom-depth-mime-type branch May 12, 2023 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants