Skip to content

Conversation

@githubdoe
Copy link
Owner

New code to blink top two ronchi selected wave fronts

@github-actions
Copy link

🚀 New build available for commit 9dbdb6a
Download installer here

@gr5
Copy link
Collaborator

gr5 commented Jan 15, 2026

Sounds great. I hope to look at this tomorrow.

@githubdoe
Copy link
Owner Author

Deleted debug line in profile.cpp hide save button during blink. Just pushed.

@github-actions
Copy link

🚀 New build available for commit 2bf27ea
Download installer here

@gr5
Copy link
Collaborator

gr5 commented Jan 15, 2026

Well it works. It seems like there should be some code to stop the timer if someone closes the window though.

I tried closing the window while it was blinking and closed the ronchi window also and even deleted one of the wavefronts but no crash so I guess it's okay? I'd feel better if you detected the window closing.

Or does the window run a destructor which in turn runs the timer destructor which stops the timer?

@githubdoe
Copy link
Owner Author

Yes I think that all gets deleted when the window closes.

@githubdoe
Copy link
Owner Author

githubdoe commented Jan 15, 2026

All objects created by a QDialog that have it as their parent are deleted when the dialog is deleted. Widgets that are in a layout have the layout as their parent and the layout eventually has has the dialog as it's parent. The timer has the dialog as it's parent. So it is deleted as well.

@gr5
Copy link
Collaborator

gr5 commented Jan 16, 2026

agreed. timer should be gone at the end of this if statement:

    // Connect the comparison trigger
    connect(compareBtn, &QPushButton::clicked, &previewDlg, [=, &previewDlg]() {
        // [=] copies individualRonchis and names so they stay
        // valid even after generateBatchRonchiImage() returns.
        if (individualRonchis.size() >= 2) {
            RonchiCompareDialog compDlg(individualRonchis[0], names[0],
                                        individualRonchis[1], names[1], &previewDlg);
            compDlg.exec();
        }
    });

@github-actions
Copy link

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-tidy (v18.1.3) reports: 1 concern(s)
  • foucaultview.cpp:217:5: warning: [clang-analyzer-security.FloatLoopCounter]

    Variable 'currentMM' with floating point type 'double' should not be used as a loop counter

      217 |     for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {
          |     ^                                   ~~~~~~~~~                    ~~~~~~~~~
    foucaultview.cpp:217:5: note: Variable 'currentMM' with floating point type 'double' should not be used as a loop counter
      217 |     for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {
          |     ^                                   ~~~~~~~~~                    ~~~~~~~~~

Have any feedback or feature suggestions? Share it here.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

Used clang-tidy v18.1.3

Have any feedback or feature suggestions? Share it here.

if (stepSizeMM <= 0) return;

// 4. Draw Rings and Labels
for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {

Choose a reason for hiding this comment

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

clang-tidy diagnostic

foucaultview.cpp:217:5: warning: [clang-analyzer-security.FloatLoopCounter]

Variable 'currentMM' with floating point type 'double' should not be used as a loop counter

  217 |     for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {
      |     ^                                   ~~~~~~~~~                    ~~~~~~~~~
foucaultview.cpp:217:5: note: Variable 'currentMM' with floating point type 'double' should not be used as a loop counter
  217 |     for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {
      |     ^                                   ~~~~~~~~~                    ~~~~~~~~~

@github-actions
Copy link

🚀 New build available for commit eee6ba2
Download installer here

@gr5
Copy link
Collaborator

gr5 commented Jan 16, 2026

I merged pr 334. @atsju go ahead and close this one and open a new one based on dale's branch. I think that is what you recommend? I have to travel for the next 4 hours and don't have time to look at any of this until much later today.

@atsju atsju closed this Jan 16, 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.

4 participants