Extend inline / fixed length bytes array support (issue #244)

Adds support for proto3 and POINTER field types to have
fixed length bytes arrays. Also changed the .proto option
to a separate fixed_length:true, while also supporting the old FT_INLINE
option.

Restructured the generator and decoder logic to threat the inline
bytes fields more like "just another field type".

7 files changed, 97 insertions, 85 deletions

diff --git a/docs/concepts.rst b/docs/concepts.rst
index ea33863..2e0d3f9 100644
--- a/docs/concepts.rst
+++ b/docs/concepts.rst
@@ -148,7 +148,7 @@ Most Protocol Buffers datatypes have directly corresponding C datatypes, such as
 
 1) Strings, bytes and repeated fields of any type map to callback functions by default.
 2) If there is a special option *(nanopb).max_size* specified in the .proto file, string maps to null-terminated char array and bytes map to a structure containing a char array and a size field.
-3) If *(nanopb).type* is set to *FT_INLINE* and *(nanopb).max_size* is also set, then bytes map to an inline byte array of fixed size.
+3) If *(nanopb).fixed_length* is set to *true* and *(nanopb).max_size* is also set, then bytes map to an inline byte array of fixed size.
 4) If there is a special option *(nanopb).max_count* specified on a repeated field, it maps to an array of whatever type is being repeated. Another field will be created for the actual number of entries stored.
 
 =============================================================================== =======================
@@ -164,7 +164,7 @@ required bytes data = 1 [(nanopb).max_size = 40];
                                                                                 |    pb_byte_t bytes[40];
                                                                                 | } Person_data_t;
                                                                                 | Person_data_t data;
-required bytes data = 1 [(nanopb).max_size = 40, (nanopb).type = FT_INLINE];    | pb_byte_t data[40];
+required bytes data = 1 [(nanopb).max_size = 40, (nanopb).fixed_length = true]; | pb_byte_t data[40];
 =============================================================================== =======================
 
 The maximum lengths are checked in runtime. If string/bytes/array exceeds the allocated length, *pb_decode* will return false.
diff --git a/docs/reference.rst b/docs/reference.rst
index ef3867a..e59a0c9 100644
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -77,11 +77,10 @@ int_size                       Override the integer type of a field.
 type                           Type of the generated field. Default value
                                is *FT_DEFAULT*, which selects automatically.
                                You can use *FT_CALLBACK*, *FT_POINTER*,
-                               *FT_STATIC*, *FT_IGNORE*, or *FT_INLINE* to
+                               *FT_STATIC* or *FT_IGNORE* to
                                force a callback field, a dynamically
-                               allocated field, a static field, to
-                               completely ignore the field or to
-                               generate an inline bytes field.
+                               allocated field, a static field or to
+                               completely ignore the field.
 long_names                     Prefix the enum name to the enum value in
                                definitions, i.e. *EnumName_EnumValue*. Enabled
                                by default.
@@ -94,6 +93,7 @@ no_unions                      Generate 'oneof' fields as optional fields
 msgid                          Specifies a unique id for this message type.
                                Can be used by user code as an identifier.
 anonymous_oneof                Generate 'oneof' fields as anonymous unions.
+fixed_length                   Generate 'bytes' fields with constant length.
 ============================  ================================================
 
 These options can be defined for the .proto files before they are converted
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index ca60c03..9cce6a5 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -262,10 +262,12 @@ class Field:
         self.enc_size = None
         self.ctype = None
 
-        self.inline = None
         if field_options.type == nanopb_pb2.FT_INLINE:
+            # Before nanopb-0.3.8, fixed length bytes arrays were specified
+            # by setting type to FT_INLINE. But to handle pointer typed fields,
+            # it makes sense to have it as a separate option.
             field_options.type = nanopb_pb2.FT_STATIC
-            self.inline = nanopb_pb2.FT_INLINE
+            field_options.fixed_length = True
 
         # Parse field options
         if field_options.HasField("max_size"):
@@ -349,17 +351,17 @@ class Field:
                 self.array_decl += '[%d]' % self.max_size
                 self.enc_size = varint_max_size(self.max_size) + self.max_size
         elif desc.type == FieldD.TYPE_BYTES:
-            self.pbtype = 'BYTES'
-            if self.allocation == 'STATIC':
-                # Inline STATIC for BYTES is like STATIC for STRING.
-                if self.inline:
-                    self.ctype = 'pb_byte_t'
-                    self.array_decl += '[%d]' % self.max_size
-                else:
-                    self.ctype = self.struct_name + self.name + 't'
+            if field_options.fixed_length:
+                self.pbtype = 'FIXED_LENGTH_BYTES'
                 self.enc_size = varint_max_size(self.max_size) + self.max_size
-            elif self.allocation == 'POINTER':
+                self.ctype = 'pb_byte_t'
+                self.array_decl += '[%d]' % self.max_size
+            else:
+                self.pbtype = 'BYTES'
                 self.ctype = 'pb_bytes_array_t'
+                if self.allocation == 'STATIC':
+                    self.ctype = self.struct_name + self.name + 't'
+                    self.enc_size = varint_max_size(self.max_size) + self.max_size
         elif desc.type == FieldD.TYPE_MESSAGE:
             self.pbtype = 'MESSAGE'
             self.ctype = self.submsgname = names_from_type_name(desc.type_name)
@@ -379,6 +381,9 @@ class Field:
             if self.pbtype == 'MESSAGE':
                 # Use struct definition, so recursive submessages are possible
                 result += '    struct _%s *%s;' % (self.ctype, self.name)
+            elif self.pbtype == 'FIXED_LENGTH_BYTES':
+                # Pointer to fixed size array
+                result += '    %s (*%s)%s;' % (self.ctype, self.name, self.array_decl)
             elif self.rules == 'REPEATED' and self.pbtype in ['STRING', 'BYTES']:
                 # String/bytes arrays need to be defined as pointers to pointers
                 result += '    %s **%s;' % (self.ctype, self.name)
@@ -396,7 +401,7 @@ class Field:
 
     def types(self):
         '''Return definitions for any special types this field might need.'''
-        if self.pbtype == 'BYTES' and self.allocation == 'STATIC' and not self.inline:
+        if self.pbtype == 'BYTES' and self.allocation == 'STATIC':
             result = 'typedef PB_BYTES_ARRAY_T(%d) %s;\n' % (self.max_size, self.ctype)
         else:
             result = ''
@@ -425,10 +430,9 @@ class Field:
             if self.pbtype == 'STRING':
                 inner_init = '""'
             elif self.pbtype == 'BYTES':
-                if self.inline:
-                    inner_init = '{0}'
-                else:
-                    inner_init = '{0, {0}}'
+                inner_init = '{0, {0}}'
+            elif self.pbtype == 'FIXED_LENGTH_BYTES':
+                inner_init = '{0}'
             elif self.pbtype in ('ENUM', 'UENUM'):
                 inner_init = '(%s)0' % self.ctype
             else:
@@ -440,15 +444,15 @@ class Field:
             elif self.pbtype == 'BYTES':
                 data = ['0x%02x' % ord(c) for c in self.default]
                 if len(data) == 0:
-                    if self.inline:
-                        inner_init = '{0}'
-                    else:
-                        inner_init = '{0, {0}}'
+                    inner_init = '{0, {0}}'
+                else:
+                    inner_init = '{%d, {%s}}' % (len(data), ','.join(data))
+            elif self.pbtype == 'FIXED_LENGTH_BYTES':
+                data = ['0x%02x' % ord(c) for c in self.default]
+                if len(data) == 0:
+                    inner_init = '{0}'
                 else:
-                    if self.inline:
-                        inner_init = '{%s}' % ','.join(data)
-                    else:
-                        inner_init = '{%d, {%s}}' % (len(data), ','.join(data))
+                    inner_init = '{%s}' % ','.join(data)
             elif self.pbtype in ['FIXED32', 'UINT32']:
                 inner_init = str(self.default) + 'u'
             elif self.pbtype in ['FIXED64', 'UINT64']:
@@ -500,8 +504,10 @@ class Field:
         elif self.pbtype == 'BYTES':
             if self.allocation != 'STATIC':
                 return None # Not implemented
-            if self.inline:
-                array_decl = '[%d]' % self.max_size
+        elif self.pbtype == 'FIXED_LENGTH_BYTES':
+            if self.allocation != 'STATIC':
+                return None # Not implemented
+            array_decl = '[%d]' % self.max_size
 
         if declaration_only:
             return 'extern const %s %s_default%s;' % (ctype, self.struct_name + self.name, array_decl)
@@ -530,7 +536,7 @@ class Field:
         result += '%3d, ' % self.tag
         result += '%-8s, ' % self.pbtype
         result += '%s, ' % self.rules
-        result += '%-8s, ' % (self.allocation if not self.inline else "INLINE")
+        result += '%-8s, ' % self.allocation
         
         if union_index is not None and union_index > 0:
             result += 'UNION, '
@@ -547,7 +553,7 @@ class Field:
             result += '&%s_fields)' % self.submsgname
         elif self.default is None:
             result += '0)'
-        elif self.pbtype in ['BYTES', 'STRING'] and self.allocation != 'STATIC':
+        elif self.pbtype in ['BYTES', 'STRING', 'FIXED_LENGTH_BYTES'] and self.allocation != 'STATIC':
             result += '0)' # Arbitrary size default values not implemented
         elif self.rules == 'OPTEXT':
             result += '0)' # Default value for extensions is not implemented
@@ -653,7 +659,6 @@ class ExtensionRange(Field):
         self.default = None
         self.max_size = 0
         self.max_count = 0
-        self.inline = None
 
     def __str__(self):
         return '    pb_extension_t *extensions;'
@@ -731,7 +736,6 @@ class OneOf(Field):
         self.default = None
         self.rules = 'ONEOF'
         self.anonymous = False
-        self.inline = None
 
     def add_field(self, field):
         if field.allocation == 'CALLBACK':
diff --git a/generator/proto/nanopb.proto b/generator/proto/nanopb.proto
index 7d39e1c..e4c1da7 100644
--- a/generator/proto/nanopb.proto
+++ b/generator/proto/nanopb.proto
@@ -16,7 +16,7 @@ enum FieldType {
     FT_POINTER = 4; // Always generate a dynamically allocated field.
     FT_STATIC = 2; // Generate a static field or raise an exception if not possible.
     FT_IGNORE = 3; // Ignore the field completely.
-    FT_INLINE = 5; // Always generate an inline array of fixed size.
+    FT_INLINE = 5; // Legacy option, use the separate 'fixed_length' option instead
 }
 
 enum IntSize {
@@ -77,6 +77,9 @@ message NanoPBOptions {
 
   // Generate an enum->string mapping function (can take up lots of space).
   optional bool enum_to_string = 13 [default = false];
+
+  // Generate bytes arrays with fixed length
+  optional bool fixed_length = 15 [default = false];
 }
 
 // Extensions to protoc 'Descriptor' type in order to define options
diff --git a/pb.h b/pb.h
index f68a1cc..a3001ba 100644
--- a/pb.h
+++ b/pb.h
@@ -427,19 +427,6 @@ struct pb_extension_s {
     pb_membersize(st, m[0]), \
     pb_arraysize(st, m), ptr}
 
-#define PB_REQUIRED_INLINE(tag, st, m, fd, ltype, ptr) \
-    {tag, PB_ATYPE_STATIC | PB_HTYPE_REQUIRED | PB_LTYPE_FIXED_LENGTH_BYTES, \
-    fd, 0, pb_membersize(st, m), 0, ptr}
-
-/* Optional fields add the delta to the has_ variable. */
-#define PB_OPTIONAL_INLINE(tag, st, m, fd, ltype, ptr) \
-    {tag, PB_ATYPE_STATIC | PB_HTYPE_OPTIONAL | PB_LTYPE_FIXED_LENGTH_BYTES, \
-    fd, \
-    pb_delta(st, has_ ## m, m), \
-    pb_membersize(st, m), 0, ptr}
-
-/* INLINE does not support REPEATED fields. */
-
 /* Allocated fields carry the size of the actual data, not the pointer */
 #define PB_REQUIRED_POINTER(tag, st, m, fd, ltype, ptr) \
     {tag, PB_ATYPE_POINTER | PB_HTYPE_REQUIRED | ltype, \
@@ -493,31 +480,30 @@ struct pb_extension_s {
 #define PB_OPTEXT_POINTER(tag, st, m, fd, ltype, ptr) \
     PB_OPTIONAL_POINTER(tag, st, m, fd, ltype, ptr)
 
-/* INLINE does not support OPTEXT. */
-
 #define PB_OPTEXT_CALLBACK(tag, st, m, fd, ltype, ptr) \
     PB_OPTIONAL_CALLBACK(tag, st, m, fd, ltype, ptr)
 
 /* The mapping from protobuf types to LTYPEs is done using these macros. */
-#define PB_LTYPE_MAP_BOOL       PB_LTYPE_VARINT
-#define PB_LTYPE_MAP_BYTES      PB_LTYPE_BYTES
-#define PB_LTYPE_MAP_DOUBLE     PB_LTYPE_FIXED64
-#define PB_LTYPE_MAP_ENUM       PB_LTYPE_VARINT
-#define PB_LTYPE_MAP_UENUM      PB_LTYPE_UVARINT
-#define PB_LTYPE_MAP_FIXED32    PB_LTYPE_FIXED32
-#define PB_LTYPE_MAP_FIXED64    PB_LTYPE_FIXED64
-#define PB_LTYPE_MAP_FLOAT      PB_LTYPE_FIXED32
-#define PB_LTYPE_MAP_INT32      PB_LTYPE_VARINT
-#define PB_LTYPE_MAP_INT64      PB_LTYPE_VARINT
-#define PB_LTYPE_MAP_MESSAGE    PB_LTYPE_SUBMESSAGE
-#define PB_LTYPE_MAP_SFIXED32   PB_LTYPE_FIXED32
-#define PB_LTYPE_MAP_SFIXED64   PB_LTYPE_FIXED64
-#define PB_LTYPE_MAP_SINT32     PB_LTYPE_SVARINT
-#define PB_LTYPE_MAP_SINT64     PB_LTYPE_SVARINT
-#define PB_LTYPE_MAP_STRING     PB_LTYPE_STRING
-#define PB_LTYPE_MAP_UINT32     PB_LTYPE_UVARINT
-#define PB_LTYPE_MAP_UINT64     PB_LTYPE_UVARINT
-#define PB_LTYPE_MAP_EXTENSION  PB_LTYPE_EXTENSION
+#define PB_LTYPE_MAP_BOOL               PB_LTYPE_VARINT
+#define PB_LTYPE_MAP_BYTES              PB_LTYPE_BYTES
+#define PB_LTYPE_MAP_DOUBLE             PB_LTYPE_FIXED64
+#define PB_LTYPE_MAP_ENUM               PB_LTYPE_VARINT
+#define PB_LTYPE_MAP_UENUM              PB_LTYPE_UVARINT
+#define PB_LTYPE_MAP_FIXED32            PB_LTYPE_FIXED32
+#define PB_LTYPE_MAP_FIXED64            PB_LTYPE_FIXED64
+#define PB_LTYPE_MAP_FLOAT              PB_LTYPE_FIXED32
+#define PB_LTYPE_MAP_INT32              PB_LTYPE_VARINT
+#define PB_LTYPE_MAP_INT64              PB_LTYPE_VARINT
+#define PB_LTYPE_MAP_MESSAGE            PB_LTYPE_SUBMESSAGE
+#define PB_LTYPE_MAP_SFIXED32           PB_LTYPE_FIXED32
+#define PB_LTYPE_MAP_SFIXED64           PB_LTYPE_FIXED64
+#define PB_LTYPE_MAP_SINT32             PB_LTYPE_SVARINT
+#define PB_LTYPE_MAP_SINT64             PB_LTYPE_SVARINT
+#define PB_LTYPE_MAP_STRING             PB_LTYPE_STRING
+#define PB_LTYPE_MAP_UINT32             PB_LTYPE_UVARINT
+#define PB_LTYPE_MAP_UINT64             PB_LTYPE_UVARINT
+#define PB_LTYPE_MAP_EXTENSION          PB_LTYPE_EXTENSION
+#define PB_LTYPE_MAP_FIXED_LENGTH_BYTES PB_LTYPE_FIXED_LENGTH_BYTES
 
 /* This is the actual macro used in field descriptions.
  * It takes these arguments:
@@ -526,7 +512,7 @@ struct pb_extension_s {
  *                 FLOAT, INT32, INT64, MESSAGE, SFIXED32, SFIXED64
  *                 SINT32, SINT64, STRING, UINT32, UINT64 or EXTENSION
  * - Field rules:  REQUIRED, OPTIONAL or REPEATED
- * - Allocation:   STATIC, INLINE, or CALLBACK
+ * - Allocation:   STATIC, CALLBACK or POINTER
  * - Placement: FIRST or OTHER, depending on if this is the first field in structure.
  * - Message name
  * - Field name
@@ -552,8 +538,6 @@ struct pb_extension_s {
     fd, pb_delta(st, which_ ## u, u.m), \
     pb_membersize(st, u.m[0]), 0, ptr}
 
-/* INLINE does not support ONEOF. */
-
 #define PB_ONEOF_FIELD(union_name, tag, type, rules, allocation, placement, message, field, prevfield, ptr) \
         PB_ONEOF_ ## allocation(union_name, tag, message, field, \
         PB_DATAOFFSET_ ## placement(message, union_name.field, prevfield), \
diff --git a/pb_decode.c b/pb_decode.c
index 92d0175..a8cd61a 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -42,6 +42,7 @@ static bool checkreturn pb_dec_fixed64(pb_istream_t *stream, const pb_field_t *f
 static bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest);
 static bool checkreturn pb_dec_string(pb_istream_t *stream, const pb_field_t *field, void *dest);
 static bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t *field, void *dest);
+static bool checkreturn pb_dec_fixed_length_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest);
 static bool checkreturn pb_skip_varint(pb_istream_t *stream);
 static bool checkreturn pb_skip_string(pb_istream_t *stream);
 
@@ -65,7 +66,7 @@ static const pb_decoder_t PB_DECODERS[PB_LTYPES_COUNT] = {
     &pb_dec_string,
     &pb_dec_submessage,
     NULL, /* extensions */
-    &pb_dec_bytes /* PB_LTYPE_FIXED_LENGTH_BYTES */
+    &pb_dec_fixed_length_bytes
 };
 
 /*******************************
@@ -1286,12 +1287,6 @@ static bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *fie
     }
     else
     {
-        if (PB_LTYPE(field->type) == PB_LTYPE_FIXED_LENGTH_BYTES) {
-            if (size != field->data_size)
-                PB_RETURN_ERROR(stream, "incorrect inline bytes size");
-            return pb_read(stream, (pb_byte_t*)dest, field->data_size);
-        }
-
         if (alloc_size > field->data_size)
             PB_RETURN_ERROR(stream, "bytes overflow");
         bdest = (pb_bytes_array_t*)dest;
@@ -1359,3 +1354,26 @@ static bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t
         return false;
     return status;
 }
+
+static bool checkreturn pb_dec_fixed_length_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest)
+{
+    uint32_t size;
+
+    if (!pb_decode_varint32(stream, &size))
+        return false;
+
+    if (size > PB_SIZE_MAX)
+        PB_RETURN_ERROR(stream, "bytes overflow");
+
+    if (size == 0)
+    {
+        /* As a special case, treat empty bytes string as all zeros for fixed_length_bytes. */
+        memset(dest, 0, field->data_size);
+        return true;
+    }
+
+    if (size != field->data_size)
+        PB_RETURN_ERROR(stream, "incorrect fixed length bytes size");
+
+    return pb_read(stream, (pb_byte_t*)dest, field->data_size);
+}
diff --git a/pb_encode.c b/pb_encode.c
index cafe853..cd731dc 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -35,6 +35,7 @@ static bool checkreturn pb_enc_fixed64(pb_ostream_t *stream, const pb_field_t *f
 static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 static bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 static bool checkreturn pb_enc_submessage(pb_ostream_t *stream, const pb_field_t *field, const void *src);
+static bool checkreturn pb_enc_fixed_length_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 
 /* --- Function pointers to field encoders ---
  * Order in the array must match pb_action_t LTYPE numbering.
@@ -50,7 +51,7 @@ static const pb_encoder_t PB_ENCODERS[PB_LTYPES_COUNT] = {
     &pb_enc_string,
     &pb_enc_submessage,
     NULL, /* extensions */
-    &pb_enc_bytes /* PB_LTYPE_FIXED_LENGTH_BYTES */
+    &pb_enc_fixed_length_bytes
 };
 
 /*******************************
@@ -694,9 +695,6 @@ static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *fie
 {
     const pb_bytes_array_t *bytes = NULL;
 
-    if (PB_LTYPE(field->type) == PB_LTYPE_FIXED_LENGTH_BYTES)
-        return pb_encode_string(stream, (const pb_byte_t*)src, field->data_size);
-
     bytes = (const pb_bytes_array_t*)src;
     
     if (src == NULL)
@@ -748,3 +746,8 @@ static bool checkreturn pb_enc_submessage(pb_ostream_t *stream, const pb_field_t
     return pb_encode_submessage(stream, (const pb_field_t*)field->ptr, src);
 }
 
+static bool checkreturn pb_enc_fixed_length_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src)
+{
+    return pb_encode_string(stream, (const pb_byte_t*)src, field->data_size);
+}
+