feat(g249 tracking): Add method to select the VFTX.#1316
Conversation
doc(tracking): change licenses fix: Fix classdefoverride fix: Fix typo fix(tracking): Add correct date in the copyright fix: Fixed minor errors suggested by copilot
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to select which VFTX (time-to-digital converter) to use in tracking classes. Previously, the code was hardcoded to use VFTX #2. The changes introduce a configurable setter method while maintaining backward compatibility by keeping VFTX #2 as the default.
Changes:
- Added
SetVFTXNumber()method to configure which VFTX data to use - Replaced hardcoded VFTX number (2) with configurable
vftxNumbermember variable - Updated copyright years to 2025-2026
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tracking/R3BTrackingG249.h | Added SetVFTXNumber() setter and vftxNumber private member; changed include guards to pragma once; incremented ClassDef version |
| tracking/R3BTrackingG249.cxx | Updated FRS data filtering to use vftxNumber variable; removed trailing semicolon from ClassImp |
| ssd/calibration/R3BFootHit2Track.h | Added SetVFTXNumber() setter and vftxNumber private member; incremented ClassDef version |
| ssd/calibration/R3BFootHit2Track.cxx | Updated FRS data filtering to use vftxNumber variable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void SetAnglesTofd(double x, double y, double z) { tofd_angles.SetXYZ(x, y, z); } // rad | ||
|
|
||
| // Setter for the VFTX number | ||
| void SetVFTXNumber(int nb) { vftxNumber = nb; } |
There was a problem hiding this comment.
The trailing semicolon after the closing brace is unnecessary for inline member function definitions. While not an error in C++, it's inconsistent with the coding style used for other setter methods in the same file (lines 50, 53, 57-58) which don't have trailing semicolons.
| int vftxNumber = 2; | ||
|
|
||
| //-- Input hit data from the TClonesArray |
There was a problem hiding this comment.
The vftxNumber member variable is placed in the private section but appears after the public ClassDefOverride at line 299. According to typical C++ class organization and ROOT conventions, all data members should be grouped together in the private section before the ClassDefOverride macro. Consider moving this declaration to a more logical position with other member variables (e.g., near line 194 where other private members begin).
| int vftxNumber = 2; | |
| //-- Input hit data from the TClonesArray | |
| //-- Input hit data from the TClonesArray | |
| int vftxNumber = 2; |
| void PrepareMinimizer(); // reset and set limits for the minimizer | ||
| bool RunMinimizer(); // execute minimzation | ||
|
|
||
| void SetVFTXNumber(int nb) { vftxNumber = nb; }; |
There was a problem hiding this comment.
The trailing semicolon after the closing brace is unnecessary for inline member function definitions. While not an error in C++, it's inconsistent with the coding style used for other methods in the class.
| void SetVFTXNumber(int nb) { vftxNumber = nb; }; | |
| void SetVFTXNumber(int nb) { vftxNumber = nb; } |
| public: | ||
| ClassDefOverride(R3BFootHit2Track, 1); | ||
| private: | ||
| // Select the VFTX |
There was a problem hiding this comment.
The vftxNumber member is declared in a public section (after line 149), which exposes it for direct modification. This breaks encapsulation since a setter method (SetVFTXNumber) is provided. The variable should be moved to a private section to enforce use of the setter method.
Classes R3BFootHit2Track and R3BTrackingG249 only use data taken with the second VFTX. This PR adds a method to select the VFTX. The second VFTX is the default one, though.
Checklist:
devbranch