RSDK-4313 - Include data request metadata #378
Conversation
| await stream.send_message(TabularDataByFilterResponse(data=[TabularData(data=struct) for struct in tabular_structs])) | ||
| tabular_structs = [] | ||
| tabular_metadata = [data.metadata for data in self.tabular_response] | ||
| for idx, tabular_data in enumerate(self.tabular_response): |
There was a problem hiding this comment.
I opted against a list comprehension here because I thought it was a bit cluttered and awkward to read. Not a strong preference; happy to revert to a list comprehension if others prefer it.
| `ViamClient`. | ||
| """ | ||
|
|
||
| class TabularData: |
There was a problem hiding this comment.
Returning the actual proto type for tabular data requests seemed like a bad idea because it's awkward to navigate and has a struct (which we wanted to get rid of. But, time_requested and time_received are their own fields, and I thought returning a 4ple would be super ugly. So, I created a class to represent meaningful tabular data, and created an analogous class for binary data for consistency's sake.
Happy to revisit the type question if there's pushback against creating these shadow classes.
| await stream.send_message(TabularDataByFilterResponse(data=[TabularData(data=struct) for struct in tabular_response_structs])) | ||
| tabular_response_structs = [] | ||
| tabular_metadata = [data.metadata for data in self.tabular_response] | ||
| for idx, tabular_data in enumerate(self.tabular_response): |
There was a problem hiding this comment.
see above comment re: use of list comprehension here.
| NANOS_END = 0 | ||
| START_TS = Timestamp(seconds=SECONDS_START, nanos=NANOS_START) | ||
| END_TS = Timestamp(seconds=SECONDS_END, nanos=NANOS_END) | ||
| START_DATETIME = START_TS.ToDatetime() |
There was a problem hiding this comment.
(flyby) I liked the idea of programmatically showing that START_DATETIME and START_TS are equivalent.
| TAGS = ["tag"] | ||
| BBOX_LABELS = ["bbox_label"] | ||
| FILTER = Filter( | ||
| FILTER = DataClient.create_filter( |
There was a problem hiding this comment.
(flyby) we both had a hardcoded filter here and - in one method below - a filter created via create_filter. this seemed redundant so I moved to a default of only using a single filter that is created via our helper method.
| self.tabular_data_requested = False | ||
| self.tabular_response = [{"PowerPct": 0, "IsPowered": False}, {"PowerPct": 0, "IsPowered": False}, {"Position": 0}] | ||
| self.tabular_response = [ | ||
| DataClient.TabularData( |
There was a problem hiding this comment.
(q) In general, what is the "data" being collected? Any data the robot is giving? And what is binary data vs tabular data?
There was a problem hiding this comment.
Great question! The data is currently just all data being collected/stored by the robot in app. I don't think I could give a very good answer on the differences between binary and tabular data. I think tabular is going to be mapped data in a lot of ways (e.g., a mapping of function call to output), while binary data is data that's just stored as bytes (e.g., raw files, or a photo). The data team could definitely give a better answer though!
| time_received=datetime_to_timestamp(tabular_data.time_received) | ||
| ) | ||
| ) | ||
| await stream.send_message(TabularDataByFilterResponse( |
There was a problem hiding this comment.
(q) Where is this message being sent?
There was a problem hiding this comment.
Good question! The message is being sent back along the channel where the request was sent, so basically back to the calling client.
Major changes
Create
BinaryDataandTabularDatatypes to encompass all meaningful data/metadata from associated callsMinor changes
Some small refactors to tests