[FR] ADVANCED_OK support, base code implemented already

[rondlh]

Marlin supports ADVANCED_OK, which returns info about the available buffers when acknowledging a command with "ok".
This is currently not used by the TFT, which sends 1 command and waits for the "ok" response before sending the next command.
I suspect that this is the reason for some random pauses and hesitations when printing #2761

Marlin support BUFFER_MONITORING, which can give you information about the buffer underruns, a buffer underrun occurs when Marlin is ready to process the next command, but no command is available yet. I found thousands of buffer underruns per hour. These underruns are likely to be of very short duration, but clearly that is not what you want. Some buffer underruns are normal and unavoidable because the TFT is holding back data during the heating phases, this accounts for about 100 underruns in my case.

I made a rudimentary support of ADVANCED_OK which works as follows:

1. When you have ADVANCED_OK enabled in Marlin, then Marlin responds to commands like this "ok P31 B15". The B-parameter indicates the amount of available command buffers.
   In parseACK.c the "ok" acknowledgement is processed. I added a check for the "B" parameter. The parameter value is stored in the HOST struct (defined in main.h).

#define BUFSIZE 4
// "ok" response handling
if (ack_starts_with("ok"))  {
  infoHost.wait = false;
  infoHost.AvailableBuffers = MIN(infoHost.AvailableBuffers + 1, BUFSIZE);
  // if regular "ok\n" response or ADVANCED_OK response (Marlin) (e.g. "ok N10 P15 B3\n")
  if (ack_cache[ack_index] == '\n')
    goto parse_end;  // there's nothing else to check for  

    if (ack_continue_seen("P")) 
      if (ack_continue_seen("B")) { // ADVANCED_OK response
        infoHost.AvailableBuffers = MAX(ack_value(), 1);
        goto parse_end;  // there's nothing else to check for
      }
}

AvailableBuffers is initialized to 4 (Marlin's default) when the printer connection is established. (parseACK.c)

  infoHost.connected = true;
  infoHost.AvailableBuffers = BUFSIZE; // Marlin's default
  requestCommandInfo.inJson = false;

2. In interfaceCmd.c I changed the 2 sections with the sendCmd. For each send I deduct the buffer counter, and pause if no more buffers are available.

if ((infoHost.AvailableBuffers > 0) || !infoHost.connected)
  if (sendCmd(false, avoid_terminal) == true)  // if the command was sent
  {        
    infoHost.AvailableBuffers--;
    if (infoHost.AvailableBuffers > 0) {
         infoHost.wait = false;
         Delay_ms(2); // Allow a little bit of time for Marlin to process the commands
      }
     else
       infoHost.wait = infoHost.connected;
  }

Now there are no more additional buffer underruns during the printing process and at least basic operations seem to work as before. (Move menu works, terminal works, adjusting values during printing works).

I think the current TouchScreen FW can even be improved significantly without using ADVANCED_OK by just assuming that at least 4 command buffers are available (can be made configurable), and keep track of the response count ("ok"). So you can send 4 commands, but then need to wait for an "ok". If an "ok" comes then another command can be send. This should solve the pauses and hesitations.

Update: I did some tests to see if this can work without having ADVANCED_OK enabled. It's possible, but a bit complicated by the fact the command and response count could be undefined at startup. Some commands could be send that are never acknowledged by Marlin.

UPDATE: Source Code files attached (based on the (currently latest) June 2023 release), just overwrite the 3 attached files. It will work both with ADVANCED_OK enabled and disabled in Marlin. If it's disabled then the default amount of 4 buffers is assumed. Make sure to increase RX_BUFFER_SIZE (Configuration_Adv.h) to at least 256 bytes, but 512 is recommended.
Updated Download: #2824

[carlosjln]

Wow I finally stumble upon the right track. I have recently rebuilt my delta and I started to notice these blobs and pauses. Even when I dial the feedrate/speed to 75%.

I'll keep an eye here :)

[carlosjln]

As I shared here #2761 (comment) -- BAUDRATE 57600 seems to have solved or greatly improved the situation 🤔

[rondlh]

As I shared here #2761 (comment) -- BAUDRATE 57600 seems to have solved or greatly improved the situation 🤔

Certainly too high baudrates can cause issues, I run 3 printers at 500K, but 1 printer doesn't work well if I don't go down to 250K. Note that at 57K, you can only send 5.7K bytes per second, so if the gcode command is 57 bytes long, then you can only send 100 commands per seconds and basically at least have a 10ms delay between each command (the TFT waits for an "ok" after each command before sending the next command).
You can give the code above a try, enable ADVANCED_OK and make sure that RX_BUFFER_SIZE is at least 256 bytes, or more. I use 1024 bytes. The code is still experimental, so don't play around with the display too much during printing, which is good advice in any case! I have great results with it and can see that the buffers are almost full the whole time.

[carlosjln]

@rondlh thanks for that info, I'll recompile with those settings tomorrow and try it with more customer prints as they come in.

[rondlh]

@rondlh thanks for that info, I'll recompile with those settings tomorrow and try it with more customer prints as they come in.

The code changes are minimal, just pickup the ADVANCED_OK feedback (parseACK.c) and do some buffer administration in interfaceCmd.c. Just copy and replace the 3 files in the July 2023 release.
UPDATED DOWNLOAD: #2824

The code is a bit different than above because it also handles the case that ADVANCED_OK is not enabled. I recommend to enable ADVANCED_OK, that's your safest bet. Note, there is no Reprap support added, only Marlin.

[rondlh]

@carlosjln Any results? Questions?
I did some more tests, and found all is working very well, both with ADVANCED_OK enabled and disabled.

[husakki]

I will also try the code changes out that you suggested, because even after the four buffer changes and lowering the baudrate to 57k I just encounterd the issue a couple of minutes ago while printing. Though I must say my TFT is not running of the newest FW because I dont have a full size SD Card currently.

I've been having these freezing issue for a couple of months now and been following on whats happening here on github, since the freezes occured the fun for 3D printing has been drasticly lowered because I'm kinda scared to print longer than 3h because of a sudden halt.

Few things I noticed aswell, after the buffer and baudrate changes, when I save to eeprom during printing the printers halts for a second or so and sometimes not only does the printer freeze, when it does, the extruder starts spinning fast too and unloads the filament.
Also ADVANCED_OK has always been enabled in my marlin.

I'll let you know after I've done the changes and tested it properly.

[Nerdcaught]

Hi, I would like to implement this Test, but whenever Im replacing the files and compiling, I get this error.

[rondlh]

@Nerdcaught Sorry, there was a missing opening bracket in interfaceCMD.c
I updated the download above, please try again.

[Nerdcaught]

Between you and me I wont officially know the true difference but if I have any hiccups Ill let you know. It compiled with your new updated files. Thank you!

[rondlh]

Between you and me I wont officially know the true difference but if I have any hiccups Ill let you know. It compiled with your new updated files. Thank you!

Great, make sure to enlarge RX_BUFFER_SIZE in Configuration_adv.h, I recommend 512 or 1024.
Enabling ADVANCED_OK is useful too, it will work fine without it, then it just assumes there are 4 buffers and keep track of the buffer use manually.

[digant73]

I can provide this solution on a PR. That feature will be configurable at runtime (Advanced OK feature ON or OFF). For backward compatibility (also with mainboard fw different than Marlin), in case Advanced OK feature is disabled in TFT then the current logic (one gcode at a time) will be used.
Eventually., this PR will include (if confirmed by the ongoing tests) also the fix discussed for #2761.

[carlosjln]

I've been using the pre-compiled version I was shared some weeks ago and so far had two freezes, one at normal speeds on a big print, but I was able to restart the printer and resume it. The other one I increased the speed to 150% just to mess around as the print was not coming well and I was going to stop it anyways.

I think I'll try the 512 or 1024 buffer config later.

[rondlh]

Implemented in #2824
Thanks go to @digant73, great job!

[digant73]

it is better you don't close a FR o BR once a PR is available. PR marked this FR as resolved (you can see that from the number linked in Milestones column) so it will be automatically closed once the PR is merged. Just to let github makes its work.

[rondlh]

I thought it was merged already... but it is not..

[rondlh]

I've been using the pre-compiled version I was shared some weeks ago and so far had two freezes, one at normal speeds on a big print, but I was able to restart the printer and resume it. The other one I increased the speed to 150% just to mess around as the print was not coming well and I was going to stop it anyways.

I think I'll try the 512 or 1024 buffer config later.

@carlosjln I'm not sure what pre-compiled version you are talking about. I did not provide one, and as far as I know there is none for ADVANCED_OK, but there is one for the hesitations issues. The hesitations issue should be solved in the current release (2023-08-24).
#2824 Adds ADVANCED_OK support to the firmware, but it is not yet merged. (You can still get the files, do you know how to do that?)
For RX_BUFFER_SIZE, if you have 4 command buffers (BUFSIZE in Configuration_adv.h) then 512 bytes is already beyond the maximum theoretical requirement, and practically you only need less than half ((BUFSIZE - 1) * 50 bytes).
You can enable RX_BUFFER_MONITOR (Configuration_adv.h) if you worry about buffer under runs, then Marlin will inform you if it is running out of buffers.

[carlosjln]

Sorry I mixed up issues because I'm tracking this other conversation #2761

And I was referring to these pre-compiled versions https://github.com/kisslorand/BTT-TFT-FW

[rondlh]

And I was referring to these pre-compiled versions https://github.com/kisslorand/BTT-TFT-FW

OK, that is what I guessed, unfortunately I don't know what is changed in this FW, but it seems to solve the hesitation issue too. I recommend to just go for the latest FW or even add the ADVANCED_OK update.

[carlosjln]

Yes, that's what I did. Last night I installed the current release you mentioned (I didn't realize the repo update) and did a couple of prints and didn't see anything out of order.

I will try a dry run print of a vase at faster speeds later on.

[rondlh]

@digant73 I just tested #2824, it doesn't seem to work for me. Marlin has ADVANCED_OK enabled, TFT too, tx_slots is set to 8 (also on Marlin), but I can see that Marlin never needs to queue more than 1 command.

[digant73]

@digant73 I just tested #2824, it doesn't seem to work for me. Marlin has ADVANCED_OK enabled, TFT too, tx_slots is set to 8 (also on Marlin), but I can see that Marlin never needs to queue more than 1 command.

yes I also saw that. Furthermore, printing from onboard (Marlin) I see 0 tx_slots and not possible to stop/control the print.
PR converted to draft just to solve the issue

EDIT: just fixed a check I changed in the past. Please check if it works now

[rondlh]

@digant73 I just tested #2824, it doesn't seem to work for me. Marlin has ADVANCED_OK enabled, TFT too, tx_slots is set to 8 (also on Marlin), but I can see that Marlin never needs to queue more than 1 command.

yes I also saw that. Furthermore, printing from onboard (Marlin) I see 0 tx_slots and not possible to stop/control the print. PR converted to draft just to solve the issue

The problem is in interfaceCmd.c, infoHost.tx_count != 0, so if only 1 command is send it's TRUE already. The condition should be something like:
infoHost.tx_count >= (tx_slots - 1)
tx_slots as defined in the config.ini

bool isNotEmptyCmdQueue(void)
{
  return (cmdQueue.count != 0 || infoHost.tx_count != 0);  // if queue not empty or pending command
}

[digant73]

just edited my previous post with the same issue (I remembered that I changed that condition in the past).
Fix already committed.
I'm also thinking to add a menu to monitor some TFT parameters (e.g. tx_slots, tx_count, queue command count etc...).

[rondlh]

just edited my previous post with the same issue (I remembered that I changed that condition in the past). Fix already committed. I'm also thinking to add a menu to monitor some TFT parameters (e.g. tx_slots, tx_count, queue command count etc...).

Condition infoHost.tx_slots == 0 seems to solve the issue, I'm testing it right now, but looks good so far.

I personally monitor this from Marlin side, not from TFT side. But adding some debug code could be useful.

BTW:

1. With your flag update in Serial_Get, you can update the wIndex every time in Serial_Get, no need to wait for the serial idle interrupt routine anymore, it can be completely removed.
2. I have DMA sending of data working (STM32F2_F4 only), it works very well. M43 now works without buffer overrun, but I still cannot get the output of M503 on the ESP3D, it's related to knownEcho in parseACK.c. I'm not sure why the messages are not forwarded.

[digant73]

last cleanup in main.c. PR ready for merge.

[rondlh]

1. Is this still valid? ([Q] Random pauses or hesitation when printing. #2761)

• ADVANCED_OK OFF -> tx_slots is always used. e.g. if 1 -> current situation, if 2 -> static A.davanced OK (improved but conservative)

• ADVANCED_OK ON:

  • tx_slots is used if ADVANCED_OK not enabled in Marlin
  • Marlin indication is used if ADVANCED_OK enabled in Marlin

If so, it seems not to be working for me, ADVANCED_OK enabled in Marlin(BUFSIZE=8) and TFT(Reset default settings, but ADVANCED_OK enabled). I can see that only 1 command is queued anyway, I would expect 8.

2. This section in Configuration.h is a bit odd:

 *   Value range: [min: 2, max: 16]
 */
#define TX_SLOTS 1  // Default: 1

3. The HOST_SLOTS enumeration seems a bit odd to me:

typedef enum
{
  HOST_SLOTS_GENERIC_OK = -1,
} HOST_SLOTS;

4. The config.ini in "\TFT\src\User" is not in sync anymore with the one in "\Copy to SD Card root directory to update\"
   I'm not sure why there is one in "\TFT\src\User" anyway, as it seems not to be used anywhere (I know it has been like this for a while, not a new thing).

[digant73]

no, I changed the logic to be easy to understand:

• advanced_ok off -> 1 gcode per time (backward compatibility with current solution)
• advanced_of on -> Marlin ADVANCED_OK if detected by TFT or static ADVANCED_OK (using tx_slots configured in config.ini)
  Description on PR and config.ini have been already updated in the past

3. The HOST_SLOTS enumeration seems a bit odd to me:

typedef enum
{
  HOST_SLOTS_GENERIC_OK = -1,
} HOST_SLOTS;

Even if ADVANCED_OK is enabled in Marlin, for some gcodes the OK ACK returned by mainboard will not provide an indication of new available tx slots (e.g. answer for M105 ok T:16.13 /0.00 B:16.64 /0.00 @:0 B@:0\n). HOST_SLOTS_GENERIC_OK will allow to update tx_slots up to the last target provided by Marlin (e.g. if the last target indication was 2 it will be limited to 2)

4. The config.ini in "\TFT\src\User" is not in sync anymore with the one in "\Copy to SD Card root directory to update"
   I'm not sure why there is one in "\TFT\src\User" anyway, as it seems not to be used anywhere (I know it has been like this for a while, not a new thing).

The master config.ini must be provided only in "\TFT\src\User" since some time now. They will be copied in "\Copy to SD Card root directory to update" when merged by BTT.

[rondlh]

• advanced_ok off -> 1 gcode per time (backward compatibility with current solution)

• advanced_of on -> Marlin ADVANCED_OK if detected by TFT or static ADVANCED_OK (using tx_slots configured in config.ini)
  Description on PR and config.ini have been already updated in the past

So does this mean that ADVANCED_OK is not correctly detected? It's enabled in Marlin and the TFT, but still not used by the TFT.

[digant73]

Marlin ADVANCED_OK (its buffer size (e.g. 15 in my case)) is properly detected by TFT if ADAVNCED_OK is enabled in Marlin. If not detected then static ADVANCED_OK is used.
But in both cases advanced_ok must be also enabled. Otherwise standard 1 gcode per time is used.

[rondlh]

About 1. Fixed now, good job.
About 2. Issue still remains
About 3. My point is that an enumeration with only 1 item is a bit odd, a simple #DEFINE would do the job, but no big deal.
About 4.

The master config.ini must be provided only in "\TFT\src\User" since some time now. They will be copied in "\Copy to
SD Card root directory to update" when merged by BTT.

In that case be aware that the config.ini in "\TFT\src\User" is incorrect, the one in "\Copy to
SD Card root directory to update" is the correct one.

[digant73]

About 1. Fixed now, good job. About 2. Issue still remains About 3. My point is that an enumeration with only 1 item is a bit odd, a simple #DEFINE would do the job, but no big deal. About 4.

The master config.ini must be provided only in "\TFT\src\User" since some time now. They will be copied in "\Copy to
SD Card root directory to update" when merged by BTT.

In that case be aware that the config.ini in "\TFT\src\User" is incorrect, the one in "\Copy to SD Card root directory to update" is the correct one.

oh yes right, thanks for the review. Fixed 2 and 4 (as I said master folder for config.ini is \TFT\src\User). For 3, yes I know but it is better than a #define or passing a value < 0 on handleOkAck() function

[rondlh]

About 2. 50% complete now (#define TX_SLOTS 2 // Default: 1)
About 3. I don't understand why, but no big deal
About 4. Seems to be fixed now

I noticed another issue. When I start a print and immediately cancel it via the TFT, then the motherboard is flooded with M108 commands, sometimes causing a buffer overruns (my buffer is even at 1024 bytes). I'm not sure what is causing this, perhaps the initialization of the BLTouch (quite slow) or the ongoing homing process is causing a delay giving time to send too many M108 commands. I guess it's better to limit the max number of M108 commands that can be send.

In printing.c

  // If there's any ongoing blocking command, "M108" will take that out from the closed loop and a response will be received
  // from that command. Than another "M108" will be sent to unlock a next possible blocking command.
  // This way enough "M108" will be sent to unlock all possible blocking command(s) (ongoing or enqueued) but not too much and
  // not too fast one after another to overload/overrun the serial communication
  TASK_LOOP_WHILE(condCallback(), if (Serial_Available(SERIAL_PORT) != 0) sendEmergencyCmd("M108\n"));

[digant73]

yes I know that. I previously put a delay of 10 ms on that loop just to avoid that errors. I will see how to improve it

[rondlh]

A delay would probably do the trick just fine, or something like this:

if (Serial_Available(SERIAL_PORT) != 0)
  for (int i = 0; (i < 4) && condCallback(); i++)
    sendEmergencyCmd("M108\n");

The amount of commands that need to be cancelled is quite limited.

[rondlh]

While your are changing interfaceCmd.c, perhaps you could correct the error below ("for" and "wait" should be swapped in the last line)

// Store gcode cmd to cmdQueue queue.
// This command will be sent to the printer by sendQueueCmd().
// If the cmdQueue queue is full, a reminder message is displayed
// and it will for wait the queue to be able to store the command.

[digant73]

While your are changing interfaceCmd.c, perhaps you could correct the error below ("for" and "wait" should be swapped in the last line)

// Store gcode cmd to cmdQueue queue.
// This command will be sent to the printer by sendQueueCmd().
// If the cmdQueue queue is full, a reminder message is displayed
// and it will for wait the queue to be able to store the command.

ok, swapped. Fixed also the possible issue on print abort

[rondlh]

@digant73 YOU ARE UNSTOPPABLE! Great job.
BTW: I just posted another BR, not a big one, but a nice one :D ... not related to ADVANCED_OK

[kisslorand]

I noticed another issue. When I start a print and immediately cancel it via the TFT, then the motherboard is flooded with M108 commands, sometimes causing a buffer overruns (my buffer is even at 1024 bytes). I'm not sure what is causing this

It is caused by the "improvements" done to the serial port read. I warned about it being buggy several times already, but I saw the new "enhancements" made it even more chaotic. Just check earlier builds (before PR #2788) or my builds where M108 do not exhibit this behaviour.

I guess it's better to limit the max number of M108 commands that can be send.

It is the very unprofessional to mitigate the outcome of a bug instead of fixing its core root. It should be absolutely the last resort. I saw in a current open PR that it does exactly that, a patch was made to somehow overcome the issue not the cause of it.

[rondlh]

@kisslorand More code, less words please. If you want to help, then be concrete, your vague statements are useless to me, and so is closed source software.
What changes do you think cause this issue? (Which lines of code? Which strategy aspects? What assumptions are wrong?)
Can you show me your solution (code please...)? Or at least describe a strategy?

[digant73]

What changes do you think cause this issue? (Which lines of code? Which strategy aspects? What assumptions are wrong?) Can you show me your solution (code please...)? Or at least describe a strategy?

Transmission of M108 was previously based on polling rx_ok although there was a scenario where it failed not sending any M108, so not aborting the print and potentially causing a freeze in the loop.
The logic in #2824 is basically the same plus it always works now in any scenario (just read the comment in the line where rIndex_old is defined). Variable rIndex could be also avoided. It has been used simply because the binary is even smaller than using two invokations of Serial_GetReadingIndex(SERIAL_PORT).

[rondlh]

Thanks, that's helpful, this is one of the areas in the code that I don't fully understand, it must be boring to have to explain things to newbies sometimes, thanks for that. It seems to work fine in my experience.

[digant73]

Thanks, that's helpful, this is one of the areas in the code that I don't fully understand, it must be boring to have to explain things to newbies sometimes, thanks for that. It seems to work fine in my experience.

no problem. The proposed logic works even better than the old one and overall (considering the logic for rx_ok is no more necessary) the code was even reduced. Trying not to be polemic, I would provide the right weight to the continuous proclaims / biased / catastrophic / pessimistic words from kisslorand. Simply enjoy the overall fixes/improvements. The TFT is pretty much stable since some time now. It only requires some minor bug fixes / minor code optimization / minor new features. Basically it needs only a minor maintenance. I asked BTT to merge many of the old PRs but consider that possibly they have no more interest on this project as they are moving towards Klipper.

[rondlh]

@digant73 Could you tell me what detectAdvancedOk does? It seems to be trying to query the amount of Marlin command buffer from a M105 response. But my M105 response looks like this:
ok T:36.88/0 B:34.83/0 @:0

My advanced OK response looks like this:
ok P15 B7

I added Target TX Slots to the monitoring screen, which reports a value of 5, my Marlin command buffer size is set to 8.
Free TX slots also reports 5. Pending/buffered gcodes are both 0. Does this make sense?
I debugged the code a bit and can see it's looking at a correct advanced ok response: "ok P15 B5"
So the reported 5 is correct, but that is not the amount of my command buffers.

[digant73]

@digant73 Could you tell me what detectAdvancedOk does? It seems to be trying to query the amount of Marlin command buffer from a M105 response. But my M105 response looks like this: ok T:36.88/0 B:34.83/0 @:0

My advanced OK response looks like this: ok P15 B7

I added Target TX Slots to the monitoring screen, which reports a value of 5, my Marlin command buffer size is set to 8. Free TX slots also reports 5. Pending/buffered gcodes are both 0. Does this make sense? I debugged the code a bit and can see it's looking at a correct advanced ok response: "ok P15 B5" So the reported 5 is correct, but that is not the amount of my command buffers.

my mistake! I forgot to push the correct gcode.c file. M107 (or any other gcode replied with ok Px By must be used. I used also M105 just for testing detectAdvancedOk. Just pushed the proper file gcode.c now.
detectAdvancedOk() will simply parse the ok reply and retrieve the Marlin queue size (e.g. 7 in your and also my case).

[rondlh]

OK, got it. I changed "M105" to "M107", now I get the correct value of 7. Thanks.

I see instability in the Marlin RX data... (Seems to be something in my changes...)
I monitor this from Marlin's side, with every received command I report how much RX data is still available and how many command buffers are used. I can see all (8) buffers are full, but still 600 bytes in the input buffer. (1 command is about 40 characters) so something is going wrong. I have DMA writing enabled, but with my simple advance ok implementation I never saw this issue. I will try the #2824 code without any changes

[rondlh]

@digant73 I found a little issue in #2824...
NotificationMenu.c

void menuNotification(void)
{
  LISTITEMS notificationItems = {
    LABEL_NOTIFICATIONS,
    // icon            item type   item title     item value text(only for custom value)
    {
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_BLANK, LIST_LABEL, LABEL_CLEAR,   LABEL_NULL},
      #ifdef DEBUG_MONITORING
        {CHARICON_BLANK, LIST_LABEL, LABEL_INFO,    LABEL_NULL},
      #else
        {CHARICON_BACK,  LIST_LABEL, LABEL_NULL,    LABEL_NULL},
      #endif
      {CHARICON_BACK,  LIST_LABEL, LABEL_NULL,    LABEL_NULL},
    }
  };

I think the "#else" case should be:
{CHARICON_NULL, LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},

Otherwise you still get a 2nd useless "back icon" in the notification menu, while DEBUG_MONITORING is not defined.

Advanced_OK seems to work fine, Marlin RX buffer use is very moderate, but when I add the DMA writing code, the buffer usage can become unstable, very odd, because your Advanced_ok implementation is smarter than mine and should not be affected by the serial send speed. I will look into it...

[digant73]

@digant73 I found a little issue in #2824... NotificationMenu.c

void menuNotification(void)
{
  LISTITEMS notificationItems = {
    LABEL_NOTIFICATIONS,
    // icon            item type   item title     item value text(only for custom value)
    {
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_NULL,  LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},
      {CHARICON_BLANK, LIST_LABEL, LABEL_CLEAR,   LABEL_NULL},
      #ifdef DEBUG_MONITORING
        {CHARICON_BLANK, LIST_LABEL, LABEL_INFO,    LABEL_NULL},
      #else
        {CHARICON_BACK,  LIST_LABEL, LABEL_NULL,    LABEL_NULL},
      #endif
      {CHARICON_BACK,  LIST_LABEL, LABEL_NULL,    LABEL_NULL},
    }
  };

I think the "#else" case should be: {CHARICON_NULL, LIST_LABEL, LABEL_DYNAMIC, LABEL_NULL},

yes of course. Wrong cut & paste. I will push the minor fix

Otherwise you still get a back icon in the notification menu, while DEBUG_MONITORING is not defined.

Advanced_OK seems to work fine, Marlin RX buffer use is very moderate, but when I add the DMA writing code, the buffer usage can become unstable, very odd, because your Advanced_ok implementation is smarter than mine and should not be affected by the serial send speed. I will look into it...

Good it works now. Yes I think DMA writing should be investigated separately

[rondlh]

Good it works now. Yes I think DMA writing should be investigated separately

Yes, of course.
I don't like to post the DMA code when it causes issues for #2824, because it's build on it.
I found a bug in my "improvements" causing some issues, now Advanced ok and DMA writing work together fine.

I guess you are aware of this... but anyway.
The number of command buffers Marlin reports in the Advanced_ok response is actually 1 too low, because the command causing the ok response is occupying a command buffer when the message is send. Sending the ok response is the last thing done by the gcode command, so the command buffer will become available about 1us after the message is sent. As a result the TFT targets 7 command buffers when there are actually 8 available. Perhaps we can consider it a safety margin to not stress the Marlin RX buffers too much.

Some more ideas for the monitoring feature.... commands per second, kbytes sent, kbytes received, polling rate (Serial_Get).

[digant73]

Good it works now. Yes I think DMA writing should be investigated separately

Yes, of course. I don't like to post the DMA code when it causes issues for #2824, because it's build on it. I found a bug in my "improvements" causing some issues, now Advanced ok and DMA writing work together fine.

I guess you are aware of this... but anyway. The number of command buffers Marlin reports in the Advanced_ok response is actually 1 too low, because the command causing the ok response is occupying a command buffer when the message is send. Sending the ok response is the last thing done by the gcode command, so the command buffer will become available about 1us after the message is sent. As a result the TFT targets 7 command buffers when there are actually 8 available. Perhaps we can consider it a safety margin to not stress the Marlin RX buffers too much.

Some more ideas for the monitoring feature.... commands per second, kbytes sent, kbytes received, polling rate (Serial_Get).

Yes I know the reported available buffer is 1 less than the configured value in Marlin. That's ok for me (safety value).
Yes I also thought to KPIs such as msg/sec etc... I always provide those kind of KPIs in applications developed for my work.

[rondlh]

@V1EngineeringInc/Everyone This would be a good time to test ADVANCED_OK #2824. I tested it fully, and it is working very well.
I hope we can get this merged asap, because not having this is holding me back currently. This PR involves a language update to add the word "Advanced OK", so witching back is painful. I have the DMA serial writing ready for test, and some good performance improvements are in progress, but they depend on this update. Perhaps you can help?

[rondlh]

#2824 now contains a performance benchmark that you can enable in Configuration.h (DEBUG_MONITORING). When enabled, you will find an info button in the in the notification screen. The button brings you to the monitoring screen. There you can monitor some system parameters, like the scan rate which shows how many times the core process (loopProcess) is executed every second. This way the impact of changes can be tested easily.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.