fprime icon indicating copy to clipboard operation
fprime copied to clipboard

GDS PktDecoder does not send out PktData, sends ChData instead

Open zimri-leisher opened this issue 1 year ago • 5 comments

F´ Version pip lists fprime-gds as version 3.3.0
Affected Component fprime-gds


Problem Description

According to the documentation of the decode_api function in fprime_gds.common.decoder.pkt_decoder.PktDecoder, it should send out instances of the PktData class to all those who register to it. However, it just sends out a list of ChData objects that make up that packet.

Expected Behavior

Classes which register to the PktDecoder should receive PktData in their data_callback methods.

zimri-leisher avatar Feb 07 '24 21:02 zimri-leisher

This should be a two line fix in fprime_gds.common.decoder.pkt_decoder.PktDecoder.decode_api

        # Retrieve the template instance for this channel
        pkt_temp = self.__dict[pkt_id]

        ch_temps = pkt_temp.get_ch_list()

        ch_data_objs = []
        for ch_temp in ch_temps:
            val_obj = self.decode_ch_val(data, ptr, ch_temp)
            ptr += val_obj.getMaxSize()
            ch_data_objs.append(ChData(val_obj, pkt_time, ch_temp))

        return ch_data_objs

Becomes this:

        # Retrieve the template instance for this channel
        pkt_temp = self.__dict[pkt_id]

        ch_temps = pkt_temp.get_ch_list()

        ch_data_objs = []
        for ch_temp in ch_temps:
            val_obj = self.decode_ch_val(data, ptr, ch_temp)
            ptr += val_obj.getMaxSize()
            ch_data_objs.append(ChData(val_obj, pkt_time, ch_temp))

        pkt_data_obj = PktData(ch_data_objs, pkt_time, pkt_temp)

        return [pkt_data_obj]

Also need to import PktData.

zimri-leisher avatar Feb 07 '24 21:02 zimri-leisher

@timcanham would you weigh-in here?

LeStarch avatar Mar 27 '24 23:03 LeStarch

Without digging deeply into the code, I didn't see the point of saving a packet history, just the channels. So I converted each packet into a list of channel entries so that the upstream caller could just put the channels into the channel history. It was the easiest way to feed the downstream consumers like the GUI and the testing API.

You could keep a packet history, but then the upstream caller would have to pull out the channels anyway. I didn't want the users of the API to have to know if channels came from packets or streaming channel updates.

timcanham avatar Mar 27 '24 23:03 timcanham

I am going to note on this issue that it might be good to do both: decode packets into channels, and decode packets. That way we can track packet-information for users (and say render packets in the GDS).

LeStarch avatar Mar 28 '24 17:03 LeStarch

It was mostly useful because we wanted to know what packet a channel was from. There's probably another way to find that out. For the time being I wrote an exact copy of the PktDecoder with this fixed, and I'm just registering it in the standard pipeline instead of the normal PktDecoder. Besides that, just in general, I would expect something named ChDecoder to output channels, and PktDecoder to output packets.

zimri-leisher avatar Apr 01 '24 14:04 zimri-leisher