Encoding/decoding nested messages

[OgniX72]

Hello Petteri and everybody!

I'm having some issues with encoding and decoding a message with nested messages, and I don't know if I'm doing wrong or if there are some incompatibilities between my encoding and the decoder "on the other side".
Let me explain: I'm working with an embedded system on my side (ARM Cortex M4) and on the remote side there's a "complex" system with Google protoc Java code decoder.

I don't have access to the remote system I'm communicating with, and I have to encode the message on a fixed proto message description that follows:
********************************************
message EdcPayload {

    message EdcMetric {

        enum ValueType {
            DOUBLE = 0;
            FLOAT = 1;
            INT64 = 2;
            INT32 = 3;
            BOOL = 4;
            STRING = 5;
            BYTES = 6;
        }

        required string name = 1;
        required ValueType type = 2;
        optional double double_value = 3;
        optional float float_value = 4;
        optional int64 long_value = 5;
        optional int32 int_value = 6;
        optional bool bool_value = 7;
        optional string string_value = 8;
        optional bytes bytes_value = 9;
    }

    message EdcPosition {
        required double latitude = 1;
        required double longitude = 2;
        optional double altitude = 3;
        optional double precision = 4;
        optional double heading = 5;
        optional double speed = 6;
        optional int64 timestamp = 7;
        optional int32 satellites = 8;
        optional int32 status = 9;
    }

    optional int64 timestamp = 1;
    optional EdcPosition position = 2;
    extensions 3 to 4999;
    repeated EdcMetric metric = 5000; // can be zero, so optional
    optional bytes body = 5001;
}
**************************************************************************
As you can see at the bottom part, the Edc Message is made of optional and repeated messages which structure is nested inside main message.
Extensions are possible but not used on my side (hope there are no issues regarding this).


This is the code I'm using for encoding some logs.
All relevant data should stay inside the EdcMetric, and fields are passed through a structure pointer (field name and log string):
*******************************************************************************
ReturnEnum GPB_enLog(char* log, unsigned char *enc_buffer, unsigned int *msg_len)
{
ReturnEnum retState = RET_BUSY;
unsigned int buffer_size = MAX_PACKET_LEN;
char error_msg[64];
bool status;
LogFieldsStruct log_fields;

/* declare our messages */
EdcPayload log_msg = { '\0' };

    /* create a stream that will write to our buffer */
    pb_ostream_t tx_stream = pb_ostream_from_buffer(enc_buffer, buffer_size);

    strcpy(log_fields.field_name, "log");  // metric name
    strcpy(log_fields.field_value, log);  // log string
   
    /* fill in data... */
    log_msg.has_timestamp = false;
    log_msg.metric.funcs.encode = &string_field_callback;
    log_msg.metric.arg = &log_fields;  // 2 array structure with metric name ('log') and its value (the log string itself)
    log_msg.has_position = false;
    log_msg.has_body = false;
   
    /* encode the message */
    status = pb_encode(&tx_stream, EdcPayload_fields, &log_msg);
    *msg_len = tx_stream.bytes_written;
   
    /* then just check for any errors.. */
    if (!status)
    {
        memcpy(error_msg, (PB_GET_ERROR(&tx_stream)), 64);
        retState = RET_ERROR;
    }
    else
    {
        retState = RET_OK;
    }

return retState;
}
***********************************************************

Here's the callback function:
*******************************************************
bool string_field_callback(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
{
EdcPayload_EdcMetric EDC_Metric = { '\0' };
LogFieldsStruct* log_fields = (LogFieldsStruct*) *arg;

    /* Fill data in structure */
    strcpy(EDC_Metric.name, log_fields->field_name);

    EDC_Metric.type = EdcPayload_EdcMetric_ValueType_STRING;  // it's a string
   
    EDC_Metric.has_double_value = false;  // because it's optional
    EDC_Metric.has_float_value = false;  // because it's optional
    EDC_Metric.has_long_value = false;  // because it's optional
    EDC_Metric.has_int_value = false;  // because it's optional
    EDC_Metric.has_bool_value = false;  // because it's optional
    EDC_Metric.has_bytes_value = false;  // because it's optional
    EDC_Metric.has_string_value = true;  // because it's optional
    strcpy(EDC_Metric.string_value, log_fields->field_value);
   
    /* This encodes the header for the field, based on the constant info
     * from pb_field_t. */
    if (!pb_encode_tag_for_field(stream, field))
        return false;
   
    /* This encodes the data for the field, based on our structure. */
    if (!pb_encode_submessage(stream, EdcPayload_EdcMetric_fields, &EDC_Metric))
        return false;
  
    return true;
}
***********************************************************

On the other side the behaviour is not constant: sometimes the message is decoded correctly, and I can get the log string, sometimes not.
As I told you I don't have access to the remote system, but I'm sure all message arrives (there's an activity log I can check).
What makes me suspicious regarding my encoding is that the database system on the other side seems not to recognise correctly the log metric, because I can't make propery query on data on the other side, even if I can see the log string (not always).

Am I wrong with encoding?
I'm doing bad things?  :-)


A brief local decoding test lead me to embedded system crash!
Here's the code:
***************************************************************************
ReturnEnum GPB_decLog(unsigned char *buffer2dec, unsigned int msg_len, char* dec_log)
{
ReturnEnum retState = RET_BUSY;
char error_msg[64];
bool status;

/* Allocate space for the decoded message. */
EdcPayload EDC_Payload_dec  = { '\0' };
/* Note: empty initializer list initializes the struct with all-0.
 * This is recommended so that unused callbacks are set to NULL instead
 * of crashing at runtime.
 */
   
/* Create a stream that reads from the buffer. */
pb_istream_t rx_stream = pb_istream_from_buffer(buffer2dec, msg_len);
   
    /* Now we are ready to decode the message. */
    status = pb_decode(&rx_stream, EdcPayload_fields, &EDC_Payload_dec);
   
    /* Check for errors... */
    if (!status)
    {
        memcpy(error_msg, (PB_GET_ERROR(&rx_stream)), 64);
        retState = RET_ERROR;
    }
    else
    {
        EDC_Payload_dec.metric.funcs.decode = &get_string_callback;
        retState = RET_OK;
    }

    return retState;
}
************************************************************


The callback function:
******************************************************************
bool get_string_callback(pb_istream_t *stream, const pb_field_t *field, void **arg)
{
    uint8_t buffer[MAX_PAYLOAD_LEN];
   
    memset(buffer, 0x00, MAX_PAYLOAD_LEN);
   
    /* We could read block-by-block to avoid the large buffer... */
    if (stream->bytes_left > sizeof(buffer) - 1)
        return false;
   
    if (!pb_read(stream, buffer, stream->bytes_left))
        return false;
   
    return true;
}
***********************************************************************

Here I would deubg just stopping after the pb_read() function and check buffer contents.

Sorry for being long, but I'd like to give as much info as possible.

...and thanks for you great work!
      Luca O.

[OgniX72]

One more thing: I'm using nanoPB V0.2.7.

I'll move to the last version, but I'd like to solve these problems first, to prevent adding other issues.

[Petteri Aimonen]

Hi,

> On the other side the behaviour is not constant: sometimes the message is
> decoded correctly, and I can get the log string, sometimes not.
> As I told you I don't have access to the remote system, but I'm sure all
> message arrives (there's an activity log I can check).

You could dump the message data to a file before decoding. It can then
be decoded using "protoc --decode=MessageName myproto.proto" to make
sure what is in the message.

> A brief local decoding test lead me to embedded system crash!

> uint8_t buffer[MAX_PAYLOAD_LEN];

How large is MAX_PAYLOAD_LEN? How large is your allocated stack?

I'm suspecting you might be having a stack overflow due to allocating
large buffers on stack. One way is to make them static, if possible.
Perhaps there is a similar problem in your actual application?

--
Petteri

[OgniX72]

On Tuesday, September 16, 2014 11:12:51 AM UTC+2, Petteri Aimonen wrote:

> Hi,

Thanks for the very fast reply.

 

> You could dump the message data to a file before decoding. It can then
> be decoded using "protoc --decode=MessageName myproto.proto" to make
> sure what is in the message.

Well, I can stop with debugger and see memory contents...
Yes...I can use the SDCard functions to save data inside of it!

 

>> A brief local decoding test lead me to embedded system crash!
>>     uint8_t buffer[MAX_PAYLOAD_LEN];

> How large is MAX_PAYLOAD_LEN? How large is your allocated stack?

MAX_PAYLOAD_LEN is 512 bytes, while my stack is  2048 bytes.

 

> I'm suspecting you might be having a stack overflow due to allocating
> large buffers on stack. One way is to make them static, if possible.
> Perhaps there is a similar problem in your actual application?

I also thought about this, but I'm not clear about memory allocation needed by encoded message.

Here is my .option file for encoding:
*******************************************
EDCPayload.EDCMetric.name            max_size:32
EDCPayload.EDCMetric.string_value    max_size:200
EDCPayload.EDCMetric.bytes_value    max_size:128
EDCPayload.body                        max_size:128
***************************************************************
My main interest now is the string_value field inside EdcMetric (where I put log data).

Thanks!
  Luca

 

--
> Petteri

 

[Petteri Aimonen]

Hi,

> I also thought about this, but I'm not clear about memory allocation
> needed by encoded message.

Most of the stack memory will be allocated by your code, so you can just
sum together the sizeof() of all your stack variables to get an
estimate.

Many embedded toolchains also have a way to compute stack usage. For
GCC, you can use the checkstack.pl from Linux kernel scripts.

--
Petteri