New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move structs and serialization to protobuf standard. #9

Closed
balukin opened this Issue Jun 19, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@balukin
Contributor

balukin commented Jun 19, 2017

Currently packets are serialized and deserialized using .NET's binary formatter (Marshal.StructureToPtr) that reads/writes fields in order of declaration in given struct. It also keeps fields aligned to number of bytes specified by Pack variable.

This is not the best way to handle this since it's not easy to find out how fields are aligned, what is the byte order, how fields are stored, etc. Generally this is confusing - see this discussion for example.

We want to move to Protocol Buffers to use widely available serialization and data structure library that should be able to pack and unpack packets properly in all environments.

@mungewell

This comment has been minimized.

Show comment
Hide comment
@mungewell

mungewell Jun 19, 2017

Seems sensible, perhaps you can confirm a few details..

1). You mentions on #4 that the old protocol will remain available (I guess at least for a while). Does this mean that the REQ packet requesting Headset or Controller will 'just' increase 'ProtocolVersion' and receive a different IP:Port to talk ProtoBuf at?

{
"RequestedEndpointName":"HeadTracking",
"ProtocolVersion":1, <---- ie, this changes.
"Code":1
}

2). Protobuf does not (natively) support total message lengths, at least not for the Python implementation. Will the communications remain as one protobuf packet in each REQ/REP transaction?

3). Will you provide '.proto' files (rather than the C++ outputs) to enable easy porting to other languages?

mungewell commented Jun 19, 2017

Seems sensible, perhaps you can confirm a few details..

1). You mentions on #4 that the old protocol will remain available (I guess at least for a while). Does this mean that the REQ packet requesting Headset or Controller will 'just' increase 'ProtocolVersion' and receive a different IP:Port to talk ProtoBuf at?

{
"RequestedEndpointName":"HeadTracking",
"ProtocolVersion":1, <---- ie, this changes.
"Code":1
}

2). Protobuf does not (natively) support total message lengths, at least not for the Python implementation. Will the communications remain as one protobuf packet in each REQ/REP transaction?

3). Will you provide '.proto' files (rather than the C++ outputs) to enable easy porting to other languages?

@balukin

This comment has been minimized.

Show comment
Hide comment
@balukin

balukin Jun 21, 2017

Contributor
  1. Something like this, yes. ProtocolVersion field will be used to decide which endpoint to create on the server side. Probably it will be the same ip:port because endpoints are client-exclusive now. If you request an endpoint while some other app is using it, it will response with InUse code.
  2. Not decided yet but probably yes. Most likely one ZMQ frame will equal to one protobuf byte[] blob.
  3. Yes. We plan to include .proto files in this repo. This way we can make sure that data layout is exactly the same on both client and server side.
Contributor

balukin commented Jun 21, 2017

  1. Something like this, yes. ProtocolVersion field will be used to decide which endpoint to create on the server side. Probably it will be the same ip:port because endpoints are client-exclusive now. If you request an endpoint while some other app is using it, it will response with InUse code.
  2. Not decided yet but probably yes. Most likely one ZMQ frame will equal to one protobuf byte[] blob.
  3. Yes. We plan to include .proto files in this repo. This way we can make sure that data layout is exactly the same on both client and server side.

@balukin balukin self-assigned this Jun 26, 2017

@balukin balukin added this to the v3 milestone Jul 28, 2017

@balukin

This comment has been minimized.

Show comment
Hide comment
@balukin

balukin Aug 8, 2017

Contributor

It's currently being tested in 1.5.0 beta but docs were not updated yet.

Contributor

balukin commented Aug 8, 2017

It's currently being tested in 1.5.0 beta but docs were not updated yet.

@balukin

This comment has been minimized.

Show comment
Hide comment
@balukin

balukin Mar 12, 2018

Contributor

Finally updated public code to protobuf + v3
68f0d71

.proto available here;
https://github.com/RiftCat/vridge-api/blob/master/src/vridge-api.proto

Contributor

balukin commented Mar 12, 2018

Finally updated public code to protobuf + v3
68f0d71

.proto available here;
https://github.com/RiftCat/vridge-api/blob/master/src/vridge-api.proto

@balukin balukin closed this Mar 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment