Set proto2 enum fields to first value when no default is given, even if nonzero (#532) (#539)
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index b572940..8eb351f 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -1231,7 +1231,6 @@
return b''
optional_only = copy.deepcopy(self.desc)
- enums = {}
# Remove fields without default values
# The iteration is done in reverse order to avoid remove() messing up iteration.
@@ -1241,18 +1240,28 @@
if parsed_field is None or parsed_field.allocation != 'STATIC':
optional_only.field.remove(field)
elif (field.label == FieldD.LABEL_REPEATED or
- field.type == FieldD.TYPE_MESSAGE or
- not field.HasField('default_value')):
+ field.type == FieldD.TYPE_MESSAGE):
optional_only.field.remove(field)
elif hasattr(field, 'oneof_index') and field.HasField('oneof_index'):
optional_only.field.remove(field)
elif field.type == FieldD.TYPE_ENUM:
# The partial descriptor doesn't include the enum type
# so we fake it with int64.
- enums[field.name] = (names_from_type_name(field.type_name), field.default_value)
- field.type = FieldD.TYPE_INT64
- field.ClearField(str('default_value'))
- field.ClearField(str('type_name'))
+ enumname = names_from_type_name(field.type_name)
+ enumtype = dependencies[str(enumname)]
+ if field.HasField('default_value'):
+ defvals = [v for n,v in enumtype.values if n.parts[-1] == field.default_value]
+ else:
+ # If no default is specified, the default is the first value.
+ defvals = [v for n,v in enumtype.values]
+ if defvals and defvals[0] != 0:
+ field.type = FieldD.TYPE_INT64
+ field.default_value = str(defvals[0])
+ field.ClearField(str('type_name'))
+ else:
+ optional_only.field.remove(field)
+ elif not field.HasField('default_value'):
+ optional_only.field.remove(field)
if len(optional_only.field) == 0:
return b''
@@ -1273,14 +1282,6 @@
setattr(msg, field.name, float(field.default_value))
elif field.type == FieldD.TYPE_BOOL:
setattr(msg, field.name, field.default_value == 'true')
- elif field.name in enums:
- # Lookup the enum default value
- enumname = enums[field.name][0]
- defval = enums[field.name][1]
- enumtype = dependencies[str(enumname)]
- defvals = [v for n,v in enumtype.values if n.parts[-1] == defval]
- if defvals:
- setattr(msg, field.name, defvals[0])
else:
setattr(msg, field.name, int(field.default_value))
diff --git a/tests/alltypes/alltypes.proto b/tests/alltypes/alltypes.proto
index df15d40..317b783 100644
--- a/tests/alltypes/alltypes.proto
+++ b/tests/alltypes/alltypes.proto
@@ -1,6 +1,12 @@
syntax = "proto2";
// package name placeholder
+enum NonZeroBasedEnum {
+ Two = 2;
+ Three = 3;
+ Four = 4;
+}
+
message SubMessage {
required string substuff1 = 1 [default = "1"];
required int32 substuff2 = 2 [default = 2];
@@ -127,6 +133,8 @@
SubMessage oneof_msg1 = 60;
EmptyMessage oneof_msg2 = 61;
}
+
+ optional NonZeroBasedEnum opt_non_zero_based_enum = 62;
// Check support for custom integer sizes
required IntSizes req_intsizes = 96;
diff --git a/tests/alltypes/decode_alltypes.c b/tests/alltypes/decode_alltypes.c
index 1c49080..3b92614 100644
--- a/tests/alltypes/decode_alltypes.c
+++ b/tests/alltypes/decode_alltypes.c
@@ -158,6 +158,9 @@
TEST(memcmp(alltypes.opt_fbytes, "4059", 4) == 0);
TEST(alltypes.which_oneof == 0);
+
+ TEST(alltypes.has_opt_non_zero_based_enum == false);
+ TEST(alltypes.opt_non_zero_based_enum == NonZeroBasedEnum_Two);
}
else if (mode == 1)
{
@@ -209,6 +212,9 @@
TEST(alltypes.which_oneof == AllTypes_oneof_msg1_tag);
TEST(strcmp(alltypes.oneof.oneof_msg1.substuff1, "4059") == 0);
TEST(alltypes.oneof.oneof_msg1.substuff2 == 4059);
+
+ TEST(alltypes.has_opt_non_zero_based_enum == true);
+ TEST(alltypes.opt_non_zero_based_enum == NonZeroBasedEnum_Three);
}
else if (mode == 2)
{
@@ -285,6 +291,8 @@
TEST(alltypes.has_opt_fbytes == false);
TEST(alltypes.which_oneof == 0);
+
+ TEST(alltypes.has_opt_non_zero_based_enum == false);
}
TEST(alltypes.end == 1099);
diff --git a/tests/alltypes/encode_alltypes.c b/tests/alltypes/encode_alltypes.c
index 00d2615..7f4ad2b 100644
--- a/tests/alltypes/encode_alltypes.c
+++ b/tests/alltypes/encode_alltypes.c
@@ -150,6 +150,9 @@
alltypes.which_oneof = AllTypes_oneof_msg1_tag;
strcpy(alltypes.oneof.oneof_msg1.substuff1, "4059");
alltypes.oneof.oneof_msg1.substuff2 = 4059;
+
+ alltypes.has_opt_non_zero_based_enum = true;
+ alltypes.opt_non_zero_based_enum = NonZeroBasedEnum_Three;
}
alltypes.end = 1099;
diff --git a/tests/alltypes_callback/decode_alltypes_callback.c b/tests/alltypes_callback/decode_alltypes_callback.c
index 8079a04..71da215 100644
--- a/tests/alltypes_callback/decode_alltypes_callback.c
+++ b/tests/alltypes_callback/decode_alltypes_callback.c
@@ -450,6 +450,9 @@
alltypes.oneof_msg1.funcs.decode = &read_submsg;
alltypes.oneof_msg1.arg = &oneof_msg1;
+
+ alltypes.opt_non_zero_based_enum.funcs.decode = &read_varint;
+ alltypes.opt_non_zero_based_enum.arg = (void *)NonZeroBasedEnum_Three;
}
status = pb_decode(stream, AllTypes_fields, &alltypes);
diff --git a/tests/alltypes_callback/encode_alltypes_callback.c b/tests/alltypes_callback/encode_alltypes_callback.c
index 8f00b77..d123bf7 100644
--- a/tests/alltypes_callback/encode_alltypes_callback.c
+++ b/tests/alltypes_callback/encode_alltypes_callback.c
@@ -447,6 +447,9 @@
alltypes.oneof_msg1.funcs.encode = &write_submsg;
alltypes.oneof_msg1.arg = &oneof_msg1;
+
+ alltypes.opt_non_zero_based_enum.funcs.encode = &write_varint;
+ alltypes.opt_non_zero_based_enum.arg = (void *)NonZeroBasedEnum_Three;
}
alltypes.end.funcs.encode = &write_varint;
diff --git a/tests/alltypes_pointer/decode_alltypes_pointer.c b/tests/alltypes_pointer/decode_alltypes_pointer.c
index 5659012..f652b17 100644
--- a/tests/alltypes_pointer/decode_alltypes_pointer.c
+++ b/tests/alltypes_pointer/decode_alltypes_pointer.c
@@ -103,6 +103,8 @@
TEST(alltypes.opt_fbytes == NULL);
TEST(alltypes.which_oneof == 0);
+
+ TEST(alltypes.opt_non_zero_based_enum == NULL);
}
else
{
@@ -134,6 +136,8 @@
TEST(alltypes.which_oneof == AllTypes_oneof_msg1_tag);
TEST(alltypes.oneof.oneof_msg1 && strcmp(alltypes.oneof.oneof_msg1->substuff1, "4059") == 0);
TEST(alltypes.oneof.oneof_msg1->substuff2 && *alltypes.oneof.oneof_msg1->substuff2 == 4059);
+
+ TEST(alltypes.opt_non_zero_based_enum && *alltypes.opt_non_zero_based_enum == NonZeroBasedEnum_Three);
}
TEST(alltypes.req_limits->int32_min && *alltypes.req_limits->int32_min == INT32_MIN);
diff --git a/tests/alltypes_pointer/encode_alltypes_pointer.c b/tests/alltypes_pointer/encode_alltypes_pointer.c
index ed780de..7f4b391 100644
--- a/tests/alltypes_pointer/encode_alltypes_pointer.c
+++ b/tests/alltypes_pointer/encode_alltypes_pointer.c
@@ -91,6 +91,8 @@
static int32_t oneof_substuff = 4059;
SubMessage oneof_msg1 = {"4059", &oneof_substuff};
+ NonZeroBasedEnum opt_non_zero_based_enum = NonZeroBasedEnum_Three;
+
/* Values for the Limits message. */
static int32_t int32_min = INT32_MIN;
static int32_t int32_max = INT32_MAX;
@@ -197,6 +199,8 @@
alltypes.which_oneof = AllTypes_oneof_msg1_tag;
alltypes.oneof.oneof_msg1 = &oneof_msg1;
+
+ alltypes.opt_non_zero_based_enum = &opt_non_zero_based_enum;
}
alltypes.end = &end;
diff --git a/tests/common_unittests/common_unittests.c b/tests/common_unittests/common_unittests.c
index faa8173..fabe67c 100644
--- a/tests/common_unittests/common_unittests.c
+++ b/tests/common_unittests/common_unittests.c
@@ -91,6 +91,8 @@
TEST(iter.required_field_index == 19)
TEST(iter.submessage_index == 7)
+ TEST(pb_field_iter_next(&iter) && iter.tag == 62 && iter.pData == &msg.opt_non_zero_based_enum && iter.pSize == &msg.has_opt_non_zero_based_enum)
+
TEST(pb_field_iter_next(&iter) && iter.tag == 96 && iter.pData == &msg.req_intsizes && !iter.pSize)
TEST(iter.required_field_index == 19)
TEST(iter.submessage_index == 8)
diff --git a/tests/field_size_16/alltypes.proto b/tests/field_size_16/alltypes.proto
index c094421..9191ca0 100644
--- a/tests/field_size_16/alltypes.proto
+++ b/tests/field_size_16/alltypes.proto
@@ -1,5 +1,11 @@
syntax = "proto2";
+enum NonZeroBasedEnum {
+ Two = 2;
+ Three = 3;
+ Four = 4;
+}
+
message SubMessage {
required string substuff1 = 1 [default = "1"];
required int32 substuff2 = 2 [default = 2];
@@ -127,6 +133,8 @@
EmptyMessage oneof_msg2 = 10061;
}
+ optional NonZeroBasedEnum opt_non_zero_based_enum = 10062;
+
// Check support for custom integer sizes
required IntSizes req_intsizes = 96;
diff --git a/tests/field_size_32/alltypes.proto b/tests/field_size_32/alltypes.proto
index 536fc2a..2ae0f1f 100644
--- a/tests/field_size_32/alltypes.proto
+++ b/tests/field_size_32/alltypes.proto
@@ -1,5 +1,11 @@
syntax = "proto2";
+enum NonZeroBasedEnum {
+ Two = 2;
+ Three = 3;
+ Four = 4;
+}
+
message SubMessage {
required string substuff1 = 1 [default = "1"];
required int32 substuff2 = 2 [default = 2];
@@ -127,6 +133,8 @@
EmptyMessage oneof_msg2 = 10061;
}
+ optional NonZeroBasedEnum opt_non_zero_based_enum = 10062;
+
// Check support for custom integer sizes
required IntSizes req_intsizes = 96;
diff --git a/tests/package_name/SConscript b/tests/package_name/SConscript
index 4afc503..071a3e8 100644
--- a/tests/package_name/SConscript
+++ b/tests/package_name/SConscript
@@ -22,7 +22,7 @@
data = open(str(source[0]), 'r').read()
- type_names = ['AllTypes', 'MyEnum', 'HugeEnum']
+ type_names = ['AllTypes', 'MyEnum', 'HugeEnum', 'NonZeroBasedEnum']
for name in type_names:
data = data.replace(name, 'test_package_' + name)
diff --git a/tests/package_name_nanopb/SConscript b/tests/package_name_nanopb/SConscript
index 990602d..9088f12 100644
--- a/tests/package_name_nanopb/SConscript
+++ b/tests/package_name_nanopb/SConscript
@@ -24,7 +24,7 @@
data = open(str(source[0]), 'r').read()
- type_names = ['AllTypes', 'MyEnum', 'HugeEnum']
+ type_names = ['AllTypes', 'MyEnum', 'HugeEnum', 'NonZeroBasedEnum']
for name in type_names:
data = data.replace(name, 'test_package_' + name)
