Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
coretl opened this issue Feb 29, 2024 · 9 comments · May be fixed by #196
Open

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

coretl opened this issue Feb 29, 2024 · 9 comments · May be fixed by #196
Assignees
Labels

Comments

@coretl
Copy link
Contributor

coretl commented Feb 29, 2024

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
@coretl coretl added the bug label Feb 29, 2024
@tomtrafford tomtrafford self-assigned this Apr 11, 2024
@tomtrafford
Copy link
Contributor

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 PandABlocks/PandABlocks-server#37 ?

@EmilioPeJu
Copy link
Contributor

EmilioPeJu commented May 13, 2024

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
Copy link
Contributor

This current problem is different behaviour to what Glenn reported in #129 (which was fixed by PandABlocks/PandABlocks-server#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
Copy link
Contributor Author

coretl commented Jun 26, 2024

Is it this:

elsif (ts_start_enable = '0' and gate_rise = '1') then
ts_start_enable <= '1';
ts_start <= std_logic_vector(timestamp);

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
Copy link
Contributor

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

@tomtrafford
Copy link
Contributor

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

@EmilioPeJu
Copy link
Contributor

EmilioPeJu commented Jul 1, 2024

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

@tomtrafford
Copy link
Contributor

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

@EmilioPeJu
Copy link
Contributor

EmilioPeJu commented Jul 1, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants