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.GATE_DURATION produces two int32 in RAW mode and a single double in SCALED #47

Closed
evalott100 opened this issue Feb 23, 2024 · 5 comments · Fixed by #52
Closed

PCAP.GATE_DURATION produces two int32 in RAW mode and a single double in SCALED #47

evalott100 opened this issue Feb 23, 2024 · 5 comments · Fixed by #52
Assignees

Comments

@evalott100
Copy link

@dmgav found an issue:

With these PCAP settings
image

In RAW mode, GATE_DURATION is sent twice as two int32:

$ nc <ip> 8889
RAW
OK
arm_time: 2024-02-23T13:25:42.576617315Z
start_time: 2024-02-23T13:25:42.576666305Z
missed: 0
process: Raw
format: ASCII
fields:
 PCAP.GATE_DURATION int32 Value scale: 8e-09 offset: 0 units: s
 PCAP.GATE_DURATION int32 Value scale: 8e-09 offset: 0 units: s
 COUNTER1.OUT int32 Min scale: 1 offset: 0 units: 
 COUNTER1.OUT int32 Max scale: 1 offset: 0 units: 
 PCAP.TS_TRIG int64 Value scale: 8e-09 offset: 0 units: s
 COUNTER1.OUT int64 Mean scale: 1 offset: 0 units: 

 0x0032a27b 0x0032a27b 0x00026fb1 0x00026fb1 0x0032a27b 0x00000000 0x5c6bac0b 0x0000007b
 0x07735940 0x07735940 0x00026fb1 0x00026fb2 0x07a5fbbb 0x00000000 0xe466ce7f 0x00001226
 ...

While in SCALED it is sent as a single double:

$ nc <ip> 8889

OK
arm_time: 2024-02-23T13:25:54.448425957Z
start_time: 2024-02-23T13:25:54.448473957Z
missed: 0
process: Scaled
format: ASCII
fields:
 PCAP.GATE_DURATION double Value scale: 8e-09 offset: 0 units: s
 COUNTER1.OUT double Min scale: 1 offset: 0 units: 
 COUNTER1.OUT double Max scale: 1 offset: 0 units: 
 PCAP.TS_TRIG double Value scale: 8e-09 offset: 0 units: s
 COUNTER1.OUT double Mean scale: 1 offset: 0 units: 

 0.154739792 159677 159677 0.154739792 159677
 1 159677 159678 1.154739792 159678
 1 159678 159679 2.154739792 159679
 1 159679 159680 3.154739792 159680
 1 159680 159681 4.154739792 159681
 1 159681 159682 5.154739792 159682
 ...
@Araneidae Araneidae self-assigned this Feb 23, 2024
@Araneidae
Copy link
Contributor

Hmm. For reference, the corresponding entry for GATE_DURATION in config is:

PCAP
    GATE_DURATION ext_out samples

and is processed using CAPTURE_MODE_SCALED32, which is pretty well the normal capture mode.

However ... I seem to remember that ext_out samples needs special processing, I think it needs to be used to scale other captured values, so there must be a subtle bug in how this is generated.

Capturing sample_count is configured here:

/* Populate the sample_count extension bus field. This is treated specially
* as it is needed for averaging operations. */
get_samples_capture_info(captured_fields.sample_count);
it looks like the decision to send the field separately is here:
/* In RAW mode we might have an anonymous sample count field to publish */
bool raw_process = options->data_process == DATA_PROCESS_RAW;
if (raw_process && sample_count_is_anonymous(capture))
send_field_info(file, options, fields->sample_count);
.

My best guess is that sample_count_is_anonymous() is returning true which means that ensure_sample_count() defined here:

static bool ensure_sample_count(
const struct captured_fields *fields, struct gather *gather)
is failing to spot that is already being captured. I'll try and reproduce unless you can spot the obvious mistake!

@coretl
Copy link
Contributor

coretl commented Feb 23, 2024

Might have something to do with

IF(ext_out->ext_type == EXT_OUT_SAMPLES,

Did we change the ext_out type of the field from samples to something else when we changed its name?

@coretl
Copy link
Contributor

coretl commented Feb 23, 2024

Ignore that last comment, we didn't change its type... I can't see anything obvious

@EmilioPeJu
Copy link
Contributor

EmilioPeJu commented Mar 1, 2024

I'm not able to reproduce this problem using my branch, either there is some extra configuration needed, or I accidentally fixed it

EmilioPeJu added a commit that referenced this issue Mar 6, 2024
`ensure_sample_count` is responsible for finding sample count if it
was already captured and we need it for a mean or std_dev capture.
The problem was the it was looking for it in the wrong capture group, it
should be the scaled32 group instead of the unscaled group.

This fixes #47
EmilioPeJu added a commit that referenced this issue Mar 6, 2024
`ensure_sample_count` is responsible for finding sample count if it
was already captured and we need it for a mean or std_dev capture.
The problem was that it was looking for it in the wrong capture group, it
should be the scaled32 group instead of the unscaled group.

This fixes #47
@Araneidae
Copy link
Contributor

Turns out we did change the type of EXT_OUT_SAMPLES here: 2b39c87. Fixed by @EmilioPeJu in #52.

EmilioPeJu added a commit that referenced this issue Mar 6, 2024
`ensure_sample_count` is responsible for finding sample count if it
was already captured and we need it for a mean or std_dev capture.
The problem was that it was looking for it in the wrong capture group, it
should be the scaled32 group instead of the unscaled group.

This fixes #47
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

Successfully merging a pull request may close this issue.

4 participants