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

Add output of print progress / remaining time to serial interface... - or release the sources for the MKS_GD_TFT28_V4_0? #13

Open
bernisss opened this issue Jan 17, 2024 · 12 comments

Comments

@bernisss
Copy link

I'm using your latest TFT firmware for the MKS_GD_TFT28_V4_0 (Artillery Sidewinder X2), and it works very well.
I added an ESP8266 to the "wifi" serial port on the MKS_GD_TFT28_V4_0, in order to get the current printer status into my smart home.
Now, the print progress (and caculated from that, the remaining print time) is very unprecise, as "M27" only reports the current position in the g-code file. The values reported by the TFT are much more precise, but I can not access them through serial commands.
I identified how I could add this output, for example by adding a new "M27 T" command or similar to interfaceCmd.c.

I would like to try this, but I can't find the sources of the current FW in your repository, and the original BTT-TFT repo does not include the MKS_GD_TFT28_V4_0 yet. Would it be possible to publish the current version sources?

@kisslorand
Copy link
Owner

You could do a PR in the original repo and I would integrate it in this FW.
...or write here the changes you want and I can integrate that or just compile a personalized FW for you.

@bernisss
Copy link
Author

Hi Kisslorand,
thank you for your suggestions! I would prefer if you integrated it into the FW, as I don't have experience in doing PRs. and as I can't test as I only have the GD32f305-based Artillery X2 TFT.
If you could integrate and build a test version, I can try it out, though...

My suggestion is the following:
Add the following lines to /TFT/src/User/API/interfaceCmd.c at line 717, right after the
if (cmd_seen('C')) {...} block:

if (cmd_seen('T')) //New Command "M27 T" to report estimated print time and progress as shown in the TFT main screen during print over serial 2.
                {
                  Serial_Forward(cmd_port_index, "Current file: ");
                  Serial_Forward(cmd_port_index, infoFile.path);
                  Serial_Forward(cmd_port_index, ".\n");
				  sprintf(buf, "Print time: %d elapsed, %d remaining, %d total, Layer: %d/%d, Progress: %d%% done", getPrintTime(), getPrintRemainingTime(), getPrintExpectedTime(), getPrintLayerNumber(), getPrintLayerCount(), getPrintProgress());
				  Serial_Forward(cmd_port_index, buf);
                }

If the source were available, I would have tested this myself, as I'm not sure whether:

  • the data types of the getPrint...() functions are ok for a conversion with sprintf,
  • the buf (which I just reused) is long enough with buf[55].

@kisslorand
Copy link
Owner

kisslorand commented Jan 22, 2024

Would you prefer to have the time in seconds or rather in H:M:S (hours, minutes, seconds) format?

On the other hand, there's a PR that would make the original repo to support the TFT you have.

In the meantime here's a FW version that sends the time in seconds: mkstft28evo.zip

@bernisss
Copy link
Author

Thank you, this is awesome! I just tested your FW from above.
I think time in seconds is perfectly fine, as it allows easier processing in whatever system the data is used in.
The firmware seems to be good and the output string looks good too, except one bug that prevents printing:
As soon as I send a single "M27 T" I get thousands of responses (roughy 130 per second at 115200 baud...),
and this seems to spam the controler, as the print stops right away. Stopping the print on the display and hence stopping the output, the printer becomes usable again. This shows that it does not crash, but just gets spammed.
Sending only "M27" or "M27 C" is ok, but then of course we don't get the new output.
I don't see how this could happen with the code above, except maybe if the buffer "buf" is too short for the output or something goes wrong with sprintf?

Unitl now, I polled the status by M27 or M27 C only every 10 seconds, which is enough for a status output, I think.
Hearing that the GD32f305 is added to the original repo is nice, too - let's see how this develops....

Best regards and thank you,i
Berni

@kisslorand
Copy link
Owner

kisslorand commented Jan 22, 2024

I'll re-check the code later today and will get back to you.
In the meantime can you show me what responses did you get from "M27 T"?
I found the problem, I forgot to delete the "M27 T" from the command queue. Here's the new FW for you: mkstft28evo.zip.

@bernisss
Copy link
Author

oh, cool!
I'll try right away...

@bernisss
Copy link
Author

This did the trick!

Instead of one line, we now get three as a response:

21:09:00.709 -> Print time: 40 elapsed, 0 remaining, 0 total
21:09:00.709 -> Layer: 17/0
21:09:00.709 -> Progress: 73% done
21:09:00.709 -> ok

but that is fine!

Thank you so much!

@kisslorand
Copy link
Owner

This did the trick!

Instead of one line, we now get three as a response:

21:09:00.709 -> Print time: 40 elapsed, 0 remaining, 0 total 21:09:00.709 -> Layer: 17/0 21:09:00.709 -> Progress: 73% done 21:09:00.709 -> ok

but that is fine!

Thank you so much!

That's by design. "buf" was too short(55 bytes) and I broke the message into pieces. However, if needed, I can make to have all the info in one single line (just removing the '\n' from the first lines and leave it only at the end of the last info)

@bernisss
Copy link
Author

bernisss commented Jan 22, 2024

Actually, the length of the buffer was what I expected to be the problem:
Considering the datatypes of the different fields (uint23_t, 16_t and 8_t), the longest status message as a one-liner would be
Print time: 4294967295 elapsed, 4294967295 remaining, 4294967295 total, Layer: 65535/65535, Progress: 255% done
which is 113 characters long.
Splitting the lines is good and can stay as it is, but still we may need buf to be 71 characters long to accomodate the time fields for a "really" long print, in order to avoid a buffer overflow?

Apart from that, it works really well now, and is awesome!
I hooked up a simple esp8266 to the wifi serial port, which transfers the status via mqtt to homeassistant, so I can monitor the print...

@kisslorand
Copy link
Owner

kisslorand commented Jan 22, 2024

Splitting the lines is good and can stay as it is, but still we may need buf to be 71 characters long to accomodate the time fields for a "really" long print, in order to avoid a buffer overflow?

That can be split further (but kept in the same line in the response) if needed or just simply use a bigger buffer. Whichever you prefer, I'll do it.

Apart from that, it works really well now, and is awesome!

Thanks for the kind words, I am glad that my work finds home in more printers than just mine.

@bernisss
Copy link
Author

Actually, I don't know what is the more elegant way to do it - maybe sending out in multiple steps may keep the tft busy for a longer time, which we would want to avoid? If so, then increasing the buffer may be the better way to go?
I would suggest using buf[72] (one byte more because of the end-byte) for the time-part of the output...

Your FW helped me a lot! While looking for a TFT FW newer then the stock one to support newer marlin versions, your build was the one that was the most up-to-date, and I really appreciate that you are actively maintaining it!
Before doing this request, I tried to compile the BTT sources by myself but soon realized that the GD32f305 was not supported by the original repo yet, and that there are many locations where the code needs adaption, so I guess it was a lot of work to get the 'f305 working. Thank you for all these efforts!
By the way, if you need someone for testing anything, let me know, maybe I can give something back this way...

@kisslorand
Copy link
Owner

Actually, I don't know what is the more elegant way to do it - maybe sending out in multiple steps may keep the tft busy for a longer time, which we would want to avoid? If so, then increasing the buffer may be the better way to go?
I would suggest using buf[72] (one byte more because of the end-byte) for the time-part of the output...

Don't worry about the TFT being busy, I implemented the DMA transfer, so the load is lifted from the MCU.

so I guess it was a lot of work to get the 'f305 working

The work is not mine, it is ciotto who did the implementation for the GD32F305, I just fixed some bugs that it had. My extra work for these TFTs is the patching of the bootloader to remove the annoying long beep at boot and to simplify the FW update procedure (no fake files and folders needed, no need to reupload every time the bmp and icons folder).

The only testing I would need is with the newest TFT that has an ILI9488 display driver.

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

No branches or pull requests

2 participants