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

Think of a way to tell the TCP server what 1 clock tick is #8

Closed
thomascobb opened this issue Feb 12, 2021 · 14 comments
Closed

Think of a way to tell the TCP server what 1 clock tick is #8

thomascobb opened this issue Feb 12, 2021 · 14 comments
Assignees

Comments

@thomascobb
Copy link

No description provided.

@thomascobb
Copy link
Author

Add a dedicated *REG register that counts the currently configured fabric clock for 1 second (using a fixed frequency reference clock) which can be used by the server

@coretl
Copy link
Contributor

coretl commented Oct 5, 2022

But this is constantly changing, how can we use this to format TIME fields?

@coretl
Copy link
Contributor

coretl commented Oct 5, 2022

Maybe add a special star command *TIMEBASE? for this, could be set in state so this persists. Would have to walk through all the time fields and update them if it changed

@tomtrafford
Copy link
Contributor

This is required for the 3.0 release. I will organise a discussion about this.

@tomtrafford
Copy link
Contributor

tomtrafford commented Sep 11, 2023

2 new:

  • *CLOCK.TARGET (User specified clock frequency)
  • *CLOCK.ACTUAL (Measured clock frequency)

in *REG (Mirrors of SYSTEM registers)

  • CLOCK_TARGET
  • CLOCK_ACTUAL

The .state file will need to store time field values in RAW

@Araneidae
Copy link
Contributor

Comments from a discussion with @coretl:

  • Let's just have the one extra *REG register, NOMINAL_CLOCK. This is maintained by the FPGA to report the clock that is to be reported to the user. The server isn't going to worry about how truthful this is. This will be polled at, perhaps, 1 Hz.
  • The clock frequency can be interrogated via *CLOCK_FREQ? ... what units does this report in? Let's say Hz with integer resolution.
  • When this frequency changes the server should(?) internally trigger a *CHANGES.CONFIG=S event to reset change reporting for fields that may have changed.

Separately we discussed changing the ext_out SAMPLES field from CAPTURE_MODE_UNSCALED to ..._SCALED32.

@coretl
Copy link
Contributor

coretl commented Sep 18, 2023

After a conversation with @EdWarrick we wondered if changing ext_out SAMPLES to GATE_DURATION might be more descriptive at the same time as changing it from UNSCALED to SCALED32. Thoughts?

@Araneidae
Copy link
Contributor

Araneidae commented Sep 19, 2023

That seems a reasonable idea.

I'm tempted to suggest that this name change is only made in the field name, so that the PCAP entry in config changes as follows:

-    SAMPLES          ext_out   samples
+    GATE_DURATION    ext_out   samples

In particular, this means that this name change isn't part of the server, and we just need the following diff in PandABlocks-FPGA:

diff --git a/modules/pcap/pcap.block.ini b/modules/pcap/pcap.block.ini
index 54d137d3..dcd184cd 100644
--- a/modules/pcap/pcap.block.ini
+++ b/modules/pcap/pcap.block.ini
@@ -43,7 +43,7 @@ description: Timestamp of last gate high +1 in current capture relative to enabl
 type: ext_out timestamp
 description: Timestamp of capture event relative to enable
 
-[SAMPLES]
+[GATE_DURATION]
 type: ext_out samples
 description: Number of gated samples in the current capture
 

Though hmmm... what about the description now?

@Araneidae
Copy link
Contributor

Ok, I will add a new register *REG.NOMINAL_CLOCK at register offset 24 with the following definition:

If this register is non zero the value returned will be used as the reported clock frequency in Hz, otherwise the standard frequency of 125 MHz will be reported and used.

@coretl
Copy link
Contributor

coretl commented Sep 19, 2023

Though hmmm... what about the description now?

How about "Total duration in seconds that GATE was high for each capture"

@Araneidae
Copy link
Contributor

The problem with this is that the unscaled value is simply number of samples, as before!

@coretl
Copy link
Contributor

coretl commented Sep 19, 2023

I think that it's still fine, the scaled value is what they are requesting to capture, if they choose to get it unscaled they see the raw number of samples and the scale factor to turn it into time in the header

Araneidae added a commit that referenced this issue Sep 20, 2023
This means that when samples are captured as scaled data they will now be
reported as durations in seconds.

This in part addresses github issue #8
Araneidae added a commit to PandABlocks/PandABlocks-FPGA that referenced this issue Sep 20, 2023
This adds a new register *REG.NOMINAL_CLOCK which can safely be left to
return the default value of 0 and adds some formatting changes.

This in part addresses issues #512 and PandABlocks/PandABlocks-server#8
@Araneidae
Copy link
Contributor

This should now be addressed by a number of pull requests:

Outstanding related issues are:

Araneidae added a commit that referenced this issue Sep 22, 2023
Addresses issue #8 which can now be closed
@Araneidae
Copy link
Contributor

This should now be closed by merge 19f58f6.

shihab-dls pushed a commit to PandABlocks/PandABlocks-FPGA that referenced this issue May 17, 2024
This adds a new register *REG.NOMINAL_CLOCK which can safely be left to
return the default value of 0 and adds some formatting changes.

This in part addresses issues #512 and PandABlocks/PandABlocks-server#8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants