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

Disabled the warning messages about too many digits for TR numbers #84

Merged
merged 3 commits into from
May 15, 2024

Conversation

bieryAtFnal
Copy link
Collaborator

Commented out the warnings about too many digits for TR number and sequence number, since we now realize we want the config params for them to specify the minimum number of digits not a fixed number of digits.

Also, changed the documentation strings in the schema for the relevant config params to sa that the params specify the minimum number of digits.

In the future, we may change the name of the config params to have "minimum" in them, but we won't do that now, in the spirit of minimizing changes

…uence number, since we now realize we want the config params to specify the minimum number of digits not a fixed number of digits.
@bieryAtFnal bieryAtFnal added the miscellaneous deliverable A change that is/will be part of a release but is not substantial enough to be a daq-deliverable label Apr 30, 2024
@bieryAtFnal
Copy link
Collaborator Author

To test this, I used steps similar to numbers 1-6 on this page, then I edited the generated dataflow0_conf.json file so that the number of expected TriggerRecord digits is 2 instead of 5,

[biery@daq data]$ diff dataflow0_conf.json dataflow0_conf.json~
19c19
<                         "digits_for_record_number": 2,
---
>                         "digits_for_record_number": 5,

then I ran nanorc until more than 100 TriggerRecords were generated, and I noted the warning messages in the logs.
Then I updated the software area to use this branch, re-did the nanorc run, and verified that the warnings went away.

@bieryAtFnal bieryAtFnal requested review from wesketchum and eflumerf May 8, 2024 17:51
@@ -67,7 +67,7 @@ HDF5FileLayout::get_record_number_string(uint64_t record_number, // NOLINT(build
int width = m_conf_params.digits_for_record_number;

if (record_number >= m_powers_ten[m_conf_params.digits_for_record_number]) {
ers::warning(FileLayoutNotEnoughDigitsForPath(ERS_HERE, record_number, m_conf_params.digits_for_record_number));
//ers::warning(FileLayoutNotEnoughDigitsForPath(ERS_HERE, record_number, m_conf_params.digits_for_record_number));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would vote for either removing this line entirely, or perhaps making it a TLOG_DEBUG so we still have visibility when the minimum is insufficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Eric suggested, I removed the lines entirely. (And, in fact, I took the opportunity to remove a few more lines of unnecessary code.)

@bieryAtFnal bieryAtFnal merged commit 532f90e into patch/fddaq-v4.4.x May 15, 2024
@bieryAtFnal bieryAtFnal deleted the kbiery/no_warnings_hdf5_digits branch May 15, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miscellaneous deliverable A change that is/will be part of a release but is not substantial enough to be a daq-deliverable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants