Skip to content
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

Consider a top-level union message #1

Open
kentonv opened this issue May 29, 2015 · 5 comments
Open

Consider a top-level union message #1

kentonv opened this issue May 29, 2015 · 5 comments

Comments

@kentonv
Copy link

kentonv commented May 29, 2015

I notice each of your message types has a comment assigning it a number. Probably what you want is one union message that encapsulates the others:

struct Message {
  union {
    connect @0 :ConnectMsg;
    connected @1 :ConnectedMsg;
    failed @2 :FailedMsg;
    #...
  }
}

Now you can define your protocol as "a stream of Message"; no need to encode the type of each message separately. Notice that the field numbers in the union are the IDs; you don't have to define numeric type IDs separately.

You might want to look at Cap'n Proto's own rpc.capnp for more inspiration.

@thani-sh
Copy link
Contributor

hi, thanks for the info. We'll definitely check it out.

On Fri, May 29, 2015, 06:32 Kenton Varda notifications@github.com wrote:

I notice each of your message types has a comment assigning it a number.
Probably what you want is one union message that encapsulates the others:

struct Message {
union {
connect @0 :ConnectMsg;
connected @1 :ConnectedMsg;
failed @2 :FailedMsg;
#...
}
}

Now you can define your protocol as "a stream of Message"; no need to
encode the type of each message separately. Notice that the field numbers
in the union are the IDs; you don't have to define numeric type IDs
separately.

You might want to look at Cap'n Proto's own rpc.capnp
https://github.com/sandstorm-io/capnproto/blob/master/c++/src/capnp/rpc.capnp
for more inspiration.


Reply to this email directly or view it on GitHub
#1.

@thani-sh
Copy link
Contributor

Hi @kentonv, I've just added the Message struct. Once again, thanks for the tip and thanks a lot for cap'n proto :)

@kentonv
Copy link
Author

kentonv commented May 30, 2015

Cool.

BTW, I notice you're representing params and results as Data. If the idea is that these will contain some other message format (perhaps a binary JSON representation?) that makes sense, but if the goal is to embed Cap'n Proto objects of arbitrary type, then you should use AnyPointer instead. (Note that List(AnyPointer) isn't allowed for technical reasons, but you can create a struct like struct Param { value @0 :AnyPointer } and then do List(Param).)

@thani-sh
Copy link
Contributor

I guess this will save some cpu time so spent on encoding/decoding structs.
On the other hand, if the user wants to send some arbitrary binary data, (
like images, audio ) it's easier with Data.

I'll leave this issue open, we'll see how bddp is actually used and decide.

On Sat, May 30, 2015, 05:35 Kenton Varda notifications@github.com wrote:

Cool.

BTW, I notice you're representing params and results as Data. If the idea
is that these will contain some other message format (perhaps a binary JSON
representation?) that makes sense, but if the goal is to embed Cap'n Proto
objects of arbitrary type, then you should use AnyPointer instead. (Note
that List(AnyPointer) isn't allowed for technical reasons, but you can
create a struct like struct Param { value @0 :AnyPointer } and then do
List(Param).)


Reply to this email directly or view it on GitHub
#1 (comment).

@kentonv
Copy link
Author

kentonv commented May 30, 2015

An AnyPointer value can be any pointer type, which includes Data, so you're covered. :)

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

No branches or pull requests

2 participants