larpix / larpix-control

Control the LArPix chip
Other
4 stars 12 forks source link

IO group is not saved to hdf5 file for TimestampPackets #249

Open peter-madigan opened 2 years ago

peter-madigan commented 2 years ago

TimestampPackets only have a chip_key attribute (not an io_group field), so this is skipped when converting the packets to array data within the LArPix hdf5 format. This is a bug - we should be tracking the io_group data for all packet types.

sam-fogarty commented 1 year ago

@peter-madigan I've recently noticed this issue in larnd-sim when it simulates timestamp packets, since it uses larpix-control. Are there any plans to address this bug? Do you have a vision of how this bug could be addressed? Is it as "simple" as modifying packet/timestamp_packet.py?

peter-madigan commented 1 year ago

No plans as far as I know, but this should be fixed.

I think it should be a simple fix of adding a property to the timestamp packet so that it will parse the io group out of the chip key if it is available. Something like:

@property
def io_group(self):
    if self.chip_key is not None:
        return None
    else:
        return self.chip_key.io_group

You'd also need to modify the method that creates the timestamp packets from the pacman messages: https://github.com/larpix/larpix-control/blob/7910985fa26d22723b2ed4360195a27f51ab890c/larpix/format/pacman_msg_format.py#L239

This should add a chip key to the timestamp packet of io_group-0-0.

I'm about 90% sure that these two changes would solve the issue, but you would need to do some testing to verify.