Comments (4)
Hi Piezoid
I'll answer inline :-)
As I understand it, on a partial flush, the loop looks in reverse order for a move with an entry velocity that doesn't depend on the following moves. This will act as the starting point for propagating peak velocity to previous moves. When such move can't be found in the unplanned part of the buffer, nothing is done or yielded downstream.
We look in reverse order, assuming the machine comes to a complete stop at the end of the currently available command sequence. When we find a move that can no longer be affected by adding more moves, we set the flush_count
and keep moving backwards. All moves we examine from this point are "certain" and so will be flushed. This is why we have the update_flush_count
to track if we already found a new flush point.
When this search reaches an non-move operation which is also the first unplanned operation, it sets
flush_count
to its index. This enables the operation to be readily yielded bynext_move
. At least, I think this is the intent of this code.Now the issue is that
self.flush_count + 1
is actually the index of the second operation in the unplanned part of the buffer.
This was intended as an optimization to ensure forward progress in some specific case that happened while I was refactoring code. I don't actually believe it's required anymore, but it won't really hurt.
Another thing that I noticed: when the search can't find starting point for planing,
self.flush_count
is set to 0, which will plan again the moves at the start of the buffer. Not sure if this intended, but otherwise, the old value should be left alone. Your implementation also recompute the planning of all moves during a complete flush (!partial
), I don't think the original implementation does this.
Setting flush_count
to zero when nothing could be planned is indeed intended, during partial flushes, as it means the total duration of the buffered moves wasn't long enough to ensure that no future moves could affect them.
You are right wrt. recomputing the buffer on full flushes, that's simply an oversight on my part.
Mainly these issues probably stem from converting from a push style to pull style API. In Klipper, once moves are flushed they are pushed to the next stage. In klipper_estimator
, they stay at the front of the move queue until they are pulled.
I'm not really happy with how this part turned out, as you note it's basically a direct port, warts and all. At some point when I have time I'll go through it and see if I can improve it a bit.
Best regards,
/Lasse
from klipper_estimator.
Thanks for the detailed reply!
Setting
flush_count
to zero when nothing could be planned is indeed intended, during partial flushes, as it means the total duration of the buffered moves wasn't long enough to ensure that no future moves could affect them.
I was noting that partial flushes that cannot plan anything shouldn't mark previously planned moves as unplanned. Although, this doesn't matter when pull driven: planned moves are all removed from the queue before the next partial flush.
I'm not really happy with how this part turned out, as you note it's basically a direct port, warts and all. At some point when I have time I'll go through it and see if I can improve it a bit.
I came with these questions while refactoring the planner to avoid mutations and "make the monoids more explicit". My goal was understanding the pipeline better before trying some variations (like tagging values to get the originals limits). If I end up with something simple enough I'll make a PR, but I fear that this project want to keep the code layout close to Klipper's for maintenance reasons.
from klipper_estimator.
I just pushed a commit that properly splits move sequences. I discovered, based on your input, that my last changes, making G4
pauses instead of indeterminate dwells, resulted in potential issues.
MoveSequences no longer contain timeouts, as those were the potential issue. Instead, there's now an OperationSequence
which contains dwells and move sequences. This means MoveSequence::process
(formerly OperationSequence::process
) no longer handles non-moves at all. I did add a small optimization in the form of a loop to advance over any fill commands in a move sequence.
from klipper_estimator.
MoveSequences no longer contain timeouts, as those were the potential issue. Instead, there's now an
OperationSequence
which contains dwells and move sequences. This meansMoveSequence::process
(formerlyOperationSequence::process
) no longer handles non-moves at all. I did add a small optimization in the form of a loop to advance over any fill commands in a move sequence.
Nice! I'll close this, as it solves the issue.
from klipper_estimator.
Related Issues (20)
- resolving .local? HOT 2
- Works in W11? HOT 2
- Setup instructions for MacOs HOT 3
- FR: scaled_xy_accel support for limited_cartesian HOT 5
- Moonraker API doesn't support "Authorization" HOT 1
- ParseError with macros using leading underscore HOT 1
- Support floats for M73 P HOT 2
- Time estimate way above actual time HOT 4
- Simplyfi3D Support HOT 4
- FR: Timelapse Evaluation HOT 3
- unclear docs
- Massive overestimate of print time HOT 7
- FR: Output corrected printtime to filename HOT 1
- Klipper estimator.exe no launch.
- Logging HOT 6
- Integrate kilpper estimator into OctoPrint-PrintTimeGenius plugin HOT 4
- FEATURE: support bonjour .local names HOT 1
- Failed to run klipper_estimator HOT 1
- FR: Rename function HOT 1
- Use config.js if moonraker is not online HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from klipper_estimator.