Skip to content

Merge DFOModule consensus behavior and update peer announcement transport to DFODecision#486

Draft
Copilot wants to merge 25 commits intodevelopfrom
copilot/revise-dfo-protocol-resilience
Draft

Merge DFOModule consensus behavior and update peer announcement transport to DFODecision#486
Copilot wants to merge 25 commits intodevelopfrom
copilot/revise-dfo-protocol-resilience

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 15, 2026

Description

If full description and testing details are included on a parent issue, please link to that here.

Otherwise, please include a summary of the change and which issue is fixed (if any).
Include relevant motivation and context, including a target environment and dunedaq version if known.
Also list any dependencies that are required for this change.

This PR merges standalone and consensus behavior into DFOModule, with consensus controlled by DFOConf::consensus_enabled.
It also incorporates follow-up review feedback by:

  • updating DFOCore comments to reflect the single-module architecture, and
  • replacing consensus peer announcements from TriggerDecisionToken-based signaling to DFODecision-based signaling.

DFOModule now handles peer announcement send/receive through DFODecision channels, and unit tests were updated accordingly to inject synthetic peer announcements via DFODecision.

Please also include instructions for how a reviewer can test your changes.

  1. Configure with a DFOModule where DFOConf.consensus_enabled=true and DFODecision input/output connections are present.
  2. Start the module and verify partition formation/peer registration still occurs.
  3. Run updated unit tests for DFOModule consensus scenarios in a full DUNE-DAQ build environment.

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature or enhancement (non-breaking change which adds functionality)
  • Optimization (non-breaking change that improves code/performance)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Testing checklist

  • Unit tests pass (e.g. dbt-build --unittest)
  • Minimal system quicktest passes (pytest -s minimal_system_quick_test.py)
  • Full set of integration tests pass (dunedaq_integtest_bundle.sh)
  • Python tests pass if applicable (e.g. python -m pytest)
  • Pre-commit hooks run successfully if applicable (e.g. pre-commit run --all-files)

Comments here on the testing

In this sandbox, full package build/test is environment-limited:

  • cmake -S . -B ... fails because daq-cmake is not installed.
  • ctest reports no discovered tests in the local sandbox context.

Further checks

  • Code is commented where needed, particularly in hard-to-understand areas
  • Code style is correct (dbt-build --lint, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)
  • If applicable, new tests have been added or an issue has been opened to tackle that in the future.
    (Indicate issue here: # (issue))

Copilot AI linked an issue Apr 15, 2026 that may be closed by this pull request
@eflumerf eflumerf closed this Apr 15, 2026
@eflumerf eflumerf reopened this Apr 15, 2026
Copilot stopped work on behalf of eflumerf due to an error April 15, 2026 15:01
Copilot AI requested a review from eflumerf April 15, 2026 15:27
@eflumerf
Copy link
Copy Markdown
Member

@copilot The new module should only inherit from DAQModule. Common DFO code can be moved to library functions in either src or plugins/detail

Copy link
Copy Markdown
Author

Copilot AI commented Apr 16, 2026

@copilot The new module should only inherit from DAQModule. Common DFO code can be moved to library functions in either src or plugins/detail

Done in commit b280c75. DFOConsensusModule now inherits DAQModule directly, with no dependency on DFOModule. All common DFO processing logic (TRB management, slot selection, dispatching, token handling, busy notification, opmon counters) has been extracted into src/dfmodules/DFOCore.hpp/.cpp and added to the library. Both DFOModule and DFOConsensusModule use DFOCore via unique_ptr composition.

Copilot AI changed the title [WIP] Revise DFO protocol to enhance resilience and consensus Revise DFO protocol to enhance resilience and consensus Apr 16, 2026
@eflumerf
Copy link
Copy Markdown
Member

@copilot Add unit tests for DFOCore

Agent-Logs-Url: https://github.com/DUNE-DAQ/dfmodules/sessions/a1889c64-61e1-42b6-9525-fb129d8e5168

Co-authored-by: eflumerf <61473357+eflumerf@users.noreply.github.com>
Copilot AI and others added 2 commits April 20, 2026 14:48
…comments, watchdog logs)

Agent-Logs-Url: https://github.com/DUNE-DAQ/dfmodules/sessions/8cfa3196-9570-48f5-a552-ad88bae3d298

Co-authored-by: eflumerf <61473357+eflumerf@users.noreply.github.com>
…ments are handled

Agent-Logs-Url: https://github.com/DUNE-DAQ/dfmodules/sessions/ad5ee7fa-bbd5-49fc-8323-82e234ed90b9

Co-authored-by: eflumerf <61473357+eflumerf@users.noreply.github.com>
@eflumerf
Copy link
Copy Markdown
Member

eflumerf commented May 1, 2026

@copilot Fix comments in DFOCore since there is only one DFOModule now. Replace announcement using token with announcement using DFODecision

Copy link
Copy Markdown
Author

Copilot AI commented May 1, 2026

@copilot Fix comments in DFOCore since there is only one DFOModule now. Replace announcement using token with announcement using DFODecision

Implemented in 6f1aeb6. I updated DFOCore comments to refer to the single DFOModule implementation and switched peer announcements from TriggerDecisionToken to DFODecision (send + receive paths, plus updated unit-test injection paths). No UI changes in this PR, so no screenshot applies.

Copilot AI changed the title Revise DFO protocol to enhance resilience and consensus Merge DFOModule consensus behavior and update peer announcement transport to DFODecision May 1, 2026
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.

2 participants