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

Use an enum for known simple message types in Protobuf spec #14

Open
peplin opened this issue Sep 5, 2014 · 5 comments
Open

Use an enum for known simple message types in Protobuf spec #14

peplin opened this issue Sep 5, 2014 · 5 comments

Comments

@peplin
Copy link
Member

peplin commented Sep 5, 2014

At the moment, the TranslatedMessage type in the Protocol Buffers spec has a string field for the name of the message. The binary format is supposed to be tiny - strings are huge!

Since we have a known set of official translated message names (now for both CAN signals and OBD-II responses, it would be more efficient to have a enum in the protobuf spec, one for each known type.

With that, the TranslateMessage could have 2 fields for the name, both optional:

  • One field is an optional enum, referring to an official signal name. That'll take just a byte or two in the serialized binary message.
  • The other field is the existing string name field, for messages outside the standard set. We retain the flexibility of arbitrary names, but pick up a big space savings for anything from the official set.
peplin added a commit that referenced this issue Sep 5, 2014
@peplin
Copy link
Member Author

peplin commented Sep 5, 2014

Things that would need to be changed:

  • Update the protobuf format spec, obviously
  • Bump the version of the message format, because this isn't a backwards compatible change (i.e. clients would be expecting the string name to be there, and it wouldn't be)
  • Modify the vi-firmware so it recognizes the official message names and encodes them as the enums instead of string names, but sticks to strings for unrecognized message names.
  • Modify the Python and Android libraries so they use the new protobuf spec

@zacnelson
Copy link

@peplin sounds like a good move for reducing data rate and space requirements. Do we have an idea of how much space could be saved by going this route? Obviously things like "transmission_gear_position" could greatly be reduced.

Also, just to make sure: this wouldn't affect the JSON stream at all, just the protobuf stream format?

@peplin
Copy link
Member Author

peplin commented Sep 5, 2014

Do we have an idea of how much space could be saved by going this route? Obviously things like "transmission_gear_position" could greatly be reduced.

Yeah, that string is 26 characters, so 26 bytes long. As an enum it would be 1 or 2!

and yes, it only effects binary data, not the JSON.

John Schmotzer over at Ford expressed an interest in tackling this TODO, so I pulled it up to this issue for discussion.

@jwschmo
Copy link

jwschmo commented Sep 5, 2014

Hey guys, thrilled to help!

I'm still a bit on the new side with the Google Proto Buff technology, however, I figured that while I'm learning about them I could contribute to this TODO effort. I'm still getting set up with the proper development environments for OpenXC, however, if you guys have any suggestions on how to get "properly" set up for coding for this specific TODO I'm all ears.

@jwschmo
Copy link

jwschmo commented Sep 8, 2014

So there is a couple things here as far as direction, I can start by providing an estimate of bandwidth reduction by switching to enums for proto buffs, or I can start by tackling Chris' task list:

I would probably start with the documentation ;D

Things that would need to be changed:

•Update the protobuf format spec, obviously
•Bump the version of the message format, because this isn't a backwards compatible change (i.e. clients would be expecting the string name to be there, and it wouldn't be)
•Modify the vi-firmware so it recognizes the official message names and encodes them as the enums instead of string names, but sticks to strings for unrecognized message names.
•Modify the Python and Android libraries so they use the new protobuf spec

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

No branches or pull requests

3 participants