Skip to content

[issue-873] Templated Event Buffer #876

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: SharedDevelopment
Choose a base branch
from
Open

[issue-873] Templated Event Buffer #876

wants to merge 5 commits into from

Conversation

@NicolasJPosey
Copy link
Contributor

@NicolasJPosey NicolasJPosey commented Aug 10, 2025 •
edited
Loading

Closes #873 #870

Description

  • Upgraded EventBuffer to be a templated class so that it could be used with other types.
  • Cleaned up redundant overrides in EventBuffer .
  • Added getters, setters, and a couple of methods overrides like size in EventBuffer and data in RecordableVector so that the friend coupling to neuro classes in EventBuffer could be removed.

Checklist (Mandatory for new features)

  • Added Documentation
  • Added Unit Tests

Testing (Mandatory for all changes)

  • GPU Test: test-medium-connected.xml Passed
  • GPU Test: test-large-long.xml Passed

@NicolasJPosey
Upgrade EventBuffer to be a templated class
4fae6c0 
This involved moving the EventBuffer.cpp information into the header file and updating previous usages to use uint64_t type which was the old default. Also cleaned up redudent overrides and added getters and setters to the EventBuffer so that it's friendship with neuro classes could be removed.
@NicolasJPosey NicolasJPosey self-assigned this Aug 10, 2025
@NicolasJPosey NicolasJPosey added cleanup refactor doesn't change functionality, just improves code labels Aug 10, 2025
@NicolasJPosey NicolasJPosey changed the title Upgrade EventBuffer to be a templated class [issue-873] Templated Event Buffer Aug 10, 2025
@NicolasJPosey NicolasJPosey requested a review from stiber August 10, 2025 05:30
stiber
stiber requested changes Aug 11, 2025
View reviewed changes
Copy link
Contributor

@stiber stiber left a comment

Choose a reason for hiding this comment

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

Minor stuff. Let me know if this is essential for this summer's release; otherwise, we will hold it back from SharedDevelopment until the CPU compilation hotfix gets merged into the release.

@NicolasJPosey
Copy link
Contributor Author

NicolasJPosey commented Aug 11, 2025

@stiber this PR is not essential for this summer's release. I can use the DoubleEventBuffer implementation until this makes it into SharedDevelopment then update the 911 GPU code to use the templated version once SharedDevelopment is merged in.

@NicolasJPosey NicolasJPosey requested a review from stiber August 12, 2025 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stiber stiber Awaiting requested review from stiber

Requested changes must be addressed to merge this pull request.

Assignees

@NicolasJPosey NicolasJPosey

Labels

cleanup refactor doesn't change functionality, just improves code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NicolasJPosey @stiber