PFW-898 : Linear Advance 1.5 Returns #1909
Conversation
This discards several Prusa optimizations for LA1.0. We'll re-implement those later if needed. Debugging is turned on.
Use _NEXT_ISR and st_reset_timer to correctly reinitialize and re-schedule the advance ticks.
Initialize at 0 both on startup and on reset on regular (non-LA) segments to avoid cumulating errors.
This reverts commit aae03ad.
|
@DRracer Thinking about this, I would reserve a larger size for the sheets struct. Even though we effectively use only 3 currently. If we ever need them, we already have a good struct. Otherwise, it will be actually easier to carve a hole in there than to put another sheet struct somewhere else. In fact, I would do it like that: for exactly this purpose. Hopefully there's still time to do this. |
|
@wavexx ok, 3 sheets may seem not enough. We can rise the default after we receive some more input from ordinary users. |
|
Won't it cause problems if it is added as the last part of the EEPROM (since it will be moved if other things gets added before it), and if it is added in the middle, won't changing the number later on cause problems for everything located after it in the EEPROM? :) |
I know, but I was thinking of keeping the same offset in the eeprom and reducing the number of sheets without having to change the offset to active_sheet. This will keep the original values of the first sheets. We can also move the offset downwards too. But then we should count sheets starting from the end as well (I didn't check if this is already the case). |
|
On Mon, Aug 05 2019, GurliGebis wrote:
In other words, once it has been released, won't it be difficult to
change the number of sheets to allocate room for?
It's going to be annoying anyway :). But reserving more space from the
start is less problematic, because if we need to grow we'll have to
split the storage in two zones, and that's going to require more code
(something that definitely we're trying to avoid).
That being said, what's the effective available eeprom space left?
The current SHEETS_BASE is at 3456, so I would just reserve space for 8
sheets and think about this later (and possibly never).
|
|
Prusa should have an idea on how many sheets people should have (since they have been shipping them to people), and then multiply it by two (since a sheet has two sides, and they are not identical) - that should give a number that should fit most people :) |
|
Maybe I'm not a typical user but I have two smooth PEI, a textured PEI, and another with BuildTak on one side and kapton tape on the other. My initial three slots will be filled with more needed already. |
|
I have smooth PEI sheet from the original MK3 (on which I made two holes recently, but to be honest I can still claim warranty on it, since it is rusted as hell ;-) ), I have another smooth PEI, newer one with black coating which was bought, so that I could abuse the rusted sheet with PETG without fear of breaking it apart completely (but that never happened because I had no time for long period), and finally I got the textured sheet that I bought with the MK3S update, where I print PETG and some TPU (both so far didn't destroy it - knocks wood). The rusty sheet is now reserved for 0.15 nozzle I played with recently (doesn't print well, it prints GREAT), since I don't dare to use 0.1 first layer on textured sheet. |
|
So basically I have a one good textured sheet and two smooth sheets, one old and rusty, other one new. I will for sure buy at least another textured sheet for backup. Also it looks like that 0.15 nozzle is a bit shorter, thus it would be even nicer to have different live Z values per nozzle. |
|
As a tip, you can (should?) use the Z offset in the print settings when switching nozzles. Calculate the offset using the LiveZ difference, but restore the original value and use the difference in slic3r instead. This will allow to keep the all the same sheet values in the printer every time you do a swap, as you are essentially moving the gantry height. |
|
I thought about it, but I think that this would kind of mess with the Z values that printer reports (assuming that the Z offset is just added to all G0/G1 gcodes), and since I use the rusted sheet anyway only for that nozzle, I just factored this into the live Z value of that sheet. Plus with 0.15 nozzle, the live Z has to be really spot on, so I will probably have to re-calibrate after I put that nozzle back anyway. btw, I put my profiles on my github, so if you want to play with 0.15 nozzle, feel free to use them https://github.com/maximlevitsky/PrusaSlicerSettings Apart from the very good settings found at There is a need fro two gcode tweaks, one is to have gradual purge line so that extruder doesn't have to suddenly jump from the thik purge line to the super thin regular line (this makes skirt not to attach to the bed) and another tweak is that you need to offset the home position, because it is hardcoded at Z=0.15 which makes the printer ignore slicer attempt to print the first layer at Z=0.1, it just clamps this to 0.15. One day when I have more free time, I'll write more detailed review of all things done. PS: The 10% benchy on e3d blog is a lie - it is 22% benchy - its trivial to verify it. I manged to get as low as 12% percent with reasonable results. |
|
And of course back to the topic, since for 0.15 nozzle, the amount of filament extruded is such a tiny amount, I wonder if LA 1.5 will improve the quality even further. I have stock MK3S extruder btw. |
|
Been running these LA1.5 branches for quite some time now, and it seems to work flawlessly. I probably still need some better per-filament tweaking of K-values, but I really like the results! |
Allow existing gcode using LA10 to transparently take advantage of LA15 by using a simple linear conversion function based on experimental results with the MK3 implementation of linear advance. Autodetect LA10 values based on the first M900 instruction contained in the print. In order to support printing mixed files without resetting the printer we also reset the autodetection status when starting a new SD print and/or when explicitly disabling LA. Since we cannot reliably detect whether a new print is started when printing via USB, also reset the detection status when homing in G28, which is generally performed once at each print. Note that this doesn't clear the previous K value, it only allows a subsequent M900 to provide LA10 values when printed after a LA15 file.
|
In the last commit I added a compatibility layer that allows to print files sliced with LA1.0 to print fine with LA1.5, hopefully smoothing the transition across versions. The conversion function is not as perfect as I would have liked it to be. At least with this version you'll only get optimal results when supplying/reslicing with an exact LA1.5 value. That being said, it should still provide a printed result which is close and/or identical to a stock profile. There are many factors that influence the conversion between LA1.0 and LA1.5 that make this more cumbersome than what it should be:
To detect whether the K value is LA1.0 or LA1.5 we use K>=10 as a boundary, since the default LA1.5 implementation ignores these values entirely, while the LA1.0 shows no appreciable effect below 15 already. It would still be helpful to know if anyone has been using K values lower than 10 during the MK3 lifetime, since the threshold could be lowered comfortably down to 5. I'd avoid doing that to keep as close as possible to the official Marlin LA1.5 implementation though, which would add even more confusion as of which version of LA the firmware actually supports. Any user which has already updated the filament gcode to a LA1.5 value does not need to do anything. However the stock profiles in PrusaSlicer need to be updated to include a LA1.5 value for improved results. The right way to support old and new firmware releases is to supply both values in the following order: A LA1.5 firmware without this module (such as an official Marlin release) will only parse the first correctly, rejecting the second. An older LA1.0 firmware (MK3 or Marlin with LA1.0) will parse both, but keep the last value instead. Since this is specific to the MK2/MK3, the compatibility stub can be disabled entirely by |
Since the advance factor is computed per-segment in LA15, there's no need to stop the planner. Allow changing K freely at each segment. This allows varying quality factors for different filling roles, see: supermerill/SuperSlicer#108 During pause/resume/crashdetect or powerpanic K might temporarily be out of sync when used this way. If this becomes an issue, we might need to store K for each block, as done for the feedrate.
In preparation for prusa3d#2161, use MBL (G80) as a "new print" boundary instead of just re-homing to ensure the reset is issued only once for each print. Similarly, do not reset the autodetection when LA is disabled via M900 K0. This can/will be used during a print if different quality settings are used for different filling roles.
Properly check K independently for each version by delegating it to la10c_value() Handle -1 as a special case to allow manual reset.
DRracer
changed the title
|
@wavexx so I did a LA15 test on my MK3S/MMU2S printer w/petg and found the best value to be 0.14 to 0.15 for LA15. But according to the update notes, the firmware will ignore K values higher than 0.10 for LA15. What do I do now? edit: wait.. maybe i read it wrong. It ignores value of >10.0, not 0.10. Right? |
|
It's >10, so you're ok. The values are the same that Marlin is using now. |
This is a revised implementation of the Linear Advance 1.5 algorithm for the MK3. This supersedes the original implementation by @Sebastianv650 (see #504).
Linear Advance 1.5 is the successor to 1.0 (also developed by Sebastian) and provides several core advantages over the original implementation:
For a more detailed explanation, please see the original documentation:
http://marlinfw.org/docs/features/lin_advance.html
Although the version of Linear Advance in the MK3 is a slightly improved version over the original, all the above points still hold and this implementation is both more efficient and provides superior print quality.
Thanks to the community and to @vertigo235, this has been tested on several printer variants, including the MK2.5/2.5S/3/3S:
https://github.com/vertigo235/Build-Prusa-LA-15/releases
Linear Advance 1.5 needs an updated K value, which is now expressed mm/mm/s, generally ranging from 0.01 to 0.2. Because the updated firmware intentionally ignores K values >= 10, it's actually possible to support both LA1.0 and LA1.5 in the same filament profile by specifying the LA1.5 K value first, and the LA1.0 value second:
Although the work started roughly on March, the PR has been rebased on commit 098c097 and I've been constantly working by merging the changeset on top of master to ensure that all changes work as intended on the firmware as of today.
I encourage you to read the original PR (#504) and the feedback we collected on the current PR:
wavexx#1
Because this PR changes several core parts of the stepper ISR, and to justify such changes, what follows is a detailed technical description so you can evaluate them in context. Please don't hesitate to ask for more if needed.
Let's start with some performance stats. This is how LA looks in the original commit 098c097 (top of the image) and how it looks with LA1.5 on a MK3 (1_75mm_MK3-EINSy10a-E3Dv6full.h):
In the image, CH1 has been hooked to X_MIN_PIN, which is being used to measure the entire length of the TIMER1 interrupt (the stepper isr). Note that I tried to match the time scale, but they're not exactly the same.
It's the beginning of an acceleration loop which makes use of linear advance.
The GCODE I used for testing (of which you see the very few pulses) is the following:
with the difference that for LA1.0 we use "M900 K30" (the optimal value for PLA) and "M900 K0.05" for LA1.5 (which is again the new optimal value for PLA).
From the image you can immediately see that a regular isr which includes XYZ movements is significantly shorter. There's next to no difference between the first acceleration step and any subsequent ones. The speed at which advance steps are delivered is slower thanks to the jerk limit, while the interval between each is precisely controlled resulting in a much smoother acceleration curve.
A full isr in LA1.0 takes north of 90µs, while it's generally 65µs in LA1.5. The advance ticks take slightly longer, going from 9µs to 12µs.
In the test gcode, both versions spend ~17.5% of the time in the isr giving a comparable load, but on one side LA1.5 delivers 12196 ticks spending an average of 11.1µs-per-step, while LA1.0 only delivers 12095 (11.5µ/step).
In LA1.5 we never schedule advance steps at a higher frequency than
MAX_STEP_FREQUENCY. We work in sync with the regular stepping policy, by performing dual and quad-stepping as needed. Still, for the speeds that the MK3 can operate, this never happens in practice. For this reason, I believe LA1.5 solves the serial communication issues that plagued LA1.0 as the time spent in the isr() will not grow exponentially as K is increased but will remain essentially constant. More on that later.Code wise, we go from 253430 to 253330 with LA1.5. The PR also includes the ability to change K from the Tune menu, which has been invaluable for debugging but has been disabled from
Configuration_adv.has it takes additional space:Size summary:
The implementation of Live K /can/ be improved though by making the existing menu functions a bit less specialized instead of introducing new ones as I currently do.
Code changes:
In the GCODE, the "R" E/D ratio is ignored and always internally calculated. We only keep K when in the interval [0-10), meaning we can support both old and new profiles as described above.
Code changes in ConfigurationStore, Configuration_adv.h, Marlin_main.cpp, planner.cpp, planner.h mostly deal with the new logic and are well isolated through ifdefs. You can still build without LIN_ADVANCE and it does work as intended. I've commented the code where needed, it's more compact and I'm not going describe all lines here (ask if needed).
In stepper.cpp I've moved out the function
calc_timer, MultiU24X24* and moved the lookup tables as well (which are identical) intospeed_lookuptable.*with proper declarations. It's a lot cleaner, but the reason I originally did that is that I needed anther copy to compute the advance step interval inside the planner and later removed it to save extra space.calc_timeris still inline, but now accepts step_loops as a parameter instead of changing a global. There's no difference in the generated code, and although not strictly necessary, I would recommend to keep it as it is.In Marlin_main we remove the extruder_under_pressure flag. It's useless, and conflicts in terms with LA.
We now also store/reload the K value in the eeprom when a powerfail condition occurs, as it affects the print quality and it's not generally repeated in the gcode more than once. This should have been done also with LA1.0.
In menu.* we remove some of the static keywords as we re-use some more functions to implement LIVE_K. Again, this is not strictly necessary, but I don't see any harm even when LIVE_K is not enabled.
In stepper.cpp I still preserved two checkRx calls, of which the second one has been moved a lot earlier in order to be run roughly in the middle of the isr:
https://github.com/wavexx/Prusa-Firmware/blob/c40e3b550dde8118d1c1ba8c7e0662c09e9913cf/Firmware/stepper.cpp#L864
This has been just a defensive move on my side though. I do believe neither of these calls are required now as there's always time to drain the rx buffer outside of the isr, and removing them would save more space. This should hold also when dual/quad-stepping, but I have no means to reproduce this scenario.
Since you are in a better condition to test this, I would recommend to remove the calls and test on your farm. At the very least, I would consider integrating wavexx#2 by @thess instead. I didn't have time to check if it introduces too much variation/delay in the pulses, but it's certainly a superior solution to checkRx and something we can work on as a further improvement.
A big chunk of changes involve the PAT9125 optical filament sensor of the MK3. As the optical sensor tracks the filament, and LA1.5 changes the way the filament is fed, some changes were unfortunately necessary.
First, in fsensor.cpp during initialization we do call a full update instead of
update_y(). This populates the sensor stats in the menu so that we can see the initial state even during a print. I removed all direction checks originally present in stepper.cpp: the original code assumed that the filament direction would always move in the same direction as the direction assigned in the block, but this is not true. Both in LA1.0 and LA1.5 the filament can reverse direction while decelerating. For this reason we simply keep a signed counter (that was already present), and trigger the sensor after any esteps have been performed. This is both simpler and much more accurate. The internal code in fsensor only checks for forward anyway, but it does that taking into account all the retractions correctly.There's no longer a difference between st_block_chunk and st_block_begin. I kept the distinction only for clarity and because it might come in handy if the block direction will eventually be ever taken into account in the future (you could ignore long retractions).
I still think the error counters in fsensor.cpp (FSENSOR_ERR_MAX specifically) are too lax. In your original code, FSENSOR_CHUNK_LEN*ERR_MAX would imply that a runout condition can only be detected after the filament has dropped 10.88mm below the sensor window, which happens when the filament stops halfway through the gears. If the filament is tracked correctly, this would very well be too late.
For discussion, on my firmware build of the MK3 I intentionally reduced this to ~6mm to test how accurate the filament sensor tracking works:
wavexx@510e760
You can also see above that I changed the way the dY error conditions are counted. I assume you wanted to detected a runout earlier in the condition when the filament is missing and a negative direction would likely be a false detection. But I suspect that this happens exactly as likely as non-moving filament instead, which would inflate the error count for no reason, requiring a higher threshold just to handle a slow-moving filament. With both changes I still had no false trigger after days of printing in various non-clear PETG. These changes are not part of the PR, but also suggest that the tracking is working accurately and as expected.
Finally, because there might be tweaks that I'd like to perform later on, I would be glad to discuss any required change before a merge. I'd also love to keep the entire history as it is, as commits provide important clues for each specific line that I don't want to lose.
I'd like to thank @Sebastianv650 yet again for developing the concept of linear advance. It is truly marvelous. Ping @thess, @3d-gussner, @vertigo235, @leptun, @bubnikv (if somebody knows who else should be pinged, please do).