Skip to content

MKS UI - Fix G-code command result display #27825

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

Merged
merged 5 commits into from
May 2, 2025

Conversation

staff1010
Copy link
Contributor

@staff1010 staff1010 commented Apr 30, 2025

Description

The serial port hook gcode command result takes time to complete, so it is added to the UI refresh queue.

CC: @makerbase-mks @MKS-Sean

@@ -160,7 +160,7 @@ static void lv_kb_event_cb(lv_obj_t *kb, lv_event_t event) {
goto_previous_ui();
break;
case GCodeCommand:
if (ret_ta_txt[0] && !queue.ring_buffer.full(3)) {
if (ret_ta_txt[0] && !queue.ring_buffer.full(BUFSIZE - 1)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would require the buffer to have no more than 1 command in it, but we probably want to allow for the buffer to have more things.

Meanwhile, there may be a fundamental misunderstanding from Makerbase about how queue.inject works. It doesn't put anything into the G-code buffer right away. It sets up commands that will be injected before the regular G-code buffer. So it doesn't matter here if the buffer is "full" or "close to full" or not! The commands will be run before the next command(s) in the queue.

@thinkyhead thinkyhead changed the title Fix hook gcode commad result display MKS UI - Fix G-code command result display Apr 30, 2025
@thinkyhead
Copy link
Member

thinkyhead commented Apr 30, 2025

Thanks for the fix!

We want to continue to improve our support for this MKS display and extend support for this display to all boards. The first step is #25650 which will follow the 2.1.3 release.

The only refactor work required to modernize the MKS UI code and make this display compatible with old-school AVR is to fix all the code that handles static data so that it stores and retrieves via PROGMEM, and to use MString with setf / setf_P in place of sprintf wherever possible.

@thinkyhead thinkyhead merged commit e16885c into MarlinFirmware:bugfix-2.1.x May 2, 2025
66 checks passed
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants