WindhoverLabs / juicer

5 stars 0 forks source link

Juicer not parsing unions #31

Open swhart115 opened 1 year ago

swhart115 commented 1 year ago

Hi, We were using juicer to parse NASA cFE message structures, but noticed that it was "missing" some key items. Specifically, the CFE_MSG_Message primary header: https://github.com/nasa/cFE/blob/9ebdf2e901174531541f962b0d39a12a9713f187/modules/msg/option_inc/default_cfe_msg_hdr_pri.h#L92

It occurred to us that it may be missing because this msg is in fact a union and not a struct. We may be off base here, but for using juicer with cFS, as I believe has been done, this was somewhat surprising as this is a key aspect of the architecture.

Is there any further insight on this issue?

Thanks.

lorenzo-gomez-windhover commented 1 year ago

Hi there!

So you are right--juicer does not support unions at the moment. But we do we use it for CFS. Bu there are a few things to note.

We user it for CFS 6.5.0.0 Specifically the union you are referring to, is just a header for messages. Which at least in our version of CFS, and it looks like in the source file you point it out, is just an array of bytes. With that said, what we do with our CFS messages, is something like the following:

typedef struct
{
    uint8                       TlmHeader[CFE_SB_TLM_HDR_SIZE]; /**< \brief cFE Software Bus Telemetry Message Header */
    CFE_ES_HkPacket_Payload_t   Payload;

} CFE_ES_HkPacket_t;

But it looks like in newer versions, it might be slightly different. Most likely unions should be an easy thing to implement.Not promising anything--it is not a high priority item at the moment. And I have to also consult with my team about adding unions to juicer since it might require a fundamental schema change.

Anyway with that said, another thing to understand is that while juicer can be used on its own, it was really written to be used as part of auto-yamcs. I highly recommend following the tutorial that's part of the auto-yamcs documentation to get a better idea of how juicer is really used, specifically in the context of automating a ground system for CFS(or any other flight software). In particular message headers of messages have to go through some special processing...this gets into the details. But if you go through the tutorial, a lot more of this will make sense.

Hope this helps

swhart115 commented 1 year ago

Okay, thank you for the information. We'll take a look at the yamcs tutorials, though our use case is slightly different as we are creating a ROS2 bridge to cFS, and are generating ROS2 msg equivalents from the Juicer output. We were just surprised to not see the primary header, which seemed critical. It makes sense, though, if things have changed with the underlying source code.

we'll probably do a temporary workaround in the short term---if there's a secondary header, we can "manually" add the primary header---though eventually it will be good if things were fully automatable. We'll be on the look out in case you guys get around to addressing the union thing.

Thanks again for the response and clarification.

lorenzo-gomez-windhover commented 1 year ago

So I looked into unions a bit...and you can go ahead and use the branch I pushed and it should give you some more info on the union, but the support is not complete. This will be a future item at some point, I would hope anyway. You guys might be better off doing what you are already doing--adding the headers manually to the sqlite database.

Best of luck :+1:

tmilam commented 1 year ago

Hi, This branch does create the CFE_MSG_Message symbol. This helps our use case, but doesn't completely solve it as there are no fields associated with the symbol. In this particular case we'd like to have the _CCSDS_SpacePackett CCSDS element of the union associated with it, but I'm not sure how to generalize a rule for choosing a field for a union.

lorenzo-gomez-windhover commented 1 year ago

@tmilam

but I'm not sure how to generalize a rule for choosing a field for a union.

You hit the nail on the head. At first I thought...yeah let's just add unions it will be easy. But after talking with my team, we don't know exactly what the right approach might be when it comes to choosing the field of the union. Especially when it comes to protocols like XTCE. The best I can think of is let the user decide via some config file.