Skip to content

Modified integtests to make use of new utility functions for setting trigger config params#491

Open
bieryAtFnal wants to merge 3 commits into
developfrom
kbiery/integtest_trigger_choices
Open

Modified integtests to make use of new utility functions for setting trigger config params#491
bieryAtFnal wants to merge 3 commits into
developfrom
kbiery/integtest_trigger_choices

Conversation

@bieryAtFnal

Copy link
Copy Markdown
Collaborator

Description

These changes make use of the changes made in the integrationtest repo to provide utility functions that set various trigger parameters for integration tests (DUNE-DAQ/integrationtest#161).

There are instructions for testing these changes in DUNE-DAQ/integrationtest#161, and these changes need to be tested and merged together the with changes in the integrationtest repo.

Type of change

  • Optimization (non-breaking change that improves code/performance)

Testing checklist

  • Full set of integration tests pass (dunedaq_integtest_bundle.sh)

@MRiganSUSX MRiganSUSX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one comment here.

)
utility_functions.set_RTCM_trigger_params(oversize_conf, trigger_rate=trigger_rate,
readout_window_backshift_ticks=0,
readout_window_before_ticks=2.5*readout_window_time_before,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

readout_window_time_before is hinted to be int (as it should) in https://github.com/DUNE-DAQ/integrationtest/blob/24fce0f387d7609946277b77ee39612213cc65f1/src/integrationtest/utility_functions.py#L102 but used here with float multiplication (2.5).
While this won't cause any problems at runtime, it can break some python type checkers (as int is compatible with float but not the other way around).
I'd suggest casting to int here as:

int(2.5*readout_window_time_before),

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 this pull request may close these issues.

3 participants