Subscribed C messages can't be recorded

[bbercoviciUspace]

It appears that the mechanism by which wrapped C-messages can be recorded is limited to the case where such C messages are self authors. Indeed, the recorder's creation only reads from the payload field of the message structure :

Consider msg_C.h

//! structure definition
typedef struct {{
    Msg2Header header;              //!< message header, zero'd on construction
    {type}Payload payload;		        //!< message copy, zero'd on construction
    {type}Payload *payloadPointer;	    //!< pointer to message
    Msg2Header *headerPointer;      //!< pointer to message header
}} {type}_C;

This definition comes in play when a recorder associated to this C message is created

//! -- Use this to record C messages
    Recorder(void* message, uint64_t timeDiff = 0){
        this->timeInterval = timeDiff;
        Msg2Header msgHeader;
        
        Msg2Header* pt = (Msg2Header *) message;
        messageType* payloadPointer;
        payloadPointer = (messageType *) (++pt);
        this->readMessage = ReadFunctor<messageType>(payloadPointer, &msgHeader);
       
        this->ModelTag = "Rec:";
        Message<messageType> tempMsg;
        std::string msgName = typeid(tempMsg).name();
        this->ModelTag += findMsgName(msgName);
    }

It is my understanding that the pointer payloadPointer on the line preceding this -> readMessage() actually points to the payload field of the message definition structure, and not to the payloadPointer field. This if fine for self-author messages that are written to, since payloadPointer == &payload is true for self-author messages. Recorders work just fine in this case.

However, this is no longer the case for subscribed C messages as their payload field is never written to. So creating a recorder for such a subscribed C message and hoping for the recorder to pick up whatever was written in the upstream message fails : the recorder will always record a zero payload (the content of the message's payload field).

I think a way around would be to replace the call to the ReadFunctor constructor by the following (note the pointer that is passed to the constructor of the ReadFunctor, which once de-referenced should be equal to the payloadPointer field of the message structure)

 Msg2Header* pt = (Msg2Header *) message;
 messageType* payloadPointer;
 payloadPointer = (messageType *) (++pt);
 messageType  ** payloadPointerPointer;
 payloadPointerPointer = (messageType ** )(++ payloadPointer)
 this->readMessage = ReadFunctor<messageType>(*payloadPointerPointer, &msgHeader);

The question as to how the Recorder constructor can detect that it is dealing with a Subscribed C message is still open though.

What are you thoughts about this ?

[schaubh]

Howdy Benjamin, thanks for bringing up this issue. I haven't dug into recorders and read functors in 2+ years and will have to dig into this again. What you discuss above reads correct. If the ReadFunctor reads from the C-msg payload, and does not use the payload pointer, then this would be incorrect behavior. I'll look at this and if your fix works out.

[schaubh]

Howdy Benjamin,

I looked at making your proposed change, but this led to several issues.

1. The C-modules initialize the C-wrapped output messages during the InitializeSimulation() method. If we setup a recorder that saves off the payloadPointer prior to running InitializeSimulation(), then this will point to address 0 and cause a seg fault. I modified the C-module wrapper method to also call SelfInit() which fixed this, but led to other issues.
2. If the C-module call SelfInit() when the module wrapper is created, then modules that had optional output messages started to fail because they were not properly initialized.
3. Also, C++ fswModules create both C and C++ wrapped output messages. The C-wrapped message is only initialized when InitializeSimulation() is called. Thus, any recorders setup on this output message prior to this call will yield another seg fault.

Thus, I'm going to leave the behavior as it has been. If you re-direct a module to write to a stand-alone C-wrapped message, then you need to setup a recorder on the stand along message, not the original C-module output message.

I'm going to update the documentation to make it clear that the .recorder() method on a C-wrapped message is recording the payload, and does not consider if this message has been added as an author to another message.

[bbercoviciUspace]

Hello HP,

Thanks for your prompt response and investigation. The solution I envisioned indeed had a few sides effects.

I think there's an easier fix though.

I've copied/pasted the relevant lines of msg_C.cpp.in.

//! C interface to read to a message 
 {type}Payload {type}_C_read({type}_C *source) {{ 
     if (!source->header.isLinked) {{          BSK_PRINT(MSG_ERROR,"In C input msg, you are trying to read an un-connected message of type {type}."); 
     }} 
     return *source->payloadPointer; 
 }};

I think that adding the following line before return

source -> payload = *source->payloadPointer;

(Or, an equivalent call to memcpy).

will solve the issue of zeroed-out records for subscribed C messages.

Note that the above proposed fix should have no impact on self-author C messages.

Recorded subscribed C messages are typically module input messages, and as such are read in during the module's Update(). One just has to ensure that the module's Update() is called before the recorder's for the recorded payload to indeed be up to date.

Could this do the trick ?

[schaubh]

Mm, interesting idea. In essence, the read command on the message would copy over the target message content into the local message container. Let me try this and see if I run into issues.

[schaubh]

I'm not seeing the expected behavior be adding that line. See attached python test script.
msgTest.py.zip

I use the dummy BSK module, redirect the writing of the module output message to an external message cMsg. Setting up the recorder on the original module output message is still recording zero's.

If you are making a C-msg object the author of another C-msg, then I would simply add the recorder to the target message, not the original message?

[bbercoviciUspace]

Hello HP,

I think there was a misunderstanding on what I was trying to achieve here. Attached is a snippet demonstrating the successful recording of a C-module's input message content.

Here is the template C-message read function I am using

//! C interface to read to a message
{type}Payload {type}_C_read({type}_C *source) {{
    if (!source->header.isLinked) {{
        BSK_PRINT(MSG_ERROR,"In C input msg, you are trying to read an un-connected message of type {type}.");
    }}
    // copy over target of payloadPointer into payload field.
    // this will change nothing for C self-author messages
    // but this makes subscribed C messages recordable
    source -> payload = *source -> payloadPointer;
    return *source->payloadPointer;
}};

If the source -> payload = *source -> payloadPointer line is commented out, then the attached snippet should print the following in the terminal

Recorded Input Data : 
[0. 0. 0.]
[0. 0. 0.]
Recorded Output Data : 
[2. 2. 3.]
[3. 2. 3.]

If the line is uncommented, then the output changes to

Recorded Input Data : 
[1. 2. 3.]
[1. 2. 3.]
Recorded Output Data : 
[2. 2. 3.]
[3. 2. 3.]

which is the expected behaviour.

I'm in the process of recompiling BSK and running the unit tests so that I can make sure that this didn't break anything, but I think this fix addresses the issue I raised.

testRecordInputMessages.zip

[schaubh]

Happy New Year Benjamin,

thanks for the sample script. I was miss-understanding what you were trying to do. I agree with your solution to allow a recorder module to be added to a BSK modules C-wrapped input message. I added a unit test for this behavior and I'm pushing this branch to be reviewed in the next release.