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

Fix bugs in the file selector screen and icon not being displayed correctly #31

Merged
merged 1 commit into from
Dec 28, 2020

Conversation

X-Ryl669
Copy link

Requirements

This is a general MKS LVGL code update, not specific printer is required

Description

When using the WIFI's Cura plugin to upload a gcode file, the resulting file breaks the printer when listed on the SD-card.
Opening the file show that they are many difference between what the printer's firmware expect and what it gets.
For example, the file I get (see attachement) crash the printer when asked to list the file (in the "Print file" menu). It happens because the small image is 50x50 in the file and the code expect a 100x100. It also happens because the firmware is unable to deal with different new lines characters (which happens when using HTTP to send file) and expect fixed position data so it's seeking in the middle of nowhere when fetching a line.

This pull request replace the image parser with one that is a lot more resilient and it's able to deal with both 100x100 small images and 50x50 (in that case, it's zooming the picture by a factor 2x2).

In a future work, I'm wondering if it would not be even better to downscale the big picture (gimage) instead of upscaling the small image. In case you accept this change, I'll try to deal with this case as well so the displayed picture is top quality.
Also, I think the code that's creating a fake .bin file for LVGL is completely useless, it would be better to throw this code completely and let LVGL display the ".gcode" directly, that would reduce the flash code size and memory requirement by a lot of bytes (since you don't need to store temporary .bin filename anymore, you don't need seeking/patching code, you don't need special cases, etc...) but this is not part of this patch.

Benefits

With this PR, the printer no long crashes.

Related Issues

See attached file that's generated by Cura with MKS Wifi plugin and place it in the SD card. It'll crash the printer immediately when listed.

SP_MU~1.GCO.zip

@MKS-Sean MKS-Sean merged commit 89a4a2e into makerbase-mks:bugfix-2.0.x Dec 28, 2020
@rhapsodyv
Copy link

The old parser already handled both image sizes. It got broken in some changes in the middle of the process. I don't know what this PR is fixing, but the fix for the gcode preview is far far far simpler than all this changes: https://github.com/MarlinFirmware/Marlin/pull/20564/files

@X-Ryl669
Copy link
Author

@rhapsodyv Please try the attached gcode file in this issue. Unzip on a SDcard and try to list the files on your printer. On my printer it crashes and the fix you're linking to is unlikely to fix it since the picture size decoding is wrong in the current code.
The current code expect a line of 400 chars per simage, so it's 200 bytes (HEX->byte), so it's 100pixels. The picture I have are 200 chars per simage (so 50 pixels per line). The current code seeks to the middle of the next line after decoding the first line (see here:

sd_read_addr_offset = sd_read_base_addr + (pos - 4) / 200 * 409; // This is wrong, at least for my file since there are 2 M10086 headers per line

And since it expects to jump at a beginning of a line and reads fixed size, it can't find the end of the picture ;;gimage and continues decoding whatever it finds in the public_buf largely overwritting the buffer size, completely crashing the printer.

This code does not make the assumption the line are fixed size, and does not seek anywhere. Instead it's able to deal with different format (100px lines and 50px lines) and also zoom in that case. It's able to resume if it's seeked at the wrong position.

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.

3 participants