Jcampos/add channel status by jicampos · Pull Request #49 · Mu2e/otsdaq-mu2e-calorimeter

jicampos · 2026-01-15T15:25:04Z

No description provided.

rlcee · 2026-01-15T17:33:28Z

From the offline point of view, the name "cal.channelstatus" is internal to the conditions system, so I didn't expect to see it here. We would expect the table is only referenced with the public name "CalChannelStatus". I can't follow what the code is doing so I can't comment in any more detail.

…tion coherence, added few comments

giro94 · 2026-01-23T17:02:17Z

First version ready to be merged

rrivera747

Looks good, thanks!

Copilot

Pull request overview

This pull request adds a new SubsystemCalorimeterParametersTable plugin that generates calorimeter-specific configuration files for the online trigger system. The implementation reads channel mapping and status information from configuration tables and exports them as CSV files and JSON structures.

Changes:

Adds new SubsystemCalorimeterParametersTable class that extends TableBase to handle calorimeter parameter configuration
Implements CSV file generation for channel maps and status tables that are written to the filesystem
Adds JSON export functionality for configuration data visualization
Integrates the new TablePlugins directory into the CMake build system

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 18 comments.

File	Description
SubsystemCalorimeterParametersTable.h	Header file defining the table plugin class with methods for CSV and JSON generation
SubsystemCalorimeterParametersTable_table.cc	Implementation of channel map and status table conversion to CSV and JSON formats
TablePlugins/CMakeLists.txt	Build configuration for the new table plugin
otsdaq-mu2e-calorimeter/CMakeLists.txt	Adds TablePlugins subdirectory to the build

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParametersTable_table.cc

Copilot · 2026-01-23T17:42:24Z

otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParametersTable_table.cc

+
+		// assume data is 1-dimensional
+		for(uint32_t j = 0; j < bitmap.numberOfColumns(0); j++) {
+			OfflineTable << mapChannels_.at(boardID * 20 + j) << ", ";


Using mapChannels_.at() will throw an std::out_of_range exception if the key boardID * 20 + j doesn't exist. This could happen if the channel map and status tables are inconsistent. Consider using find() with proper error handling, or document that the tables must be consistent.

Suggested change

OfflineTable << mapChannels_.at(boardID * 20 + j) << ", ";

const uint32_t onlineId = boardID * 20 + j;

auto it = mapChannels_.find(onlineId);

if(it == mapChannels_.end()) {

__COUT_WARN__ << "No channel map entry found for online ID " << onlineId

<< " (boardID=" << boardID << ", channel=" << j

<< "); skipping status entry." << __E__;

continue;

}

OfflineTable << it->second << ", ";

@copilot use SS and SS_THROW exception with the find.

otsdaq-mu2e-calorimeter/TablePlugins/CMakeLists.txt

otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParametersTable_table.cc

Copilot · 2026-01-23T18:35:00Z

@rrivera747 I've opened a new pull request, #51, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-01-23T18:35:59Z

@rrivera747 I've opened a new pull request, #52, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-01-23T18:37:07Z

@rrivera747 I've opened a new pull request, #53, to work on those changes. Once the pull request is ready, I'll request review from you.

…d rethrow in catch block Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Replace silent failures with __SS__ / __SS_THROW__ in offline table generation

Copilot · 2026-01-23T21:31:38Z

@rrivera747 I've opened a new pull request, #54, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

…etersTable_table.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Replace magic number with CHANNELS_PER_BOARD constant

giro94 · 2026-01-26T10:11:47Z

otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParametersTable.h

 	const static std::string PATH_TO_TRIGGER_OFFLINE_DB;
 	const static std::string CHANNEL_STATUS_TABLE;
 	const static std::string CHANNEL_MAP_TABLE;
+	const static uint32_t CHANNELS_PER_BOARD = 20;


Note that we have mu2e::CaloConst::_nChPerDIRAC defined in Offline, if you prefer.

@copilot use mu2e::CaloConst::_nChPerDIRAC for the 20

…etersTable_table.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-01-26T15:38:32Z

@rrivera747 I've opened a new pull request, #55, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-01-26T15:39:24Z

@rrivera747 I've opened a new pull request, #56, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Fix field naming inconsistency in ColChannelThreshold struct

Replace hardcoded channel count with mu2e::CaloConst::_nChPerDIRAC

…rd. Also, added throw exception if channel not found in map

giro94 · 2026-01-29T17:44:55Z

PR should be ready to be merged @rrivera747

jicampos added 4 commits January 9, 2026 17:52

Adding TablePlugin for DB info dump

a0acf33

Adding comment

3d703aa

Clangify

751012a

Writing offline tables to file and adding to JSON blob

426e4da

jicampos and others added 5 commits January 15, 2026 12:25

Adding nominal channel map and loopup in CSV dump

1a9855d

Merge branch 'develop' into jcampos/AddChannelStatus

c7c75e2

Added ChannelMap table logic, removed nominal.txt reads.

019e9e7

Swapped channel map order to new standard. Fixed some bracket indenta…

3d33780

…tion coherence, added few comments

fix whitespace, clangify

d0949d3

giro94 requested a review from rrivera747 January 23, 2026 17:32

rrivera747 approved these changes Jan 23, 2026

View reviewed changes

Merge branch 'develop' into jcampos/AddChannelStatus

0cc0aaf

rrivera747 requested a review from Copilot January 23, 2026 17:36

Copilot started reviewing on behalf of rrivera747 January 23, 2026 17:37 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

Initial plan

d6f48c9

Copilot AI mentioned this pull request Jan 23, 2026

Replace silent failures with __SS__ / __SS_THROW__ in offline table generation #51

Merged

Initial plan

7ef7f2e

Copilot AI mentioned this pull request Jan 23, 2026

Replace magic number with CHANNELS_PER_BOARD constant #52

Merged

Copilot AI mentioned this pull request Jan 23, 2026

Use __SS__ and __SS_THROW__ for channel map lookup error handling #53

Closed

Copilot AI and others added 5 commits January 23, 2026 18:37

Replace simple return with __SS__ and __SS_THROW__ error handling, ad…

39c4454

…d rethrow in catch block Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Apply suggestion from @Copilot

eb4ed03

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Create and use CHANNELS_PER_BOARD constant

cca443e

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Final update: all feedback addressed

9fc9a2b

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Complete CHANNELS_PER_BOARD refactoring

58f34f4

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

rrivera747 and others added 3 commits January 23, 2026 15:21

Merge branch 'jcampos/AddChannelStatus' into copilot/sub-pr-49

b0ccb32

Merge pull request #51 from Mu2e/copilot/sub-pr-49

cf9a6d9

Replace silent failures with __SS__ / __SS_THROW__ in offline table generation

Initial plan

c8fb0f0

Copilot AI mentioned this pull request Jan 23, 2026

Fix field naming inconsistency in ColChannelThreshold struct #54

Merged

rrivera747 and others added 6 commits January 23, 2026 15:32

Apply suggestions from code review

31621c9

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Rename colStatus_ to colBoardId_ in ColChannelThreshold struct

5dd8b4c

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Update otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParam…

cd1036d

…etersTable_table.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParam…

ec0e7a8

…etersTable_table.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'jcampos/AddChannelStatus' into copilot/sub-pr-49-again

9297f32

Merge pull request #52 from Mu2e/copilot/sub-pr-49-again

ac0a29d

Replace magic number with CHANNELS_PER_BOARD constant

giro94 reviewed Jan 26, 2026

View reviewed changes

Update otsdaq-mu2e-calorimeter/TablePlugins/SubsystemCalorimeterParam…

bd25651

…etersTable_table.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Jan 26, 2026

Replace hardcoded channel count with mu2e::CaloConst::_nChPerDIRAC #55

Closed

Initial plan

a21a6cc

Copilot AI mentioned this pull request Jan 26, 2026

Replace hardcoded channel count with mu2e::CaloConst::_nChPerDIRAC #56

Merged

Copilot AI and others added 6 commits January 26, 2026 15:41

Replace hardcoded 20 with mu2e::CaloConst::_nChPerDIRAC

36a3f1d

Co-authored-by: rrivera747 <107584474+rrivera747@users.noreply.github.com>

Merge pull request #54 from Mu2e/copilot/sub-pr-49

921a82c

Fix field naming inconsistency in ColChannelThreshold struct

Merge branch 'jcampos/AddChannelStatus' into copilot/sub-pr-49-yet-again

e9205ca

Merge pull request #56 from Mu2e/copilot/sub-pr-49-yet-again

07ea18d

Replace hardcoded channel count with mu2e::CaloConst::_nChPerDIRAC

Using caloconst constants instead of hardcoded 20 for channels in boa…

5cc0c5f

…rd. Also, added throw exception if channel not found in map

Removed unused function

e941230

rrivera747 merged commit d5e8cc6 into develop Jan 29, 2026
7 of 9 checks passed

rrivera747 deleted the jcampos/AddChannelStatus branch January 29, 2026 17:55

-			OfflineTable << mapChannels_.at(boardID * 20 + j) << ", ";
+			const uint32_t onlineId = boardID * 20 + j;
+			auto           it       = mapChannels_.find(onlineId);
+			if(it == mapChannels_.end()) {
+				__COUT_WARN__ << "No channel map entry found for online ID " << onlineId
+				              << " (boardID=" << boardID << ", channel=" << j
+				              << "); skipping status entry." << __E__;
+				continue;
+			}
+			OfflineTable << it->second << ", ";

Conversation

jicampos commented Jan 15, 2026

Uh oh!

rlcee commented Jan 15, 2026

Uh oh!

giro94 commented Jan 23, 2026

Uh oh!

rrivera747 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

rrivera747 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jan 23, 2026

Uh oh!

Copilot AI commented Jan 23, 2026

Uh oh!

Copilot AI commented Jan 23, 2026

Uh oh!

Copilot AI commented Jan 23, 2026

Uh oh!

giro94 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

rrivera747 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 26, 2026

Uh oh!

Copilot AI commented Jan 26, 2026

Uh oh!

giro94 commented Jan 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants