pw_protobuf: Don't write empty nested messages
When using the message-struct based API, do not write out a zero-sized
field for a nested struct that has all default fields.
Change-Id: I56f40ebf710b776371693e4ce2b27312244f2c6c
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/97900
Reviewed-by: Armando Montanez <amontanez@google.com>
Commit-Queue: Scott James Remnant <keybuk@google.com>
diff --git a/pw_protobuf/codegen_message_test.cc b/pw_protobuf/codegen_message_test.cc
index 4fee12c..de6111a 100644
--- a/pw_protobuf/codegen_message_test.cc
+++ b/pw_protobuf/codegen_message_test.cc
@@ -1286,12 +1286,6 @@
// clang-format off
constexpr uint8_t expected_proto[] = {
- // pigweed.pigweed (empty)
- 0x3a, 0x00,
- // pigweed.proto
- 0x4a, 0x02,
- // pigweed.proto.meta (empty)
- 0x2a, 0x00,
// pigweed.bytes (default)
0x5a, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
};
@@ -1476,12 +1470,6 @@
// clang-format off
constexpr uint8_t expected_proto[] = {
- // pigweed.pigweed (empty)
- 0x3a, 0x00,
- // pigweed.proto (default)
- 0x4a, 0x02,
- // pigweed.proto.meta (empty)
- 0x2a, 0x00,
// pigweed.bytes (default)
0x5a, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
// pigweed.description
@@ -1520,12 +1508,6 @@
// clang-format off
constexpr uint8_t expected_proto[] = {
- // pigweed.pigweed (empty)
- 0x3a, 0x00,
- // pigweed.proto (default)
- 0x4a, 0x02,
- // pigweed.proto.meta (empty)
- 0x2a, 0x00,
// pigweed.bytes (default)
0x5a, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
// pigweed.special_property
@@ -1687,12 +1669,6 @@
0x0a, 0x04, 'c', 'h', 'i', 'p',
// pigweed.device_info.attributes[1].value
0x12, 0x08, 'l', 'e', 'f', 't', '-', 's', 'o', 'c',
- // pigweed.pigweed (empty)
- 0x3a, 0x00,
- // pigweed.proto (default)
- 0x4a, 0x02,
- // pigweed.proto.meta (empty)
- 0x2a, 0x00,
// pigweed.bytes (default)
0x5a, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
};
diff --git a/pw_protobuf/encoder.cc b/pw_protobuf/encoder.cc
index e41e934..b367766 100644
--- a/pw_protobuf/encoder.cc
+++ b/pw_protobuf/encoder.cc
@@ -35,7 +35,8 @@
namespace pw::protobuf {
-StreamEncoder StreamEncoder::GetNestedEncoder(uint32_t field_number) {
+StreamEncoder StreamEncoder::GetNestedEncoder(uint32_t field_number,
+ bool write_when_empty) {
PW_CHECK(!nested_encoder_open());
PW_CHECK(ValidFieldNumber(field_number));
@@ -62,7 +63,7 @@
} else {
nested_buffer = ByteSpan();
}
- return StreamEncoder(*this, nested_buffer);
+ return StreamEncoder(*this, nested_buffer, write_when_empty);
}
StreamEncoder::~StreamEncoder() {
@@ -109,6 +110,10 @@
return;
}
+ if (!nested.memory_writer_.bytes_written() && !nested.write_when_empty_) {
+ return;
+ }
+
status_ = WriteLengthDelimitedField(temp_field_number,
nested.memory_writer_.WrittenData());
}
@@ -511,7 +516,8 @@
// Nested Message. Struct member is an embedded struct for the
// nested field. Obtain a nested encoder and recursively call Write()
// using the fields table pointer from this field.
- auto nested_encoder = GetNestedEncoder(field.field_number());
+ auto nested_encoder = GetNestedEncoder(field.field_number(),
+ /*write_when_empty=*/false);
PW_TRY(nested_encoder.Write(values, *field.nested_message_fields()));
} else if (field.is_fixed_size()) {
// Fixed-length bytes field. Struct member is a std::array<std::byte>.
diff --git a/pw_protobuf/public/pw_protobuf/encoder.h b/pw_protobuf/public/pw_protobuf/encoder.h
index c05c9db..fc724fb 100644
--- a/pw_protobuf/public/pw_protobuf/encoder.h
+++ b/pw_protobuf/public/pw_protobuf/encoder.h
@@ -118,6 +118,7 @@
// provide a zero-length scratch buffer.
constexpr StreamEncoder(stream::Writer& writer, ByteSpan scratch_buffer)
: status_(OkStatus()),
+ write_when_empty_(true),
parent_(nullptr),
nested_field_number_(0),
memory_writer_(scratch_buffer),
@@ -154,7 +155,9 @@
//
// Postcondition: Until the nested child encoder has been destroyed, this
// encoder cannot be used.
- StreamEncoder GetNestedEncoder(uint32_t field_number);
+ StreamEncoder GetNestedEncoder(uint32_t field_number) {
+ return GetNestedEncoder(field_number, /*write_when_empty=*/true);
+ }
// Returns the current encoder's status.
//
@@ -628,6 +631,7 @@
// acts like a parent encoder with an active child encoder.
constexpr StreamEncoder(StreamEncoder&& other)
: status_(other.status_),
+ write_when_empty_(true),
parent_(other.parent_),
nested_field_number_(other.nested_field_number_),
memory_writer_(std::move(other.memory_writer_)),
@@ -649,12 +653,21 @@
Status Write(std::span<const std::byte> message,
std::span<const MessageField> table);
+ // Protected method to create a nested encoder, specifying whether the field
+ // should be written when no fields were added to the nested encoder. Not
+ // part of the public API since callers can simply not create a nested encoder
+ // in those situations.
+ StreamEncoder GetNestedEncoder(uint32_t field_number, bool write_when_empty);
+
private:
friend class MemoryEncoder;
- constexpr StreamEncoder(StreamEncoder& parent, ByteSpan scratch_buffer)
+ constexpr StreamEncoder(StreamEncoder& parent,
+ ByteSpan scratch_buffer,
+ bool write_when_empty = true)
: status_(scratch_buffer.empty() ? Status::ResourceExhausted()
: OkStatus()),
+ write_when_empty_(write_when_empty),
parent_(&parent),
nested_field_number_(0),
memory_writer_(scratch_buffer),
@@ -769,6 +782,10 @@
// encoder enters an error state.
Status status_;
+ // Checked by the parent when the nested encoder is closed, and if no bytes
+ // were written, the field is not written.
+ bool write_when_empty_;
+
// If this is a nested encoder, this points to the encoder that created it.
// For user-created MemoryEncoders, parent_ points to this object as an
// optimization for the MemoryEncoder and nested encoders to use the same
