Protobuf Formatting and Style Guide (UBC-Thunderbots#1285)

* Add proto support to clang format and format all proto files

* Protobuf style guide

* Fix build errors

Author: jonathanlew

.clang-format

@@ -31,6 +31,29 @@ Language: JavaScript
 DisableFormat: true
 ---
 Language: Proto
-# Don't format proto files.
-DisableFormat: true
+BasedOnStyle: Google
+AlignAfterOpenBracket: Align
+AlignConsecutiveAssignments: 'true'
+AlignEscapedNewlinesLeft: 'false'
+AllowShortBlocksOnASingleLine: 'false'
+AllowShortCaseLabelsOnASingleLine: 'false'
+AllowShortFunctionsOnASingleLine: Empty
+AllowShortIfStatementsOnASingleLine: 'false'
+AllowShortLoopsOnASingleLine: 'false'
+BinPackArguments: 'true'
+BinPackParameters: 'true'
+BreakBeforeBraces: Allman
+BreakStringLiterals: 'false'
+ColumnLimit: '90'
+Cpp11BracedListStyle: 'true'
+IncludeBlocks: Regroup
+IndentCaseLabels: 'true'
+IndentWidth: '4'
+MaxEmptyLinesToKeep: '3'
+NamespaceIndentation: All
+ReflowComments: 'true'
+SortIncludes: 'true'
+Standard: Cpp11
+TabWidth: '4'
+UseTab: Never
 ---

docs/code-style-guide.md

@@ -12,6 +12,7 @@
 * [Getter And Setter Functions](#getter-and-setter-functions)
 * [Spelling](#spelling)
 * [Miscellaneous](#miscellaneous)
+* [Protobuf](#protobuf)
 
 
 Our C++ coding style is based off of [Google's C++ Style Guide](https://google.github.io/styleguide/cppguide.html). We use [clang-format](https://clang.llvm.org/docs/ClangFormat.html) to enforce most of the nit-picky parts of the style, such as brackets and alignment, so this document highlights the important rules to follow that clang-format cannot enforce.
@@ -315,3 +316,6 @@ Some general guidelines when writing tests are:
   c[i] = i + 1;
   ```
 
+### Protobuf
+
+Protobufs that we define should follow [Google's Protobuf Style Guide](https://developers.google.com/protocol-buffers/docs/style).

formatting_scripts/fix_formatting.sh

@@ -14,7 +14,7 @@ CURR_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 BAZEL_ROOT_DIR="$CURR_DIR/../src"
 
 # Extensions to check formatting for clang-format
-CLANG_FORMAT_EXTENSIONS=(h cpp c hpp tpp)
+CLANG_FORMAT_EXTENSIONS=(h cpp c hpp tpp proto)
 
 # Function to run clang format
 function run_clang_format () {

src/firmware_new/boards/frankie_v1/udp_multicast.c

@@ -18,8 +18,8 @@
 // TODO the messages are global for now, will need to be moved
 // when this file is properly integrated. This file only serves
 // as a reference implementation of reliable multicast with IPv6
-robot_ack ack       = robot_ack_init_zero;
-control_msg control = control_msg_init_zero;
+RobotAck ack       = RobotAck_init_zero;
+ControlMsg control = ControlMsg_init_zero;
 
 /*
  * Thread that creates a send and recv socket, joins the specified
@@ -71,7 +71,7 @@ static void blocking_udp_multicast_loop(void *arg)
     netconn_join_leave_group(recvconn, &config->multicast_address, NULL, NETCONN_JOIN);
 
     // this buffer is used to hold serialized proto
-    uint8_t buffer[robot_ack_size];
+    uint8_t buffer[RobotAck_size];
 
     while (1)
     {
@@ -80,20 +80,20 @@ static void blocking_udp_multicast_loop(void *arg)
         if (err == ERR_OK)
         {
             tx_buf = netbuf_new();
-            netbuf_alloc(tx_buf, robot_ack_size);
+            netbuf_alloc(tx_buf, RobotAck_size);
 
             // Create a stream that reads from the buffer
             pb_istream_t in_stream =
                 pb_istream_from_buffer((uint8_t *)rx_buf->p->payload, rx_buf->p->tot_len);
 
-            if (pb_decode(&in_stream, control_msg_fields, &control))
+            if (pb_decode(&in_stream, ControlMsg_fields, &control))
             {
                 // update proto
                 ack.msg_count = msg_count++;
 
                 // serialize proto
                 pb_ostream_t stream = pb_ostream_from_buffer(buffer, sizeof(buffer));
-                pb_encode(&stream, robot_ack_fields, &ack);
+                pb_encode(&stream, RobotAck_fields, &ack);
 
                 // package payload and send over udp
                 tx_buf->p->payload = buffer;

src/firmware_new/proto/control.proto

@@ -3,17 +3,16 @@ syntax = "proto2";
 // A test robot msg which serves as an example to modify and use to
 // facilitate firmware testing.
 
-message control_msg {
-
-    required wheel_speed_msg wheel_1_control = 1;
-    optional wheel_speed_msg wheel_2_control = 2;
-    optional wheel_speed_msg wheel_3_control = 3;
-    optional wheel_speed_msg wheel_4_control = 4;
-
+message ControlMsg
+{
+    required WheelSpeedMsg wheel_1_control = 1;
+    optional WheelSpeedMsg wheel_2_control = 2;
+    optional WheelSpeedMsg wheel_3_control = 3;
+    optional WheelSpeedMsg wheel_4_control = 4;
 }
 
-message wheel_speed_msg {
-
+message WheelSpeedMsg
+{
     // speed at which to rotate
     required uint32 rpm = 1;
 
@@ -22,6 +21,7 @@ message wheel_speed_msg {
 }
 
 // Robot response
-message robot_ack {
+message RobotAck
+{
     required uint32 msg_count = 1;
 }

src/firmware_new/tools/communication/robot_communicator.cpp

@@ -77,4 +77,4 @@ void RobotCommunicator<SendProto, ReceiveProto>::send_loop(
 }
 
 // place all templated communcation msg send/receive pair initializations here
-template class RobotCommunicator<control_msg, robot_ack>;
+template class RobotCommunicator<ControlMsg, RobotAck>;

src/firmware_new/tools/send_proto_over_udp.cpp

@@ -22,27 +22,27 @@ int main(int argc, char* argv[])
 
     // we create a wheel control msg, and request wheel 1 to spin at 100 rpm forwards
     // these wheel profile will be used across multiple wheels
-    wheel_speed_msg wheel_control;
-    wheel_control.set_rpm(69);
-    wheel_control.set_forwards(true);
+    WheelSpeedMsg WheelControl;
+    WheelControl.set_rpm(69);
+    WheelControl.set_forwards(true);
 
     // turn two of the wheels on with this profile
     // NOTE that the other two wheels are not being populated
-    control_msg control_req;
-    control_req.mutable_wheel_1_control()->CopyFrom(wheel_control);
-    control_req.mutable_wheel_2_control()->CopyFrom(wheel_control);
-    control_req.mutable_wheel_2_control()->CopyFrom(wheel_control);
+    ControlMsg control_req;
+    control_req.mutable_wheel_1_control()->CopyFrom(WheelControl);
+    control_req.mutable_wheel_2_control()->CopyFrom(WheelControl);
+    control_req.mutable_wheel_2_control()->CopyFrom(WheelControl);
 
     int count = 0;
 
     // create a RobotCommunicator with a NetworkMedium
-    RobotCommunicator<control_msg, robot_ack> communicator(
+    RobotCommunicator<ControlMsg, RobotAck> communicator(
         std::make_unique<NetworkMedium>(std::string(AI_MULTICAST_ADDRESS) + "%eth0",
                                         AI_MULTICAST_SEND_PORT, AI_UNICAST_LISTEN_PORT),
-        [&](const control_msg& msg) {
+        [&](const ControlMsg& msg) {
             std::cout << "COMP Txed " << count++ << " msgs " << std::endl;
         },
-        [&](const robot_ack& msg) {
+        [&](const RobotAck& msg) {
             std::cout << "STM32 Rxed " << msg.msg_count() << " msgs " << std::endl;
         });
 

src/software/proto/grSim_Commands.proto

@@ -1,24 +1,24 @@
 syntax = "proto2";
 
-message grSim_Robot_Command {
-required uint32 id = 1;
-required float kickspeedx = 2;
-required float kickspeedz = 3;
-required float veltangent = 4;
-required float velnormal  = 5;
-required float velangular = 6;
-required bool spinner = 7;
-required bool wheelsspeed = 8;
-optional float wheel1 = 9;
-optional float wheel2 = 10;
-optional float wheel3 = 11;
-optional float wheel4 = 12;
+message grSim_Robot_Command
+{
+    required uint32 id        = 1;
+    required float kickspeedx = 2;
+    required float kickspeedz = 3;
+    required float veltangent = 4;
+    required float velnormal  = 5;
+    required float velangular = 6;
+    required bool spinner     = 7;
+    required bool wheelsspeed = 8;
+    optional float wheel1     = 9;
+    optional float wheel2     = 10;
+    optional float wheel3     = 11;
+    optional float wheel4     = 12;
 }
 
-message grSim_Commands {
-required double timestamp = 1;
-required bool isteamyellow = 2;
-repeated grSim_Robot_Command robot_commands = 3;
+message grSim_Commands
+{
+    required double timestamp                   = 1;
+    required bool isteamyellow                  = 2;
+    repeated grSim_Robot_Command robot_commands = 3;
 }
-
-

src/software/proto/grSim_Packet.proto

@@ -1,7 +1,8 @@
 syntax = "proto2";
 import "software/proto/grSim_Commands.proto";
 import "software/proto/grSim_Replacement.proto";
-message grSim_Packet {
-    optional grSim_Commands commands = 1;
+message grSim_Packet
+{
+    optional grSim_Commands commands       = 1;
     optional grSim_Replacement replacement = 2;
 }

src/software/proto/grSim_Replacement.proto

@@ -1,21 +1,24 @@
 syntax = "proto2";
-message grSim_RobotReplacement {
-required double x=1;
-required double y=2;
-required double dir=3;
-required uint32 id=4;
-required bool yellowteam=5;
-optional bool turnon=6;
+message grSim_RobotReplacement
+{
+    required double x        = 1;
+    required double y        = 2;
+    required double dir      = 3;
+    required uint32 id       = 4;
+    required bool yellowteam = 5;
+    optional bool turnon     = 6;
 }
 
-message grSim_BallReplacement {
-required double x=1;
-required double y=2;
-required double vx=3;
-required double vy=4;
+message grSim_BallReplacement
+{
+    required double x  = 1;
+    required double y  = 2;
+    required double vx = 3;
+    required double vy = 4;
 }
 
-message grSim_Replacement {
-optional grSim_BallReplacement ball = 1;
-repeated grSim_RobotReplacement robots = 2;
+message grSim_Replacement
+{
+    optional grSim_BallReplacement ball    = 1;
+    repeated grSim_RobotReplacement robots = 2;
 }

src/software/proto/messages_robocup_ssl_detection.proto

@@ -1,32 +1,35 @@
 syntax = "proto2";
 
-message SSL_DetectionBall {
-  required float  confidence = 1;
-  optional uint32 area       = 2;
-  required float  x          = 3;
-  required float  y          = 4;
-  optional float  z          = 5;
-  required float  pixel_x    = 6;
-  required float  pixel_y    = 7;
+message SSL_DetectionBall
+{
+    required float confidence = 1;
+    optional uint32 area      = 2;
+    required float x          = 3;
+    required float y          = 4;
+    optional float z          = 5;
+    required float pixel_x    = 6;
+    required float pixel_y    = 7;
 }
 
-message SSL_DetectionRobot {
-  required float  confidence  =  1;
-  optional uint32 robot_id    =  2;
-  required float  x           =  3;
-  required float  y           =  4;
-  optional float  orientation =  5;
-  required float  pixel_x     =  6;
-  required float  pixel_y     =  7;
-  optional float  height      =  8;
+message SSL_DetectionRobot
+{
+    required float confidence  = 1;
+    optional uint32 robot_id   = 2;
+    required float x           = 3;
+    required float y           = 4;
+    optional float orientation = 5;
+    required float pixel_x     = 6;
+    required float pixel_y     = 7;
+    optional float height      = 8;
 }
 
-message SSL_DetectionFrame {
-  required uint32             frame_number  = 1;
-  required double             t_capture     = 2;
-  required double             t_sent        = 3;
-  required uint32             camera_id     = 4;
-  repeated SSL_DetectionBall  balls         = 5;
-  repeated SSL_DetectionRobot robots_yellow = 6;
-  repeated SSL_DetectionRobot robots_blue   = 7;
+message SSL_DetectionFrame
+{
+    required uint32 frame_number              = 1;
+    required double t_capture                 = 2;
+    required double t_sent                    = 3;
+    required uint32 camera_id                 = 4;
+    repeated SSL_DetectionBall balls          = 5;
+    repeated SSL_DetectionRobot robots_yellow = 6;
+    repeated SSL_DetectionRobot robots_blue   = 7;
 }