Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide skew-correct's "Lost X us!" messages by default #310

Merged
merged 2 commits into from
Jun 20, 2021

Conversation

wavexx
Copy link
Collaborator

@wavexx wavexx commented Jun 7, 2021

Description

When enabling skew-correct, the "Lost X us!" messages can overwhelm the
output. Hide the message behind the verbosity switch.

A single -v switch now shows only uncompensated time, which is what
matters most.

-vv also shows the existing message as well.

Behaviour/ Breaking changes

No breaking behavior AFAIK.

Have you tested the changes?

Yes

Other

n/a

Linked issues:

none

When enabling skew-correct, the "Lost X us!" messages can overwhelm the
output. Hide the messages behind the verbosity switch.

A single -v switch now shows only uncompensated time, which is what
matters most.

-vv also shows the existing message as well.
@vintagepc
Copy link
Owner

TBH it was kind of deliberate; I'd done it that way to make it abundantly clear when a host computer can't keep up (and that --skew-correct can't be considered reliable for timing-precise applications). Let me think on this, perhaps there is some sort of middleground that doesn't bury a potentially nasty issue behind -v switches.

@codecov
Copy link

codecov bot commented Jun 20, 2021

Codecov Report

Merging #310 (50bdeda) into master (7b5d679) will decrease coverage by 0.11%.
The diff coverage is 9.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #310      +/-   ##
==========================================
- Coverage   89.02%   88.91%   -0.12%     
==========================================
  Files         140      140              
  Lines        6645     6655      +10     
==========================================
+ Hits         5916     5917       +1     
- Misses        729      738       +9     
Impacted Files Coverage Δ
parts/Board.h 91.66% <ø> (ø)
parts/Board.cpp 75.26% <9.09%> (-2.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b5d679...50bdeda. Read the comment docs.

@wavexx
Copy link
Collaborator Author

wavexx commented Jun 20, 2021

I like this approach. Does your computer keep up? Because mine can't ;(

@vintagepc
Copy link
Owner

For the most part, yes - I was able to run Klipper without any time skewing issues. I'd still lose a few ms here and there. But the full MK404 stack with graphics and also klipper threads in the background requires a lot of overhead simply because it's software emulation of the hardware. and AVR CPU. Throw in cpu context switching if there aren't enough cores for the required threads and it'll go downhill pretty quickly. Some loss is to be expected regardless, but it's really the repeated and large values that are a problem when interfacing with external applications.

@vintagepc vintagepc merged commit bb0a4e2 into vintagepc:master Jun 20, 2021
@wavexx wavexx deleted the silent_skew_correct branch September 11, 2021 14:12
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