Skip to content

Refactor main#476

Merged
jgfoster merged 10 commits intoOpen-Acidification:mainfrom
jgfoster:december
Jan 8, 2025
Merged

Refactor main#476
jgfoster merged 10 commits intoOpen-Acidification:mainfrom
jgfoster:december

Conversation

@jgfoster
Copy link
Member

No description provided.

@jgfoster jgfoster requested a review from je-foster December 27, 2024 04:19
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert these changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Change to match new loop timers.

unsigned long thisTime = millis();
if (report_loop_delay && lastTime && thisTime - lastTime > 500) {
// report unusual delay
serial(F("unexpected delay of %i ms"), thisTime - lastTime);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be clearer to measure the difference between currentLoopStart and previousLoopStop

Copy link
Contributor

Choose a reason for hiding this comment

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

This would also be a good place to measure and report the difference between currentLoopStart and previousLoopStart

serial(F("TankController::loop() - took %lu ms (at %lu sec uptime)"), loopTime, start / 1000);
}
lastTime = millis();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, just set previousLoopStop to millis() ("previous" isn't quite right, but it's about to be previous)

Copy link
Contributor

@je-foster je-foster left a comment

Choose a reason for hiding this comment

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

See comments regarding loop delay timers (and its test) and the sine amplitude being nonzero.

Copy link
Contributor

@je-foster je-foster left a comment

Choose a reason for hiding this comment

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

It looks good.

@jgfoster jgfoster merged commit 4d61a96 into Open-Acidification:main Jan 8, 2025
13 checks passed
@jgfoster jgfoster deleted the december branch January 8, 2025 00:59
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.

2 participants