PandABlocks / PandABlocks-FPGA

VHDL functional blocks with their simulations and test sequences
Apache License 2.0
15 stars 13 forks source link

PCAP.TS_START not zeroed when GATE already high at the start of capture #179

Open coretl opened 4 months ago

coretl commented 4 months ago

If PCAP.GATE and PCAP.ENABLE are both tied to ONE, like in this diagram: image

then TS_START on the last frame will be leftover from the previous scan rather than being 0:

$ nc 172.23.252.201 8889

OK
arm_time: 2024-02-29T17:31:03.969Z
start_time: 2024-02-29T17:31:03.970Z
missed: 0
process: Scaled
format: ASCII
fields:
 PCAP.TS_START double Value scale: 8e-09 offset: 0 units: s
 PCAP.TS_END double Value scale: 8e-09 offset: 0 units: s

 0 0.500000064
 0.500000064 1.500000064
 1.500000064 2.500000064
 2.500000064 3.500000064
 3.500000064 4.500000064
END 5 Disarmed
arm_time: 2024-02-29T17:31:10.377Z
start_time: 2024-02-29T17:31:10.377Z
missed: 0
process: Scaled
format: ASCII
fields:
 PCAP.TS_START double Value scale: 8e-09 offset: 0 units: s
 PCAP.TS_END double Value scale: 8e-09 offset: 0 units: s

 4.889428296 0.500000064
 0.500000064 1.500000064
 1.500000064 2.500000064
 2.500000064 3.500000064
 3.500000064 4.500000064
END 5 Disarmed
tomtrafford commented 2 months ago

I've added a timing test in pcap-TS_START-test . However, I cannot replicate this behaviour in the VHDL. @EmilioPeJu Does a change need to be made in the server similar to https://github.com/PandABlocks/PandABlocks-server/issues/37 ?

EmilioPeJu commented 2 months ago

The relevant change to the FPGA already happened when the start time feature was added. Was this bug introduced after that? if you don't see the problem in the timing tests, it might be a subtle problem that could be investigated with chipscope (or similar)

tomtrafford commented 2 months ago

This current problem is different behaviour to what Glenn reported in #129 (which was fixed by https://github.com/PandABlocks/PandABlocks-server/issues/37), it looks like it's only affecting TS_START whereas it previously affected the entire first capture. I think when the previous problem was fixed, it introduced the current issue (or made the current issue noticeable).

coretl commented 2 weeks ago

Is it this: https://github.com/PandABlocks/PandABlocks-FPGA/blob/3fbd543890870a42b24939b44b6d233e6a2966c2/modules/pcap/hdl/pcap_frame.vhd#L159-L161

When GATE=ONE then there will never be a gate_rise, so it's still set to the last TS.

Can we zero ts_start on rising enable? Or trigger gate_rise on rising enable?

EmilioPeJu commented 2 weeks ago

It would be nice simplifying that code, though that would require more work(and testing) than just fixing this bug

tomtrafford commented 2 weeks ago

I've tried putting these changes onto a PandA but it does not seem to have fixed this problem.

EmilioPeJu commented 2 weeks ago

Thanks for testing it, I have added two extra lines that will (hopefully) fix it

tomtrafford commented 2 weeks ago

I've just put the new changes onto a PandA. It still hasn't fixed this.

EmilioPeJu commented 2 weeks ago

is it showing exactly the same behaviour? I'm surprised, as soon as enable goes low, it should be clearing ts_start