UA_SecureConversationMessageHeader erroneous?

[open62541]

UA_SecureConversationMessageHeader erroneous?

[FlorianPalm]

I used the generator to create the functions and modified them later (enc,dec,calcSize). I encode the MessageType into a byte array and decode it by casting 3 Bytes to an UA_Int32 so I can use it in switch/case structures directly. Do you know any simpler solution? I am not really satisfied with that solution...

[uleon]

@FlorianPalm ok, just wanted to know if these functions will be generated once more. From your answer I learn that this is not the case, so I can modify directly in the code, right?

[FlorianPalm]

yes

[uleon]

ok, then we only need to decide if we want to do the encoding/decoding of the messageType in the encoders/decoders. You'll find a messy solution in commit 8da17e3. Personally I'd propose to have the messageType as a part of the struct, so it should decode it itself, eg. by calling TL_getPackageType, shouldn't it?

[FlorianPalm]

I do not like the function TL_getPacketType, it is too long and does too many comparisions. If the header structure holds the MessageType in an UA_Int32 instead of Byte[3] and we define an enum with the corresponding values it would just be a casting/shifting operation.

enum packetType
{
    UA_MESSAGETYPE_HEL = 0x48454C, // H E L
    UA_MESSAGETYPE_ACK = 0x41434B, // A C k
    UA_MESSAGETYPE_ERR = 0x455151, // E R R
    UA_MESSAGETYPE_OPN = 0x4F504E, // O P N
    UA_MESSAGETYPE_MSG = 0x4D5347, // M S G
    UA_MESSAGETYPE_CLO = 0x434C4F  // C L O
};

the decoding function would look like that

UA_Int32 UA_OPCUATcpMessageHeader_decode(char const * src, UA_Int32* pos, UA_OPCUATcpMessageHeader* dst) {
    UA_Int32 retval = UA_SUCCESS;
    UA_Byte tmpBuf[3];
    tmpBuf[3] = 0;
    retval |= UA_Byte_decode(src,pos,&(tmpBuf[0]));
    retval |= UA_Byte_decode(src,pos,&(tmpBuf[1]));
    retval |= UA_Byte_decode(src,pos,&(tmpBuf[2]));
    dst->messageType = (UA_Int32)(tmpBuf[0]<<16) + (UA_Int32)(tmpBuf[1]<<8) + (UA_Int32)(tmpBuf[2]);

    retval |= UA_Byte_decode(src,pos,&(dst->isFinal));
    retval |= UA_UInt32_decode(src,pos,&(dst->messageSize));
    return retval;
}

So the structure can still hold the MessageType as UA_Int32

[uleon]

Perfect! To encourage reuse I'd like to propose introducing a new type UA_MessageType - storage type UA_Int32, size 4; encoding/decoding UA_Byte[3], size 3.

FlorianPalm [email protected] schrieb:

I do not like the function TL_getPacketType, it is too long and does
too many comparisions. If the header structure holds the MessageType in
an UA_Int32 instead of Byte[3] and we define an enum with the
corresponding values it would just be a casting/shifting operation.

enum packetType
{
  UA_MESSAGETYPE_HEL = 0x48454C, // H E L
  UA_MESSAGETYPE_ACK = 0x41434B, // A C k
  UA_MESSAGETYPE_ERR = 0x455151, // E R R
  UA_MESSAGETYPE_OPN = 0x4F504E, // O P N
  UA_MESSAGETYPE_MSG = 0x4D5347, // M S G
  UA_MESSAGETYPE_CLO = 0x434C4F  // C L O
};

the decoding function would look like that

UA_Int32 UA_OPCUATcpMessageHeader_decode(char const * src, UA_Int32*
pos, UA_OPCUATcpMessageHeader* dst) {
  UA_Int32 retval = UA_SUCCESS;
  UA_Byte tmpBuf[3];
  tmpBuf[3] = 0;
  retval |= UA_Byte_decode(src,pos,&(tmpBuf[0]));
  retval |= UA_Byte_decode(src,pos,&(tmpBuf[1]));
  retval |= UA_Byte_decode(src,pos,&(tmpBuf[2]));
  dst->messageType = (UA_Int32)(tmpBuf[0]<<16) +
(UA_Int32)(tmpBuf[1]<<8) + (UA_Int32)(tmpBuf[2]);

  retval |= UA_Byte_decode(src,pos,&(dst->isFinal));
  retval |= UA_UInt32_decode(src,pos,&(dst->messageSize));
  return retval;
}

So the structure can still hold the MessageType as UA_Int32

---

Reply to this email directly or view it on GitHub:
#28 (comment)

Prof. L. Urbas; TU Dresden; [phone]

[Stasik0]

good evening, i was following the discussion and had an idea

please look into the commit 1b36c82

i have implemented a very messy "plugin" system for the code generator, maybe you can just create a custom type for the functions above and use the generator on custom .xml files ;)

[uleon]

@Stasik0 : please reopen new issue for plugin system, @FlorianPalm solved this issue

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.