Jyers / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. | Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
http://marlinfw.org
GNU General Public License v3.0
2.14k stars 387 forks source link

Print aborts if filename is too long #1522

Closed tom2199 closed 2 years ago

tom2199 commented 2 years ago

Description

Jyers throws me back into the main menu if the selected gcode filename is "long". After a few seconds, it displays the loading bar and aborts the printjob.

Steps to Reproduce

  1. Select gcode file with long name
  2. Get thrown into main menu

Expected behavior: Execute gcode

Actual behavior: Throwback into main menu

tom2199 commented 2 years ago

Printer is printing atm so I can't test, but I think maybe it has to do with spaces within the filename instead.

dhupee commented 2 years ago

I found the exact same problem.....just try to validate the level of my printer and then it won't print

Update: I downgrade it to 1.35(still upgrade from my previous flashing) and seems no problem so far, waiting for next update from contributors, it's not game-breaking indeed but kinda annoying to me

phenomeus commented 2 years ago

i have the same, I reduced the name (no spacings, just _) from 26 letters to exact 14 before .gcode, testing right now

hcgonzalezpr commented 2 years ago

I can confirm this also happens if the file is sent from octoprint using the precompiled release version on github. When sent from octoprint it causes Y axis crashing on my case, shortening the filename resolved the printer rebooting and axis crashing.

tom2199 commented 2 years ago

@tom2199 Check if you have #define LONG_FILENAME_HOST_SUPPORT in configuration_adv.h

I've tried the precompiled release and this configuration where it's enabled: https://github.com/Jyers/Marlin/blob/e1967c2c4bdab0661a65f6d7f6da3db1b5018f18/Configuration%20Files/E3V2%20Templates/BLTouch-3x3-HighSpeed/Configuration_adv.h#L1453 I've also noticed the menu gets very sluggish after scrolling through a few dozen items.

alexqzd commented 2 years ago

I can confirm, the printer crashes after the scrolling file name reaches the end of the screen

Nepcior commented 2 years ago

Same here with E3V2-BLTouch-5x5-HS-v4.2.2-v2.0.0.bin I noticed that if you switch to the "Tune" menu before filename reaches end of the screen all works perfectly, no crash.

There is one more thing that puzzles me. The nozzle coordinates at the bottom of the screen are weird. I think they used to give the position of the nozzle in mm before. Now they reach values up to 999 and then turned into 0 again.

tome9111991 commented 2 years ago

Known issue on upstream https://github.com/MarlinFirmware/Marlin/issues/22725

Jyers commented 2 years ago

@tome9111991 Thanks for linking it.

I was aware of this originally, but haven't had time to fix it yet. I need to overhaul the scrolling text a bit to match the upstream standard. For now just try to stick to short filenames

DragRedSim commented 2 years ago

Function in question: https://github.com/Jyers/Marlin/blob/a4d863898b006b7354262ea8801ab0e06b795105/Marlin/src/lcd/e3v2/jyersui/dwin.cpp#L734

Also noting similar behaviour with long filenames if hovering over them on the select file menu; the printer will scroll the name off-screen, then reboot (probably due to the code throwing an exception). This appears to be the issue described in the upstream bug, occurring on the file select menu, and is occurring within the File_Control function (https://github.com/Jyers/Marlin/blob/a4d863898b006b7354262ea8801ab0e06b795105/Marlin/src/lcd/e3v2/jyersui/dwin.cpp#L4700)

EDIT: Also linking in another function which uses scrolling text that will need to be refactored to upstream standards: https://github.com/Jyers/Marlin/blob/a4d863898b006b7354262ea8801ab0e06b795105/Marlin/src/lcd/e3v2/jyersui/dwin.cpp#L1018

Tapap84 commented 2 years ago

Same. 2.0 4.2.2 3x3 manual. Unable to print anything. I can only assume it's the file name. The default names like "CE3PRO_SmartTemperatureTower_PLA_180-225" Will abort. The only file that did print was "CE3PRO_9"

DragRedSim commented 2 years ago

I think we’re onto something here. The _MIN macro is defined in two ways. The first is the C++ variant. It’s designed to take two variables, figure out a variable type, and returns the lower value; but the method in which it determines the variable size to return (to ensure that the variable size can hold any member of what was originally offered as an argument) is done by adding the two variables together. In this case, we have a size_t, and an int. But what sort of variable is it returning (to avoid overflow of the sum), and is it then castable to the int8_t we are trying to store it in? This could lead to odd behaviours;This may be causing an exception and thus a watchdog restart. https://github.com/MarlinFirmware/Marlin/blob/a185ce22cf6e4fb15250815c5c39318606a7e65a/Marlin/src/core/macros.h#L376 Second is the non-C++ version. It starts by figuring out how many variables to compare (using preprocessor directives to “push” a list of variables around such that the result we want is an argument to that macro function); if it’s two, it will compare them using the classic ternary function. (If three or more, not applicable in this case, it recursively compares two at a time.) At no point before the ternary function is executed are the variable types checked, which can lead to undefined behaviour if there is no comparison of the value types available or defined. Again, exception, watchdog restart, and failed prints. https://github.com/MarlinFirmware/Marlin/blob/a185ce22cf6e4fb15250815c5c39318606a7e65a/Marlin/src/core/macros.h#L481

IrishBrewer commented 2 years ago

This is an issue for me too. ALL my filenames are long as I use that to distinguish between various versions of the same base model that may be sliced for different nozzle sizes, filament type as well as slight variations on the model itself. Going to short filenames will result in names having to be very cryptic requiring a cheat sheet to decode or lacking in detail altogether so hopefully, this can get fixed soon.

IrishBrewer commented 2 years ago

Same here with E3V2-BLTouch-5x5-HS-v4.2.2-v2.0.0.bin I noticed that if you switch to the "Tune" menu before filename reaches end of the screen all works perfectly, no crash.

Thanks for the workaround - trying it now. I assume you can't exit the tune menu going back to the main screen or the print will abort since it will again try to display the long filename, correct?

sonjamichelle commented 2 years ago

So what exactly is the "maple" version? I've seen that in VS-Code, but haven't found any info.

Unfortunately the long name bug is somewhat crippling to my workflow. I have a rather precise naming convention that allows me to differentiate the gcode between printers, infills, nozzle sizes, resolution, etc.

I haven't noticed any other bugs on this release. For the most part my Ender 3v2 works fairly well save for the long names.

sonjamichelle commented 2 years ago

@tititopher68-dev I tried compiling your linked source. In VS-Code I selected the maple build option. When I upload the bin to the printer and tried a home sequence, for lack of better words, the printer sort of "flipped out" It didn't home.

I MAY have made an error in one of the config files. I am running an Ender 3v2 with a 4.2.7 board.

In the config it just says:

// Choose the name from boards.h that matches your setup
#ifndef MOTHERBOARD
  #define MOTHERBOARD BOARD_CREALITY_V4
#endif

Looking a little deeper I see that I have to append a 27 to that line.

Once my current print job is complete I'll upload a new compile.

Jyers commented 2 years ago

@DragRedSim You were exactly correct in it being a type issue.

Looking at the old code, I realized that before using the _MIN function, pos was cast to size_t before len is set to it. However, the _MIN function has determined the best output was an int... particularly a signed int. This was creating our issue.

By casting pos to a size_t, _MIN now outputs an unsigned int which has eliminated the crashing.

This is now fixed in the source and will be in the next release

mrrstrat commented 2 years ago

I can confirm this bug as well: the 2.2.0 default choked on "E3V2-Chain_Link_Cover_print_and_Mounts.stl". When selected it would start to print but then exit print menu. I did have the max speed go in excess of 700 despite being set back to 60 twice. In a brazen demonstration of poor judgement I tried to print after attempting to correct the settings in the UI. Bad idea. But nothing was damaged except the tip of a nozzle. Re-flash and changing file names to be short completely corrected the issue. I figured it was a dangling pointer or unbound reference. A mis-cast certainly would cause this if the numeric precision/storage was truncated for the intended stored value.

IrishBrewer commented 2 years ago

If only that was the only bug .... and no, there are others alas. I have a little higher in this thread a fix for this issue.

I will compile my own firmware as suggested. I've done it before on my E3-Pro where you also had to install a bootloader. I'm thinking though that the following statement in the latest release should probably be reworded as long filenames causing failure to print would be considered a game breaker for most.

Important: This release may have some major bugs carried over from upstream, nothing should be game-breaking, but be aware of this before you decide to update. For some it may be better to wait for the next release.

IrishBrewer commented 2 years ago

I advise you to learn how to compile it ... Take a look here : #1511 (reply in thread) One compiled, the file will be in .pio\build\STM32F103RET6_creality_maple Don't be scared, it will turn out well and we will always be there just in case ;-)

Got it compiled for Manual Mesh Bed Leveling 5x5 and installed on the V2. Thanks!

One question - what is the difference between 1.3.5b and (v1.3.5.b + fixes)?

mrrstrat commented 2 years ago

I have the 2.0.0 code compiled and installed on my heavily modified Ender 3 V2 w/CR Touch. I like the Jyers spin of the firmware and could not bear on using the stock Creality while getting my CR Touch working. I needed the control over the machine to get the CR touch on-line afforded by the Jyers version. All working well and with short file names until a solid fix(es) has been identified.

Jyers commented 2 years ago

This is fixed in the latest release :)