Embedded-AMS / EmbeddedProto

Embedded Proto is a C++ Protocol Buffers implementation specifically suitable for microcontrollers. It is small, reliable and easy to use.
https://embeddedproto.com
193 stars 35 forks source link

EmbeddedProto v3.5.0 - build warnings using gcc #77

Open eS-Ha-79 opened 1 week ago

eS-Ha-79 commented 1 week ago

Hi,

i'm using gcc (13.2.1 but shouldn't matter) with additional warnings enabled: -Wall -Wextra -Wdouble-promotion

float fields

With messages containing float fields, the following warning(s) are produced in the serialize() function:

warning: implicit conversion from 'float' to 'double' to match other operand of binary expression [-Wdouble-promotion]
  910 |       if((0.0 != minimum_.get()) && (::EmbeddedProto::Error::NO_ERRORS == return_value))

I guess changing "0.0" to "0.0f" in Field.py -> class FieldBasic(Field) -> type_to_default_value (lines 159+) for the dictionary entry FieldDescriptorProto.TYPE_FLOAT should be enough.

empty messages

Using empty messages (like google.protobuf.Empty) there are some unused parameter warnings (generated constructors, assignment operators and serialize() ). Personally, i added [[maybe_unused]] attributes in TypeDefMsg.h but there might be better ways, since this is CPP17+:

BartHertog commented 1 week ago

Hello @eS-Ha-79,

Thank you for writing in with these issues! Your input helps improve embedded proto.

float fields: This has been added to the backlog. It is easy to add and will indeed clarify the meaning of the code.

empty messages: CPP17, at the moment, is not the default. There are still customers with projects on CPP14. We may increase the requirement to 17 with the next major release of EmbeddedProto. If that is the case we will include your suggestion.

On a side note, may I ask what your use case is for empty messages?

Best regards,

Bart

eS-Ha-79 commented 1 week ago

Hey Bart,

was playing around with some sort of lightweight/naive (whatever one may call it ;-) ) RPC and used empty messages for commands w/o parameters, e.g.

message request {
  int32 request_number = 1;
  oneof group {
    float set_motor_speed = 2;
    google.protobuf.Empty get_motor_speed = 3;
  }
}

and

message response {
  int32 to_request_number = 1;
  oneof group {

     float current_speed = 3;
  }
  optional int32 error = 4;
}

Best regards, Soeren

BartHertog commented 1 week ago

Hi, That is an interesting way to think of empty messages. Using the oneof with the empty messages is interesting. I understand that you than want the message to be very light.

I was used to using enums for commands thus far.

Best regards,

Bart

eS-Ha-79 commented 1 week ago

Hey,

yes keeping it small was the idea.

Regarding the floats 0.0 vs 0.0f: gcc and clang both warn about the "double promotion" - BUT gcc produces the same output for both versions whilst clangs output differs... See: compiler explorer

Best regards, Soeren