Customization of packet_config_COMETINTERCEPTOR.py File

[pasetti]

We are unable to correctly decode packets in a tmpool and we suspect that the problem lies in an incorrect customization of file packet_config_COMETINTERCEPTOR.py. Here is what happens:

We load the tmpool and the TM data appear in the pool viewer. All the header information (as it is displayed in the panel at the top left) is correct. In particular, the values of CTIME and FTIME are correct. However, we have two problems:

(1) The value of 'Time" in the main panel remains blank (see screenshot below). (2) The first byte of data of every packet is simply lost (this makes decoding the data in a TM packet impossible)

Note that we have no error messages in the log window and that we are reasonably confident that the content of the TM Packet is correct.

In file packet_config_COMETINTERCEPTOR.py we have checked the constants which define the packet headers but we have not modified any of the functions in the files.

Are we overlooking something obvious? We are simply lost ...

[pasetti]

After some debugging, we have found the likely reason for our problem:

In the packet_config_COMETINTERCEPTOR.py file, we define data structures PRIMARY_HEADER and TM_SECONDARY_HEADER as sequence of bit fields. The sum of the sizes of their bit fields is 160 bits or 20 bytes. This is correct because it matches the length of the header data in TM packets.

In the same file, the script computes the size of this data structure as follows:

    class TMHeaderBits(ctypes.BigEndianStructure):
        _pack_ = 1
        _fields_ = [(label, ctype, bits) for label, ctype, bits in PRIMARY_HEADER + TM_SECONDARY_HEADER]

    TM_HEADER_LEN = ctypes.sizeof(TMHeaderBits)

I have printed out the value of TM_HEADER_LEN. I would expect it to be 20 bytes but it is in fact 21 bytes. This is presumably due to packing/alignment problems but it likely explains what we see.

One final note: we defined our file packet_config_COMETINTERCEPTOR.py by starting from the analogous file for ARIEL. One difference between us and ARIEL is that in ARIEL the fine time (FTIME) had 16 bits whereas in our case the fine time has 24 bits. If there is some word-alignment requirement somewhere, we get one extra byte.

Assuming that our diagnosis of the problem is correct, how do we fix this issue?

[pasetti]

I think I found a solution for the alignment problem in TM_SECONDARY_HEADER: I broke up the FTIME part of the header (3 bytes) into two fields of which one has 2 bytes and the other has one byte. With this fix, the "missing byte" in the telemetry is now visible. Here is the relevant code before the change:

TM_SECONDARY_HEADER = [
    ("PUS_VERSION", ctypes.c_uint8, 4),
    ("SCTRS", ctypes.c_uint8, 4),
    ("SERV_TYPE", ctypes.c_uint8, 8),
    ("SERV_SUB_TYPE", ctypes.c_uint8, 8),
    ("MTC", ctypes.c_uint16, 16),
    ("DEST_ID", ctypes.c_uint16, 16),
    ("CTIME", ctypes.c_uint32, 32),
    ("FTIME", ctypes.c_uint32, 24)
]

And here is the same code after the change:

TM_SECONDARY_HEADER = [
    ("PUS_VERSION", ctypes.c_uint8, 4),
    ("SCTRS", ctypes.c_uint8, 4),
    ("SERV_TYPE", ctypes.c_uint8, 8),
    ("SERV_SUB_TYPE", ctypes.c_uint8, 8),
    ("MTC", ctypes.c_uint16, 16),
    ("DEST_ID", ctypes.c_uint16, 16),
    ("CTIME", ctypes.c_uint32, 32),
    ("FTIME", ctypes.c_uint16, 16),                # Modified Field
    ("FTIMEX", ctypes.c_uint8, 8)                   # New Field
]

I suppose that I will now need to update the functions which decode the time information. Any help on how this should be done is appreciated ...

I am leaving this ticket open because I believe that the alignment issue is a borderline bug: users may have an alignment problem but they have no way of realizing it. Maybe, there should be a piece of code which compares the value of TM_HEADER_LEN with the expected header size as it is given by the sum of the sizes of the header fields. If there are discrepancies, they should be reported to the user so that he can re-arrange the definition of the header fields to remove the error.

Finally, it is noted that we had the problem in the TM header but exactly the same problem can also arise in the TC header.

[mmecina]

With the current ctypes struct scheme, I did not manage to find a way to get a structure packed and aligned to 20 bytes without changing header parameters. I will have to think of a more general solution how to handle arbitrary header definitions.

Alternatively to the above solution with splitting the FTIME, one fix might be to just manually set the TM_HEADER_LEN = 20. This will crop the TMHeader object to 20 bytes by excluding the padding byte and should also solve the problem with the missing byte in the payload data. I have not fully tested such a manual 'override' though and this might cause issues elsewhere.

The problem with splitting the FTIME is that in ccs_function_lib.py there are some functions that rely on the FTIME property. Also, as this can potentially affect other header parameters as well, different header definitions in other projects might necessitate yet more adaptations. Ideally, the interface to the TM/TCHeader classes would remain compatible and any changes should be implemented 'under the hood' in the packet_config_*.py module.

In any case, I agree with the proposal to add a check of the actual with the expected header length for both TM/TC definitions.

[pasetti]

The problem with splitting the FTIME is that in ccs_function_lib.py there are some functions that rely on the FTIME property.

How bad would this problem be? My impression (and hope) is that it is tolerable. With the modified definition FTIME will only include the 16 most significant digits of the fine time. This means that we lose resolution but the loss is very small. Hence, I would imagine that we can live with this. What is your opinion? If there are more serious consequences. we need to find a different solution and we are running out of time ...

On a related issue, the Time field in the main tmviewer panel is still blank. Is this a separate problem or is it related to the issue discussed in this ticket. And what can we do to fix it?

[mmecina]

If you don't mind the lower resolution, I think your solution is a viable method. Only the timepack configuration would need to be adjusted, as well as perhaps the timecal function.

Concerning the missing timestamp in the viewer: I suspect this happens because cuc_time_str in ccs_function_lib.py fails (you don't see a message in the log because the exception is only logged on an INFO severity level). The cause is related to the header definition - there is no TIMESYNC property defined. How is the synchronisation status implemented in the PUS-C header? In any case cuc_time_str needs to be updated to accommodate for other implementations of the sync flag.

[pasetti]

The cause is related to the header definition - there is no TIMESYNC property defined. How is the synchronisation status implemented in the PUS-C header?

We are compliant to PUS-C and therefore we have the synchronization status in the SC_REFTIME field of the secondary TM header. I believe that TIMESYNC no longer exists in newer ICDs (I could be wrong here ...)

In any case cuc_time_str needs to be updated to accommodate for other implementations of the sync flag.

OK. I will look at this. But isn't this function supposed to be fixed for all configurations? Is it not enough to just update the the timecal function in the configuration file?

[mmecina]

In that case the simplest solution is to just rename the SCTRS property to TIMESYNC in the structure definition. I have not yet practically worked with PUS-C, what are the values of this field in your project? Does it also serve as a synced/unsynced flag?

Is it not enough to just update the the timecal function in the configuration file?

No, as the cuc_time_str function is now, it references the TIMESYNC property from the TMHeader instance

[pasetti]

In that case the simplest solution is to just rename the SCTRS property to TIMESYNC in the structure definition.

Actually, this looks like a very good solution. I will try it and will let you know.

I have not yet practically worked with PUS-C, what are the values of this field in your project? Does it also serve as a synced/unsynced flag?

The SCTRS does indeed serve as synchronization flag. It consists of 4 bits but only the MSB is used and it is set to 1 when the time is synchronized and to 0 otherwise. Thus, when time is synchronized, SCTRS is equal to 8. When, instead, time is not synchronized, it is equal to 0. No other values are possible.

I cannot really say whether this PUS_C or just COMETINTERCEPTOR but I think that it might be the way things will be in the future.

[mmecina]

The SCTRS does indeed serve as synchronization flag. It consists of 4 bits but only the MSB is used and it is set to 1 when the time is synchronized and to 0 otherwise. Thus, when time is synchronized, SCTRS is equal to 8. When, instead, time is not synchronized, it is equal to 0. No other values are possible.

Understood. Then also TSYNC_FLAG should be updated to TSYNC_FLAG = {0: 'U', 8: 'S'}

[pasetti]

I have update the configuration as discussed in the previous posts and I can confirm that the rendering of the time-stamp now works nominally!

Just one point: I am now ignoring the LSB of the time information in the TM header. Will this affect the computation of the CRC? In other words, is it possible that the CCS computes a wrong CRC because it neglects the LSB of the time field?

[mmecina]

Great!

is it possible that the CCS computes a wrong CRC because it neglects the LSB of the time field?

No, the TMHeader class is only used to decode the header of a PUS packet. CRC takes the raw byte string of the entire packet, which is not affected by this and remains unaltered.

[pasetti]

I have added comments in the COMETINTERCEPTOR configuration file which explain the changes discussed in this ticket. I therefore close the ticket.