Add protobuf support to KITTI example #499
Conversation
| let pbType = XVIZ_PROTOBUF_MESSAGE.StateUpdate; | ||
| let pbMsg = pbType.fromObject(pbJSON); | ||
|
|
||
| if (this.options.envelope) { |
There was a problem hiding this comment.
This enveloping looks like it should be in a helper function since it's used below too and a bit complex.
| // 2-frame is where the actual XVIZ updates begin | ||
| const messageName = index => `${index + 2}-frame`; | ||
|
|
||
| export class XVIZProtobufWriter extends XVIZBaseWriter { |
There was a problem hiding this comment.
Is the message timing bits here protobuf specific, or could it be in the base class?
There was a problem hiding this comment.
You are correct, it is shared. But I will address this in a separate PR
| return color; | ||
| } | ||
|
|
||
| // Recursively walk object performing the following conversions |
There was a problem hiding this comment.
Maybe explain what this is converting the protobuf into, I think it's the JSON format right?
There was a problem hiding this comment.
Actually, its the JSON format from builder works for other encodings but Protobuf is more strict (doesn't not support variants such as fields with both string and array values in JSON). So this normalize for protobuf encoding, which is what I'll explain in the comment.
| // | ||
| // Test data and cases for successful writer tests | ||
| // | ||
| const SAMPLE_METADATA = { |
There was a problem hiding this comment.
Should this test include things we are having enum trouble with?
There was a problem hiding this comment.
The enum's issue is covered in parsing related tests in #503
|
sorry, the base branch to merge into was master instead of #499. Fixed, now this properly only shows 2 files difference. |
7bc4f77 to
5678656
Compare
Add the option --protobuf which will output the #-frame.pbe formatted XVIZ Messages.
Add the option
--protobufwhich will output the#-frame.pbeformatted XVIZ Messages.