ApolloAuto/apollo

Not compatible with protobuf3

Closed this issue · 5 comments

Describe the bug
protobuf3 not encode default value

To Reproduce

  1. build cyber(master) on ubuntu 18.04 with protobuf(3.14.0)
  2. write cyber demo
  //talker.cpp
  apollo::cyber::Init(argv[0]);
  // create talker node
  std::shared_ptr<apollo::cyber::Node> talker_node(
      apollo::cyber::CreateNode("talker"));
  // create talker
  auto talker = talker_node->CreateWriter<Chatter>("channel/chatter");
  Rate rate(1.0);
  while (apollo::cyber::OK()) {
    static uint64_t seq = 0;
    auto msg = std::make_shared<apollo::cyber::proto::Chatter>();
    msg->set_timestamp(Time::Now().ToNanosecond());
    msg->set_lidar_timestamp(Time::Now().ToNanosecond());
    msg->set_seq(seq++);
    msg->set_content("Hello, apollo!1234567890aaaaaaaaaa");
    talker->Write(msg);
    AINFO << "talker sent a message!";
    rate.Sleep();
  }
  ////////////////////////////////////////////////////////////////////////////////////////
  //listener.cpp
void MessageCallback(
    const std::shared_ptr<apollo::cyber::proto::Chatter>& msg) {
  AINFO << "Received message seq-> " << msg->seq();
  AINFO << "msgcontent->" << msg->content();
}
  apollo::cyber::Init(argv[0]);
  // create listener node
  auto listener_node = apollo::cyber::CreateNode("listener");
  // create listener
  auto listener =
      listener_node->CreateReader<apollo::cyber::proto::Chatter>(
          "channel/chatter", MessageCallback);
  apollo::cyber::WaitForShutdown();
  1. log:
    W0117 19:51:11.722460 23747 manager.cc:226] !message::ParseFromString(msg_str, &msg) is met.
  2. reason:
    protobuf3 not encode default value, the QOS_PROFILE_DEFAULTin qos_profile_conf.cc write
    with some default value, the func SerializeToString will make a bad string(has some '\0' in string),
    this lead other node can't parse, because the string size different.
  3. solve
    use protobuf2, or fix all the code like this
//qos_profile_conf.cc
//comment if value is default
QosProfile QosProfileConf::CreateQosProfile(
    const QosHistoryPolicy& history, uint32_t depth, uint32_t mps,
    const QosReliabilityPolicy& reliability,
    const QosDurabilityPolicy& durability) {
  QosProfile qos_profile;
  qos_profile.set_history(history);
  qos_profile.set_depth(depth);
  qos_profile.set_mps(mps);
  qos_profile.set_reliability(reliability);
  qos_profile.set_durability(durability);

  return qos_profile;
}

then get the right info

I0117 19:51:05.270557 14301 main.cpp:6] msgcontent->Hello, apollo!1234567890aaaaaaaaaa

Is proto3 currently supported? What I currently know is that only proto2 is supported

syntax = "proto2";            // proto2

package apollo.canbus;

Is proto3 currently supported? What I currently know is that only proto2 is supported

syntax = "proto2";            // proto2

package apollo.canbus;

protobuf3 compatible with syntax = "proto2", I think not full compatible.

The biggest feature difference between proto2 and proto3 is the treatment of default / optional values. Proto3 has no concept of "the default value is 4" (defaults are always zero/nil), and has no concept of explicitly specifying a value that happens to also be the default value (non-zero values are always sent, zeros are never sent). So if your proto2 schema makes use of non-zero defaults, it can be awkward to transition.

proto3 default

The reason is indeed the case, it seems that apollo still only supports proto2

Sorry, I am wrong. The problem is Fast-CDR serialize string as c-string, so the last '\0' will be discarded.
I change the code in underlay_message.cc, it's OK.

void UnderlayMessage::serialize(eprosima::fastcdr::Cdr& scdr) const {
  scdr << m_timestamp;
  scdr << m_seq;
  std::vector<char> data_vector(m_data.size());
  data_vector.assign(m_data.begin(), m_data.end());
  scdr << data_vector;
  scdr << m_datatype;
}

void UnderlayMessage::deserialize(eprosima::fastcdr::Cdr& dcdr) {
  dcdr >> m_timestamp;
  dcdr >> m_seq;
  std::vector<char> data_vector;
  dcdr >> data_vector;
  m_data.assign(data_vector.begin(), data_vector.end());
  dcdr >> m_datatype;
}

Seems to be resolved, we will close this issue. If the problem persists, pls feel free to reopen it or create a new one and refer to it.