Skip to content

TOF: extend digit QC#904

Merged
Barthelemy merged 3 commits into
AliceO2Group:masterfrom
njacazio:nj-memleak
Oct 14, 2021
Merged

TOF: extend digit QC#904
Barthelemy merged 3 commits into
AliceO2Group:masterfrom
njacazio:nj-memleak

Conversation

@njacazio
Copy link
Copy Markdown
Collaborator

  • Extend configuration of checks/tasks
  • Use standard types
  • Improve performance
  • Use std::make_shared
  • Using default destructor in TaskDigits
  • Add running mode for TaskDigits
  • Simplified RawMultiplicity checker
  • Improve messages for shifter
  • Update TOF jsons

@njacazio
Copy link
Copy Markdown
Collaborator Author

This goes with AliceO2Group/AliceO2#7301 for the memory leak fix

@njacazio
Copy link
Copy Markdown
Collaborator Author

@ercolessi please have a look

@njacazio njacazio force-pushed the nj-memleak branch 2 times, most recently from f1cb7a7 to 8fbd07c Compare October 13, 2021 17:41
namespace o2::quality_control_modules::tof
{

struct MessagePad {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great idea

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

indeed

Copy link
Copy Markdown
Collaborator

@ercolessi ercolessi left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

@njacazio njacazio force-pushed the nj-memleak branch 2 times, most recently from c74dfab to 4c952b1 Compare October 13, 2021 18:48
- Extend configuration of checks/tasks
- Use standard types
- Improve performance
- Use std::make_shared
- Using default destructor in TaskDigits
- Add running mode for TaskDigits
- Simplified RawMultiplicity checker
- Improve messages for shifter
- Update TOF jsons
- Set default drawing style
- Customize ToT histogram
- Add configurable message pad class
  - Add default colors based on quality
@njacazio
Copy link
Copy Markdown
Collaborator Author

@Barthelemy @knopers8 we are ready to have this reviewed and merged! Suggestions are as always more than welcome, note that I added a class to have a message pad in the QCG, maybe this could be a framework feature instead of a TOF one.
Thanks!

Copy link
Copy Markdown
Collaborator

@Barthelemy Barthelemy left a comment

Choose a reason for hiding this comment

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

Very nice. Indeed the MessagePad could be moved to the Framework. Do not hesitate to propose a PR if you want. I can also do it.

See my comments for the rest.

Comment thread Modules/TOF/include/Base/MessagePad.h Outdated
Comment thread Modules/TOF/src/CheckCompressedData.cxx Outdated
msg->AddText("Default message for hDiagnostic");
msg->SetName(Form("%s_msg", mo->GetName()));

TPaveText* msg = mShifterMessages.MakeMessagePad(h, checkResult);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think that there is a memory leak here. msg is not deleted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But the object is added to the list of the monitoring, I think it is then deleted when the MO is deleted, or this is how it should be

Comment thread Modules/TOF/include/Base/MessagePad.h Outdated
@njacazio
Copy link
Copy Markdown
Collaborator Author

@Barthelemy your comments should be included in the version of the code, what do you think of them?

@Barthelemy Barthelemy enabled auto-merge (squash) October 14, 2021 13:12
@Barthelemy Barthelemy merged commit e03f031 into AliceO2Group:master Oct 14, 2021
@njacazio njacazio deleted the nj-memleak branch October 14, 2021 14:39
Barthelemy pushed a commit that referenced this pull request Oct 15, 2021
* TOF: extend digit QC

- Extend configuration of checks/tasks
- Use standard types
- Improve performance
- Use std::make_shared
- Using default destructor in TaskDigits
- Add running mode for TaskDigits
- Simplified RawMultiplicity checker
- Improve messages for shifter
- Update TOF jsons
- Set default drawing style
- Customize ToT histogram
- Add configurable message pad class
  - Add default colors based on quality

* Configure -> configure

* Use smart pointer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants