iamandi / nanopb

Automatically exported from code.google.com/p/nanopb
zlib License
0 stars 0 forks source link

Add option for defining message identifiers #151

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Describe the requested feature:
To properly decode protocol buffers messages, the correct message type has to 
be known. However, the .proto format provides no identifiers for messages. 
Using union messages / oneofs provides one way to do this, but is not very 
elegant when multiple files and large number of messages is involved.

Instead this could be offered as an option:

- Custom nanopb message-level option "uint32 msgid":

message msg1 {
   option (nanopb_msgopt).msgid = 1234;
   ..
}

- Compiler generates #define list of messages in each .pb.h file:

#define PB_MSG_1234 msg1
#define PB_MSG_5678 msg2

#define MYPROTOFILE1_MESSAGES PB_MSG(1234) PB_MSG(5678)

- The definition of PB_MSG_1234 etc. will give errors if multiple .h's with 
conflicting msgid's are included.

- The X-macros in the MYPROTOFILE1_MESSAGES can be used to e.g. construct list 
of messages like this:

typedef struct { uint32_t msgid; size_t size; pb_field_t *fields; } msginfo_t;

#define PB_MSG(id) {id, sizeof(PB_MSG_ ## id), PB_MSG_ ## id ## _fields},
msginfo_t msgs[] = {
  MYPROTOFILE1_MESSAGES
  MYPROTOFILE2_MESSAGES
  ..
};

- Only messages that have msgid option are included in the list. This way 
submessages will not be publicly exposed and do not need unique id's.

See here for prior discussion:
https://groups.google.com/forum/#!topic/nanopb/pZcrj-zbVEI

Original issue reported on code.google.com by Petteri.Aimonen on 21 Mar 2015 at 3:20

GoogleCodeExporter commented 9 years ago

Original comment by Petteri.Aimonen on 21 Mar 2015 at 3:23

GoogleCodeExporter commented 9 years ago
I like it! What I'll do is:

try this on two proto files, run nanopb_generator on them, and hand-modify the 
output along these lines
then cook up a third .c file which includes the npb.h files and exports the 
struct as I need it, and see  to
layer my mapping lookup API ontop.

will publish results as a git repo so the diffs are at hand easily.

Thanks!

- Michael

Original comment by haberl...@gmail.com on 21 Mar 2015 at 6:41

GoogleCodeExporter commented 9 years ago
what exactly did you mean by size - the generated '<msgname>_size' #define like 
this comment?

/* Maximum encoded size of messages (where known) */

or the size of the fields array?

Original comment by haberl...@gmail.com on 21 Mar 2015 at 9:59

GoogleCodeExporter commented 9 years ago
oh I see, the size of the generated struct. DIsregard.

Original comment by haberl...@gmail.com on 21 Mar 2015 at 10:00

GoogleCodeExporter commented 9 years ago
works! see  https://github.com/mhaberler/nanopb/commits/msgid-option

Original comment by haberl...@gmail.com on 21 Mar 2015 at 10:08

GoogleCodeExporter commented 9 years ago
my last wish on the issue: find a way to expand these fields in the PB_MSG macro

would it cause major breakage if you always emitted the #define as per below, 
but say as -1 if the size is not known?

/* Maximum encoded size of messages (where known) */
#define SecondMessage_size                       2
#define ThirdMessage_size                        2

I dont see a way to expand this in PB_MSG if the symbol isnt defined at all, do 
you?

Original comment by haberl...@gmail.com on 21 Mar 2015 at 10:32

GoogleCodeExporter commented 9 years ago
Hmm, I don't see a way either. Defining unknown messages sizes as -1 should be 
safe enough, as constructs like "uint8_t buffer[FooMsg_size];" should warn for 
that.

Took a quick look at the changes, seems good.

Original comment by Petteri.Aimonen on 22 Mar 2015 at 9:03

GoogleCodeExporter commented 9 years ago

Original comment by Petteri.Aimonen on 3 Apr 2015 at 4:50

GoogleCodeExporter commented 9 years ago
Fix released in nanopb-0.3.3

Original comment by Petteri.Aimonen on 10 Apr 2015 at 6:06